From 3248bb935e21cb3056a79c1a3f96251310f06e5f Mon Sep 17 00:00:00 2001 From: sh0rez Date: Wed, 7 Aug 2019 20:31:18 +0200 Subject: [PATCH] fix(kubernetes/diff): major fixes, performance (#12) Addresses several open points that made using subset-diff impossible: - produced a `panic` in case a slice returned from kubernetes was bigger than the one in the config - showed differences on the apiVersion. While those were indeed valid, it was too much bloat so they are removed for now - parallelizes generation of the subset. This includes parallel invocations of kubectl, which give an additional performance boost :tada: --- pkg/kubernetes/kubernetes.go | 2 +- pkg/kubernetes/subsetdiff.go | 139 ++++++++++++++++++++---------- pkg/kubernetes/subsetdiff_test.go | 6 ++ 3 files changed, 101 insertions(+), 46 deletions(-) diff --git a/pkg/kubernetes/kubernetes.go b/pkg/kubernetes/kubernetes.go index efca903bf..5efb2dc94 100644 --- a/pkg/kubernetes/kubernetes.go +++ b/pkg/kubernetes/kubernetes.go @@ -87,7 +87,7 @@ func (k *Kubernetes) Diff(state []Manifest) (string, error) { if k.Spec.DiffStrategy == "" { k.Spec.DiffStrategy = "native" if _, server, err := k.client.Version(); err == nil { - if !server.GreaterThan(semver.MustParse("0.13.0")) { + if !server.GreaterThan(semver.MustParse("1.13.0")) { k.Spec.DiffStrategy = "subset" } } diff --git a/pkg/kubernetes/subsetdiff.go b/pkg/kubernetes/subsetdiff.go index fdee1646c..7cf59058b 100644 --- a/pkg/kubernetes/subsetdiff.go +++ b/pkg/kubernetes/subsetdiff.go @@ -13,14 +13,19 @@ import ( ) type difference struct { + name string live, merged string } func (k Kubectl) SubsetDiff(y string) (string, error) { - docs := map[string]difference{} + docs := []difference{} d := yaml.NewDecoder(strings.NewReader(y)) - for { + routines := 0 + errCh := make(chan error) + resultCh := make(chan difference) + + for { // jsonnet output -> desired state var rawShould map[interface{}]interface{} err := d.Decode(&rawShould) @@ -32,57 +37,87 @@ func (k Kubectl) SubsetDiff(y string) (string, error) { return "", errors.Wrap(err, "decoding yaml") } - // filename - m := objx.New(util.CleanupInterfaceMap(rawShould)) - name := strings.Replace(fmt.Sprintf("%s.%s.%s.%s", - m.Get("apiVersion").MustStr(), - m.Get("kind").MustStr(), - m.Get("metadata.namespace").MustStr(), - m.Get("metadata.name").MustStr(), - ), "/", "-", -1) - - // kubectl output -> current state - rawIs, err := k.Get( - m.Get("metadata.namespace").MustStr(), - m.Get("kind").MustStr(), - m.Get("metadata.name").MustStr(), - ) - if err != nil { - if _, ok := err.(ErrorNotFound); ok { - rawIs = map[string]interface{}{} - } else { - return "", errors.Wrap(err, "getting state from cluster") - } - } + routines++ + go subsetDiff(k, rawShould, resultCh, errCh) - should, err := yaml.Marshal(rawShould) - if err != nil { - return "", err - } + } - is, err := yaml.Marshal(subset(m, rawIs)) - if err != nil { - return "", err + var lastErr error + for i := 0; i < routines; i++ { + select { + case d := <-resultCh: + docs = append(docs, d) + case err := <-errCh: + lastErr = err } - if string(is) == "{}\n" { - is = []byte("") - } - docs[name] = difference{string(is), string(should)} + } + close(resultCh) + close(errCh) + + if lastErr != nil { + return "", errors.Wrap(lastErr, "calculating subset") } - s := "" - for k, v := range docs { - d, err := diff(k, v.live, v.merged) + diffs := "" + for _, d := range docs { + diffStr, err := diff(d.name, d.live, d.merged) if err != nil { return "", errors.Wrap(err, "invoking diff") } - if d != "" { - d += "\n" + if diffStr != "" { + diffStr += "\n" } - s += d + diffs += diffStr } - return s, nil + return diffs, nil +} + +func subsetDiff(k Kubectl, rawShould map[interface{}]interface{}, r chan difference, e chan error) { + // filename + m := objx.New(util.CleanupInterfaceMap(rawShould)) + name := strings.Replace(fmt.Sprintf("%s.%s.%s.%s", + m.Get("apiVersion").MustStr(), + m.Get("kind").MustStr(), + m.Get("metadata.namespace").MustStr(), + m.Get("metadata.name").MustStr(), + ), "/", "-", -1) + + // kubectl output -> current state + rawIs, err := k.Get( + m.Get("metadata.namespace").MustStr(), + m.Get("kind").MustStr(), + m.Get("metadata.name").MustStr(), + ) + if err != nil { + if _, ok := err.(ErrorNotFound); ok { + rawIs = map[string]interface{}{} + } else { + e <- errors.Wrap(err, "getting state from cluster") + return + } + } + + should, err := yaml.Marshal(rawShould) + if err != nil { + e <- err + return + } + + is, err := yaml.Marshal(subset(m, rawIs)) + if err != nil { + e <- err + return + } + if string(is) == "{}\n" { + is = []byte("") + } + + r <- difference{ + name: name, + live: string(is), + merged: string(should), + } } // subset removes all keys from is, that are not present in should. @@ -93,6 +128,12 @@ func subset(should, is map[string]interface{}) map[string]interface{} { if should["namespace"] != nil { is["namespace"] = should["namespace"] } + + // just ignore the apiVersion for now, too much bloat + if should["apiVersion"] != nil && is["apiVersion"] != nil { + is["apiVersion"] = should["apiVersion"] + } + for k, v := range is { if should[k] == nil { delete(is, k) @@ -113,15 +154,23 @@ func subset(should, is map[string]interface{}) map[string]interface{} { case []interface{}: for i := range b { if a, ok := should[k].([]interface{}); ok { - aa, ok := a[i].(map[string]interface{}) + if i >= len(a) { + // slice in config shorter than in live. Abort, as there are no entries to diff anymore + break + } + + // value not a dict, no recursion needed + cShould, ok := a[i].(map[string]interface{}) if !ok { continue } - bb, ok := b[i].(map[string]interface{}) + + // value not a dict, no recursion needed + cIs, ok := b[i].(map[string]interface{}) if !ok { continue } - b[i] = subset(aa, bb) + b[i] = subset(cShould, cIs) } } } diff --git a/pkg/kubernetes/subsetdiff_test.go b/pkg/kubernetes/subsetdiff_test.go index 656fa9830..ed0245d76 100644 --- a/pkg/kubernetes/subsetdiff_test.go +++ b/pkg/kubernetes/subsetdiff_test.go @@ -99,6 +99,9 @@ func TestSubset(t *testing.T) { "bam": "bloo", "boo": "boar", }, + map[string]interface{}{ + "a": "b", + }, }, }, }, @@ -112,6 +115,9 @@ func TestSubset(t *testing.T) { map[string]interface{}{ "bam": "bloo", }, + map[string]interface{}{ + "a": "b", + }, }, }, },