From a36d5b76be62d99e722afea87cd92d2132d8b04e Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Fri, 20 Mar 2020 09:33:06 -0700 Subject: [PATCH] Skip writing to unedited files --- kyaml/setters2/set.go | 39 ++++++ kyaml/setters2/set_test.go | 154 ++++++++++++++++++++++ kyaml/setters2/settersutil/fieldsetter.go | 6 +- 3 files changed, 197 insertions(+), 2 deletions(-) diff --git a/kyaml/setters2/set.go b/kyaml/setters2/set.go index f5d163015..11b0f5714 100644 --- a/kyaml/setters2/set.go +++ b/kyaml/setters2/set.go @@ -8,7 +8,10 @@ import ( "github.com/go-openapi/spec" "sigs.k8s.io/kustomize/kyaml/errors" + "sigs.k8s.io/kustomize/kyaml/kio" + "sigs.k8s.io/kustomize/kyaml/kio/kioutil" "sigs.k8s.io/kustomize/kyaml/openapi" + "sigs.k8s.io/kustomize/kyaml/sets" "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -297,3 +300,39 @@ func (s SetOpenAPI) Filter(object *yaml.RNode) (*yaml.RNode, error) { return object, nil } + +// SetAll applies the set filter for all yaml nodes and only returns the nodes whose +// corresponding file has at least one node with input setter +func SetAll(s *Set) kio.Filter { + return kio.FilterFunc(func(nodes []*yaml.RNode) ([]*yaml.RNode, error) { + filesToUpdate := sets.String{} + // for each node record the set fields count before and after filter is applied and + // store the corresponding file paths if there is an increment in setters count + for i := range nodes { + preCount := s.Count + _, err := s.Filter(nodes[i]) + if err != nil { + return nil, errors.Wrap(err) + } + if s.Count > preCount { + path, _, err := kioutil.GetFileAnnotations(nodes[i]) + if err != nil { + return nil, errors.Wrap(err) + } + filesToUpdate.Insert(path) + } + } + var nodesInUpdatedFiles []*yaml.RNode + // return only the nodes whose corresponding file has at least one node with input setter + for i := range nodes { + path, _, err := kioutil.GetFileAnnotations(nodes[i]) + if err != nil { + return nil, errors.Wrap(err) + } + if filesToUpdate.Has(path) { + nodesInUpdatedFiles = append(nodesInUpdatedFiles, nodes[i]) + } + } + return nodesInUpdatedFiles, nil + }) +} diff --git a/kyaml/setters2/set_test.go b/kyaml/setters2/set_test.go index 64610ca9e..a3dece385 100644 --- a/kyaml/setters2/set_test.go +++ b/kyaml/setters2/set_test.go @@ -702,6 +702,160 @@ spec: } } +func TestSet_SetAll(t *testing.T) { + var tests = []struct { + name string + description string + setter string + openapi string + input []string + expected []string + }{ + { + name: "set-replicas-same-file", + setter: "replicas", + openapi: ` +openAPI: + definitions: + io.k8s.cli.setters.replicas: + x-k8s-cli: + setter: + name: replicas + value: "4" + `, + input: []string{` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment + annotations: + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'cluster.yaml' +spec: + replicas: 3 # {"$ref": "#/definitions/io.k8s.cli.setters.replicas"} + `, ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment2 + annotations: + config.kubernetes.io/index: '1' + config.kubernetes.io/path: 'cluster.yaml' +spec: + replicas: 10 + `}, + expected: []string{` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment + annotations: + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'cluster.yaml' +spec: + replicas: 4 # {"$ref": "#/definitions/io.k8s.cli.setters.replicas"} + `, ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment2 + annotations: + config.kubernetes.io/index: '1' + config.kubernetes.io/path: 'cluster.yaml' +spec: + replicas: 10 + `}, + }, + { + name: "set-replicas-different-file", + setter: "replicas", + openapi: ` +openAPI: + definitions: + io.k8s.cli.setters.replicas: + x-k8s-cli: + setter: + name: replicas + value: "4" + `, + input: []string{` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment + annotations: + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'cluster.yaml' +spec: + replicas: 3 # {"$ref": "#/definitions/io.k8s.cli.setters.replicas"} + `, ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment2 + annotations: + config.kubernetes.io/index: '1' + config.kubernetes.io/path: 'another_cluster.yaml' +spec: + replicas: 10 + `}, + expected: []string{` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment + annotations: + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'cluster.yaml' +spec: + replicas: 4 # {"$ref": "#/definitions/io.k8s.cli.setters.replicas"} + `}, + }, + } + for i := range tests { + test := tests[i] + t.Run(test.name, func(t *testing.T) { + // reset the openAPI afterward + defer openapi.ResetOpenAPI() + initSchema(t, test.openapi) + + // parse the input to be modified + var inputNodes []*yaml.RNode + for _, s := range test.input { + r, err := yaml.Parse(s) + if !assert.NoError(t, err) { + t.FailNow() + } + inputNodes = append(inputNodes, r) + } + + // invoke the setter + instance := &Set{Name: test.setter} + result, err := SetAll(instance).Filter(inputNodes) + if !assert.NoError(t, err) { + t.FailNow() + } + + // compare the actual and expected output + if !assert.NoError(t, err) { + t.FailNow() + } + if !assert.Equal(t, len(result), len(test.expected)) { + t.FailNow() + } + + for i := range result { + actual, _ := result[i].String() + actual = strings.TrimSpace(actual) + expected := strings.TrimSpace(test.expected[i]) + if !assert.Equal(t, expected, actual) { + t.FailNow() + } + } + }) + } +} + // initSchema initializes the openAPI with the definitions from s func initSchema(t *testing.T, s string) { // parse out the schema from the input openAPI diff --git a/kyaml/setters2/settersutil/fieldsetter.go b/kyaml/setters2/settersutil/fieldsetter.go index 456a9cab6..13361137f 100644 --- a/kyaml/setters2/settersutil/fieldsetter.go +++ b/kyaml/setters2/settersutil/fieldsetter.go @@ -57,11 +57,13 @@ func (fs FieldSetter) Set(openAPIPath, resourcesPath string) (int, error) { } // Update the resources with the new value - inout := &kio.LocalPackageReadWriter{PackagePath: resourcesPath} + // Set NoDeleteFiles to true as SetAll will return only the nodes of files which should be updated and + // hence, rest of the files should not be deleted + inout := &kio.LocalPackageReadWriter{PackagePath: resourcesPath, NoDeleteFiles: true} s := &setters2.Set{Name: fs.Name} err := kio.Pipeline{ Inputs: []kio.Reader{inout}, - Filters: []kio.Filter{kio.FilterAll(s)}, + Filters: []kio.Filter{setters2.SetAll(s)}, Outputs: []kio.Writer{inout}, }.Execute() return s.Count, err