From ec55ac1324cc5b6b515a5fb70a9a8f7d3c722505 Mon Sep 17 00:00:00 2001 From: Michael Bridgen <michael@weave.works> Date: Mon, 4 Jun 2018 10:37:45 +0100 Subject: [PATCH 1/3] Rename and group together kubernetes sync code Was named `release.go` for historical reasons, now named `sync.go` for clarity reasons. I've also moved the code that accumulates changes into sync.go. --- cluster/kubernetes/kubernetes.go | 24 ------------------- cluster/kubernetes/{release.go => sync.go} | 28 +++++++++++++++++++++- 2 files changed, 27 insertions(+), 25 deletions(-) rename cluster/kubernetes/{release.go => sync.go} (85%) diff --git a/cluster/kubernetes/kubernetes.go b/cluster/kubernetes/kubernetes.go index 4c8ef18da..b14aa2f2d 100644 --- a/cluster/kubernetes/kubernetes.go +++ b/cluster/kubernetes/kubernetes.go @@ -93,30 +93,6 @@ func isAddon(obj namespacedLabeled) bool { // --- /add ons -type changeSet struct { - nsObjs map[string][]*apiObject - noNsObjs map[string][]*apiObject -} - -func makeChangeSet() changeSet { - return changeSet{ - nsObjs: make(map[string][]*apiObject), - noNsObjs: make(map[string][]*apiObject), - } -} - -func (c *changeSet) stage(cmd string, o *apiObject) { - if o.hasNamespace() { - c.nsObjs[cmd] = append(c.nsObjs[cmd], o) - } else { - c.noNsObjs[cmd] = append(c.noNsObjs[cmd], o) - } -} - -type Applier interface { - apply(log.Logger, changeSet) cluster.SyncError -} - // Cluster is a handle to a Kubernetes API server. // (Typically, this code is deployed into the same cluster.) type Cluster struct { diff --git a/cluster/kubernetes/release.go b/cluster/kubernetes/sync.go similarity index 85% rename from cluster/kubernetes/release.go rename to cluster/kubernetes/sync.go index 16ca6790d..c50ebfc6c 100644 --- a/cluster/kubernetes/release.go +++ b/cluster/kubernetes/sync.go @@ -8,12 +8,38 @@ import ( "strings" "time" + rest "k8s.io/client-go/rest" + "github.com/go-kit/kit/log" "github.com/pkg/errors" "github.com/weaveworks/flux/cluster" - rest "k8s.io/client-go/rest" ) +type changeSet struct { + nsObjs map[string][]*apiObject + noNsObjs map[string][]*apiObject +} + +func makeChangeSet() changeSet { + return changeSet{ + nsObjs: make(map[string][]*apiObject), + noNsObjs: make(map[string][]*apiObject), + } +} + +func (c *changeSet) stage(cmd string, o *apiObject) { + if o.hasNamespace() { + c.nsObjs[cmd] = append(c.nsObjs[cmd], o) + } else { + c.noNsObjs[cmd] = append(c.noNsObjs[cmd], o) + } +} + +// Applier is something that will apply a changeset to the cluster. +type Applier interface { + apply(log.Logger, changeSet) cluster.SyncError +} + type Kubectl struct { exe string config *rest.Config From 8ad9943f3733af95e614441f3b3f66189fc93729 Mon Sep 17 00:00:00 2001 From: Michael Bridgen <michael@weave.works> Date: Mon, 4 Jun 2018 10:45:15 +0100 Subject: [PATCH 2/3] Sort resources to apply into dependency order Kubernetes resource kinds have a partial order relation of (loosely) "may refer to": a Deployment may mount a ConfigMap as a volume; most resources are scoped to a namespace; a RoleBinding refers to a Role or ClusterRole; and so on. Usually, you want the referenced resources to be created or changed before the referring resources. In general, Kubernetes is designed so that it will sort itself out eventually, but things go more smoothly if things are present before they are needed. Here I've boiled the ordering down to a small number of ranks, with each rank containing kinds that refer only to kinds in the ranks before. When applying resources, they are sorted by rank, so that resources that depend on other resources will get updated or created after those others. --- cluster/kubernetes/kubernetes.go | 12 +-- cluster/kubernetes/sync.go | 82 ++++++++++++++----- .../{kubernetes_test.go => sync_test.go} | 33 +++++++- 3 files changed, 99 insertions(+), 28 deletions(-) rename cluster/kubernetes/{kubernetes_test.go => sync_test.go} (69%) diff --git a/cluster/kubernetes/kubernetes.go b/cluster/kubernetes/kubernetes.go index b14aa2f2d..6a1b15131 100644 --- a/cluster/kubernetes/kubernetes.go +++ b/cluster/kubernetes/kubernetes.go @@ -43,13 +43,15 @@ type extendedClient struct { // --- internal types for keeping track of syncing +type metadata struct { + Name string `yaml:"name"` + Namespace string `yaml:"namespace"` +} + type apiObject struct { resource.Resource - Kind string `yaml:"kind"` - Metadata struct { - Name string `yaml:"name"` - Namespace string `yaml:"namespace"` - } `yaml:"metadata"` + Kind string `yaml:"kind"` + Metadata metadata `yaml:"metadata"` } // A convenience for getting an minimal object from some bytes. diff --git a/cluster/kubernetes/sync.go b/cluster/kubernetes/sync.go index c50ebfc6c..f23d8f66d 100644 --- a/cluster/kubernetes/sync.go +++ b/cluster/kubernetes/sync.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "os/exec" + "sort" "strings" "time" @@ -16,23 +17,15 @@ import ( ) type changeSet struct { - nsObjs map[string][]*apiObject - noNsObjs map[string][]*apiObject + objs map[string][]*apiObject } func makeChangeSet() changeSet { - return changeSet{ - nsObjs: make(map[string][]*apiObject), - noNsObjs: make(map[string][]*apiObject), - } + return changeSet{objs: make(map[string][]*apiObject)} } func (c *changeSet) stage(cmd string, o *apiObject) { - if o.hasNamespace() { - c.nsObjs[cmd] = append(c.nsObjs[cmd], o) - } else { - c.noNsObjs[cmd] = append(c.noNsObjs[cmd], o) - } + c.objs[cmd] = append(c.objs[cmd], o) } // Applier is something that will apply a changeset to the cluster. @@ -78,9 +71,50 @@ func (c *Kubectl) connectArgs() []string { return args } +// rankOfKind returns an int denoting the position of the given kind +// in the partial ordering of Kubernetes resources, according to which +// kinds depend on which (derived by hand). +func rankOfKind(kind string) int { + switch kind { + // Namespaces answer to NOONE + case "Namespace": + return 0 + // These don't go in namespaces; or do, but don't depend on anything else + case "ServiceAccount", "ClusterRole", "Role", "PersistentVolume", "Service": + return 1 + // These depend on something above, but not each other + case "ResourceQuota", "LimitRange", "Secret", "ConfigMap", "RoleBinding", "ClusterRoleBinding", "PersistentVolumeClaim", "Ingress": + return 2 + // Same deal, next layer + case "DaemonSet", "Deployment", "ReplicationController", "ReplicaSet", "Job", "CronJob", "StatefulSet": + return 3 + // Assumption: anything not mentioned isn't depended _upon_, so + // can come last. + default: + return 4 + } +} + +type applyOrder []*apiObject + +func (objs applyOrder) Len() int { + return len(objs) +} + +func (objs applyOrder) Swap(i, j int) { + objs[i], objs[j] = objs[j], objs[i] +} + +func (objs applyOrder) Less(i, j int) bool { + ranki, rankj := rankOfKind(objs[i].Kind), rankOfKind(objs[j].Kind) + if ranki == rankj { + return objs[i].Metadata.Name < objs[j].Metadata.Name + } + return ranki < rankj +} + func (c *Kubectl) apply(logger log.Logger, cs changeSet) (errs cluster.SyncError) { - f := func(m map[string][]*apiObject, cmd string, args ...string) { - objs := m[cmd] + f := func(objs []*apiObject, cmd string, args ...string) { if len(objs) == 0 { return } @@ -96,15 +130,19 @@ func (c *Kubectl) apply(logger log.Logger, cs changeSet) (errs cluster.SyncError } } - // When deleting resources we must ensure any resource in a non-default - // namespace is deleted before the namespace that it is in. Since namespace - // resources don't specify a namespace, this ordering guarantees that. - f(cs.nsObjs, "delete") - f(cs.noNsObjs, "delete", "--namespace", "default") - // Likewise, when applying resources we must ensure the namespace is applied - // first, so we run the commands the other way round. - f(cs.noNsObjs, "apply", "--namespace", "default") - f(cs.nsObjs, "apply") + // When deleting objects, the only real concern is that we don't + // try to delete things that have already been deleted by + // Kubernete's GC -- most notably, resources in a namespace which + // is also being deleted. GC does not have the dependency ranking, + // but we can use it as a shortcut to avoid the above problem at + // least. + objs := cs.objs["delete"] + sort.Sort(sort.Reverse(applyOrder(objs))) + f(objs, "delete") + + objs = cs.objs["apply"] + sort.Sort(applyOrder(objs)) + f(objs, "apply") return errs } diff --git a/cluster/kubernetes/kubernetes_test.go b/cluster/kubernetes/sync_test.go similarity index 69% rename from cluster/kubernetes/kubernetes_test.go rename to cluster/kubernetes/sync_test.go index b3c1d3565..8ae9066fc 100644 --- a/cluster/kubernetes/kubernetes_test.go +++ b/cluster/kubernetes/sync_test.go @@ -1,6 +1,7 @@ package kubernetes import ( + "sort" "testing" "github.com/go-kit/kit/log" @@ -15,7 +16,7 @@ type mockApplier struct { } func (m *mockApplier) apply(_ log.Logger, c changeSet) cluster.SyncError { - if len(c.nsObjs) != 0 || len(c.noNsObjs) != 0 { + if len(c.objs) != 0 { m.commandRun = true } return nil @@ -79,3 +80,33 @@ func TestSyncMalformed(t *testing.T) { t.Error("expected no commands run") } } + +// TestApplyOrder checks that applyOrder works as expected. +func TestApplyOrder(t *testing.T) { + objs := []*apiObject{ + { + Kind: "Deployment", + Metadata: metadata{ + Name: "deploy", + }, + }, + { + Kind: "Secret", + Metadata: metadata{ + Name: "secret", + }, + }, + { + Kind: "Namespace", + Metadata: metadata{ + Name: "namespace", + }, + }, + } + sort.Sort(applyOrder(objs)) + for i, name := range []string{"namespace", "secret", "deploy"} { + if objs[i].Metadata.Name != name { + t.Errorf("Expected %q at position %d, got %q", name, i, objs[i].Metadata.Name) + } + } +} From a211ea3d1eee5797028938ec195984b28df2d26f Mon Sep 17 00:00:00 2001 From: Michael Bridgen <michael@weave.works> Date: Mon, 4 Jun 2018 13:11:50 +0100 Subject: [PATCH 3/3] Use ~/.kube/config to set default namespace `kubectl` run from the pod uses the service account token, and inherits its default namespace from the service account. We want it to be the default default (named "default"), to mimic the behaviour of `kubectl` used from a terminal and thereby avoid some surprises. To reset the default namespace, we copy a Kubernetes config file with a context specifying the "default" namespace, to the place `kubectl` expects to find it, in the flux image. --- Makefile | 2 +- docker/Dockerfile.flux | 1 + docker/kubeconfig | 12 ++++++++++++ 3 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 docker/kubeconfig diff --git a/Makefile b/Makefile index ae2b9aa07..80138f264 100644 --- a/Makefile +++ b/Makefile @@ -48,7 +48,7 @@ build/.%.done: docker/Dockerfile.% -f build/docker/$*/Dockerfile.$* ./build/docker/$* touch $@ -build/.flux.done: build/fluxd build/kubectl docker/ssh_config +build/.flux.done: build/fluxd build/kubectl docker/ssh_config docker/kubeconfig build/.helm-operator.done: build/helm-operator build/kubectl docker/ssh_config build/fluxd: $(FLUXD_DEPS) diff --git a/docker/Dockerfile.flux b/docker/Dockerfile.flux index 98f449bd4..ae397bb1a 100644 --- a/docker/Dockerfile.flux +++ b/docker/Dockerfile.flux @@ -31,6 +31,7 @@ RUN mkdir ~/.ssh && touch ~/.ssh/known_hosts && \ COPY ./ssh_config /root/.ssh/config RUN chmod 600 /root/.ssh/config +COPY ./kubeconfig /root/.kube/config COPY ./kubectl /usr/local/bin/ COPY ./fluxd /usr/local/bin/ diff --git a/docker/kubeconfig b/docker/kubeconfig new file mode 100644 index 000000000..3911d1ab6 --- /dev/null +++ b/docker/kubeconfig @@ -0,0 +1,12 @@ +apiVersion: v1 +clusters: [] +contexts: +- context: + cluster: "" + namespace: default + user: "" + name: default +current-context: default +kind: Config +preferences: {} +users: []