From 3b9fac1563a75a571b512887602eb53f82e565bf Mon Sep 17 00:00:00 2001 From: sh0rez Date: Wed, 27 Nov 2019 19:02:57 +0100 Subject: [PATCH] fix(kubernetes): properly handle missing namespaces (#120) * fix(kubernetes): properly handle missing namespaces Most cluster native operations won't succeed if the namespaces of the object is yet to be created, even though it will be just created. To enable a better user experience than kubectl does, let's do the following: - on diff, just report every object in a missing object as entirely new - on apply, create namespaces first to succeed in a single run (needed two before) * doc(kubernetes): util.DiffName godoc --- pkg/kubernetes/client/apply.go | 17 ++++++ pkg/kubernetes/client/client.go | 3 ++ pkg/kubernetes/client/diff.go | 68 ++++++++++++++++++++++++ pkg/kubernetes/client/kubectl.go | 50 ++++++++++------- pkg/kubernetes/kubernetes.go | 3 +- pkg/kubernetes/subsetdiff.go | 11 ++-- pkg/kubernetes/{util.go => util/diff.go} | 22 ++++++-- 7 files changed, 140 insertions(+), 34 deletions(-) create mode 100644 pkg/kubernetes/client/diff.go rename pkg/kubernetes/{util.go => util/diff.go} (75%) diff --git a/pkg/kubernetes/client/apply.go b/pkg/kubernetes/client/apply.go index ccdddc4e1..f24ef0e8d 100644 --- a/pkg/kubernetes/client/apply.go +++ b/pkg/kubernetes/client/apply.go @@ -6,10 +6,21 @@ import ( "strings" "github.com/grafana/tanka/pkg/kubernetes/manifest" + funk "github.com/thoas/go-funk" ) // Apply applies the given yaml to the cluster func (k Kubectl) Apply(data manifest.List, opts ApplyOpts) error { + // create namespaces first to succeed first try + ns := filterNamespace(data) + if err := k.apply(ns, opts); err != nil { + return err + } + + return k.apply(data, opts) +} + +func (k Kubectl) apply(data manifest.List, opts ApplyOpts) error { argv := []string{"apply", "--context", k.context.Get("name").MustStr(), "-f", "-", @@ -26,3 +37,9 @@ func (k Kubectl) Apply(data manifest.List, opts ApplyOpts) error { return cmd.Run() } + +func filterNamespace(in manifest.List) manifest.List { + return manifest.List(funk.Filter(in, func(i manifest.Manifest) bool { + return strings.ToLower(i.Kind()) == "namespace" + }).([]manifest.Manifest)) +} diff --git a/pkg/kubernetes/client/client.go b/pkg/kubernetes/client/client.go index 8f57a3d97..a67e0c210 100644 --- a/pkg/kubernetes/client/client.go +++ b/pkg/kubernetes/client/client.go @@ -25,6 +25,9 @@ type Client interface { Delete(namespace, kind, name string, opts DeleteOpts) error DeleteByLabels(namespace string, labels map[string]interface{}, opts DeleteOpts) error + // Namespaces the cluster currently has + Namespaces() (map[string]bool, error) + // Info returns known informational data about the client. Best effort based, // fields of `Info` that cannot be stocked with valuable data, e.g. // due to an error, shall be left nil. diff --git a/pkg/kubernetes/client/diff.go b/pkg/kubernetes/client/diff.go new file mode 100644 index 000000000..f0fc4bd88 --- /dev/null +++ b/pkg/kubernetes/client/diff.go @@ -0,0 +1,68 @@ +package client + +import ( + "bytes" + "os/exec" + "regexp" + "strings" + + "github.com/grafana/tanka/pkg/kubernetes/manifest" + "github.com/grafana/tanka/pkg/kubernetes/util" +) + +// DiffServerSide takes the desired state and computes the differences on the +// server, returning them in `diff(1)` format +func (k Kubectl) DiffServerSide(data manifest.List) (*string, error) { + ns, err := k.Namespaces() + if err != nil { + return nil, err + } + + ready, missing := separateMissingNamespace(data, ns) + argv := []string{"diff", + "--context", k.context.Get("name").MustStr(), + "-f", "-", + } + cmd := exec.Command("kubectl", argv...) + + raw := bytes.Buffer{} + cmd.Stdout = &raw + cmd.Stderr = FilterWriter{regexp.MustCompile(`exit status \d`)} + + cmd.Stdin = strings.NewReader(ready.String()) + + err = cmd.Run() + + // kubectl uses exit status 1 to tell us that there is a diff + if exitError, ok := err.(*exec.ExitError); ok && exitError.ExitCode() == 1 { + } else if err != nil { + return nil, err + } + + s := raw.String() + for _, r := range missing { + d, err := util.DiffStr(util.DiffName(r), "", r.String()) + if err != nil { + return nil, err + } + s += d + } + + if s != "" { + return &s, nil + } + + // no diff -> nil + return nil, nil +} + +func separateMissingNamespace(in manifest.List, exists map[string]bool) (ready, missingNamespace manifest.List) { + for _, r := range in { + if !exists[r.Metadata().Namespace()] { + missingNamespace = append(missingNamespace, r) + continue + } + ready = append(ready, r) + } + return +} diff --git a/pkg/kubernetes/client/kubectl.go b/pkg/kubernetes/client/kubectl.go index 138b40e33..ed67c7170 100644 --- a/pkg/kubernetes/client/kubectl.go +++ b/pkg/kubernetes/client/kubectl.go @@ -2,10 +2,11 @@ package client import ( "bytes" + "encoding/json" + "fmt" "os" "os/exec" "regexp" - "strings" "github.com/Masterminds/semver" "github.com/pkg/errors" @@ -66,35 +67,44 @@ func (k Kubectl) version() (client, server *semver.Version, err error) { return client, server, nil } -// DiffServerSide takes the desired state and computes the differences on the -// server, returning them in `diff(1)` format -func (k Kubectl) DiffServerSide(data manifest.List) (*string, error) { - argv := []string{"diff", +// Namespaces of the cluster +func (k Kubectl) Namespaces() (map[string]bool, error) { + argv := []string{"get", + "-o", "json", "--context", k.context.Get("name").MustStr(), - "-f", "-", + "namespaces", } cmd := exec.Command("kubectl", argv...) - raw := bytes.Buffer{} - cmd.Stdout = &raw - cmd.Stderr = FilterWriter{regexp.MustCompile(`exit status \d`)} - - cmd.Stdin = strings.NewReader(data.String()) + var sout bytes.Buffer + cmd.Stdout = &sout + cmd.Stderr = os.Stderr err := cmd.Run() - - // kubectl uses exit status 1 to tell us that there is a diff - if exitError, ok := err.(*exec.ExitError); ok && exitError.ExitCode() == 1 { - s := raw.String() - return &s, nil - } - // another error if err != nil { return nil, err } - // no diff -> nil - return nil, nil + var list manifest.Manifest + if err := json.Unmarshal(sout.Bytes(), &list); err != nil { + return nil, err + } + + items, ok := list["items"].([]interface{}) + if !ok { + return nil, fmt.Errorf("listing namespaces: expected items to be an object, but got %T instead", list["items"]) + } + + namespaces := make(map[string]bool) + for _, i := range items { + m, err := manifest.New(i.(map[string]interface{})) + if err != nil { + return nil, err + } + + namespaces[m.Metadata().Name()] = true + } + return namespaces, nil } // FilterWriter is an io.Writer that discards every message that matches at diff --git a/pkg/kubernetes/kubernetes.go b/pkg/kubernetes/kubernetes.go index bf9fb14f1..19d2a64b9 100644 --- a/pkg/kubernetes/kubernetes.go +++ b/pkg/kubernetes/kubernetes.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/tanka/pkg/cli" "github.com/grafana/tanka/pkg/kubernetes/client" "github.com/grafana/tanka/pkg/kubernetes/manifest" + "github.com/grafana/tanka/pkg/kubernetes/util" "github.com/grafana/tanka/pkg/spec/v1alpha1" ) @@ -117,7 +118,7 @@ func (k *Kubernetes) Diff(state manifest.List, opts DiffOpts) (*string, error) { } if opts.Summarize { - return diffstat(*d) + return util.Diffstat(*d) } return d, nil diff --git a/pkg/kubernetes/subsetdiff.go b/pkg/kubernetes/subsetdiff.go index 820e136e1..f3af4b2ab 100644 --- a/pkg/kubernetes/subsetdiff.go +++ b/pkg/kubernetes/subsetdiff.go @@ -1,7 +1,6 @@ package kubernetes import ( - "fmt" "strings" "github.com/pkg/errors" @@ -9,6 +8,7 @@ import ( "github.com/grafana/tanka/pkg/kubernetes/client" "github.com/grafana/tanka/pkg/kubernetes/manifest" + "github.com/grafana/tanka/pkg/kubernetes/util" ) type difference struct { @@ -49,7 +49,7 @@ func SubsetDiffer(c client.Client) Differ { var diffs string for _, d := range docs { - diffStr, err := diff(d.name, d.live, d.merged) + diffStr, err := util.DiffStr(d.name, d.live, d.merged) if err != nil { return nil, errors.Wrap(err, "invoking diff") } @@ -78,12 +78,7 @@ func parallelSubsetDiff(c client.Client, should manifest.Manifest, r chan differ } func subsetDiff(c client.Client, m manifest.Manifest) (*difference, error) { - name := strings.Replace(fmt.Sprintf("%s.%s.%s.%s", - m.APIVersion(), - m.Kind(), - m.Metadata().Namespace(), - m.Metadata().Name(), - ), "/", "-", -1) + name := util.DiffName(m) // kubectl output -> current state rawIs, err := c.Get( diff --git a/pkg/kubernetes/util.go b/pkg/kubernetes/util/diff.go similarity index 75% rename from pkg/kubernetes/util.go rename to pkg/kubernetes/util/diff.go index a415c6eb8..c4c689de9 100644 --- a/pkg/kubernetes/util.go +++ b/pkg/kubernetes/util/diff.go @@ -1,4 +1,4 @@ -package kubernetes +package util import ( "bytes" @@ -9,11 +9,23 @@ import ( "path/filepath" "regexp" "strings" + + "github.com/grafana/tanka/pkg/kubernetes/manifest" ) -// diff computes the differences between the strings `is` and `should` using the +// DiffName computes the filename for use with `DiffStr` +func DiffName(m manifest.Manifest) string { + return strings.Replace(fmt.Sprintf("%s.%s.%s.%s", + m.APIVersion(), + m.Kind(), + m.Metadata().Namespace(), + m.Metadata().Name(), + ), "/", "-", -1) +} + +// Diff computes the differences between the strings `is` and `should` using the // UNIX `diff(1)` utility. -func diff(name, is, should string) (string, error) { +func DiffStr(name, is, should string) (string, error) { dir, err := ioutil.TempDir("", "diff") if err != nil { return "", err @@ -49,8 +61,8 @@ func diff(name, is, should string) (string, error) { return out, nil } -// diffstat uses `diffstat(1)` utility to summarize a `diff(1)` output -func diffstat(d string) (*string, error) { +// Diffstat uses `diffstat(1)` utility to summarize a `diff(1)` output +func Diffstat(d string) (*string, error) { cmd := exec.Command("diffstat", "-C") buf := bytes.Buffer{} cmd.Stdout = &buf