From 80156474cf46b919b5d4b51a4676dc395c3e90dc Mon Sep 17 00:00:00 2001 From: Henrik Schmidt Date: Tue, 6 Jun 2017 13:47:40 +0200 Subject: [PATCH] Always check if default labels on node need to be updated in kubelet --- pkg/kubelet/BUILD | 1 + pkg/kubelet/kubelet_node_status.go | 28 ++++ pkg/kubelet/kubelet_node_status_test.go | 181 +++++++++++++++++++++++- 3 files changed, 207 insertions(+), 3 deletions(-) diff --git a/pkg/kubelet/BUILD b/pkg/kubelet/BUILD index 1f0aa84dcee..e36012a8e37 100644 --- a/pkg/kubelet/BUILD +++ b/pkg/kubelet/BUILD @@ -170,6 +170,7 @@ go_test( "//pkg/api:go_default_library", "//pkg/api/install:go_default_library", "//pkg/capabilities:go_default_library", + "//pkg/kubelet/apis:go_default_library", "//pkg/kubelet/apis/kubeletconfig:go_default_library", "//pkg/kubelet/cadvisor/testing:go_default_library", "//pkg/kubelet/cm:go_default_library", diff --git a/pkg/kubelet/kubelet_node_status.go b/pkg/kubelet/kubelet_node_status.go index 9a20a75600a..fce9c3ae395 100644 --- a/pkg/kubelet/kubelet_node_status.go +++ b/pkg/kubelet/kubelet_node_status.go @@ -137,6 +137,7 @@ func (kl *Kubelet) tryRegisterWithAPIServer(node *v1.Node) bool { // the value of the controller-managed attach-detach // annotation. requiresUpdate := kl.reconcileCMADAnnotationWithExistingNode(node, existingNode) + requiresUpdate = kl.updateDefaultLabels(node, existingNode) || requiresUpdate if requiresUpdate { if _, err := nodeutil.PatchNodeStatus(kl.kubeClient, types.NodeName(kl.nodeName), originalNode, existingNode); err != nil { @@ -161,6 +162,33 @@ func (kl *Kubelet) tryRegisterWithAPIServer(node *v1.Node) bool { return false } +// updateDefaultLabels will set the default labels on the node +func (kl *Kubelet) updateDefaultLabels(initialNode, existingNode *v1.Node) bool { + defaultLabels := []string{ + kubeletapis.LabelHostname, + kubeletapis.LabelZoneFailureDomain, + kubeletapis.LabelZoneRegion, + kubeletapis.LabelInstanceType, + kubeletapis.LabelOS, + kubeletapis.LabelArch, + } + + var needsUpdate bool = false + //Set default labels but make sure to not set labels with empty values + for _, label := range defaultLabels { + if existingNode.Labels[label] != initialNode.Labels[label] { + existingNode.Labels[label] = initialNode.Labels[label] + needsUpdate = true + } + + if existingNode.Labels[label] == "" { + delete(existingNode.Labels, label) + } + } + + return needsUpdate +} + // reconcileCMADAnnotationWithExistingNode reconciles the controller-managed // attach-detach annotation on a new node and the existing node, returning // whether the existing node must be updated. diff --git a/pkg/kubelet/kubelet_node_status_test.go b/pkg/kubelet/kubelet_node_status_test.go index cd26a352cfe..ae005ea20eb 100644 --- a/pkg/kubelet/kubelet_node_status_test.go +++ b/pkg/kubelet/kubelet_node_status_test.go @@ -43,6 +43,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" + kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis" "k8s.io/kubernetes/pkg/kubelet/cm" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" "k8s.io/kubernetes/pkg/kubelet/util/sliceutils" @@ -675,8 +676,15 @@ func TestRegisterWithApiServer(t *testing.T) { kubeClient.AddReactor("get", "nodes", func(action core.Action) (bool, runtime.Object, error) { // Return an existing (matching) node on get. return true, &v1.Node{ - ObjectMeta: metav1.ObjectMeta{Name: testKubeletHostname}, - Spec: v1.NodeSpec{ExternalID: testKubeletHostname}, + ObjectMeta: metav1.ObjectMeta{ + Name: testKubeletHostname, + Labels: map[string]string{ + kubeletapis.LabelHostname: testKubeletHostname, + kubeletapis.LabelOS: goruntime.GOOS, + kubeletapis.LabelArch: goruntime.GOARCH, + }, + }, + Spec: v1.NodeSpec{ExternalID: testKubeletHostname}, }, nil }) kubeClient.AddReactor("*", "*", func(action core.Action) (bool, runtime.Object, error) { @@ -731,7 +739,13 @@ func TestTryRegisterWithApiServer(t *testing.T) { newNode := func(cmad bool, externalID string) *v1.Node { node := &v1.Node{ - ObjectMeta: metav1.ObjectMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + kubeletapis.LabelHostname: testKubeletHostname, + kubeletapis.LabelOS: goruntime.GOOS, + kubeletapis.LabelArch: goruntime.GOARCH, + }, + }, Spec: v1.NodeSpec{ ExternalID: externalID, }, @@ -961,3 +975,164 @@ func TestUpdateNewNodeStatusTooLargeReservation(t *testing.T) { assert.NoError(t, err) assert.True(t, apiequality.Semantic.DeepEqual(expectedNode.Status.Allocatable, updatedNode.Status.Allocatable), "%s", diff.ObjectDiff(expectedNode.Status.Allocatable, updatedNode.Status.Allocatable)) } + +func TestUpdateDefaultLabels(t *testing.T) { + testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) + + cases := []struct { + name string + initialNode *v1.Node + existingNode *v1.Node + needsUpdate bool + finalLabels map[string]string + }{ + { + name: "make sure default labels exist", + initialNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + kubeletapis.LabelHostname: "new-hostname", + kubeletapis.LabelZoneFailureDomain: "new-zone-failure-domain", + kubeletapis.LabelZoneRegion: "new-zone-region", + kubeletapis.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", + }, + }, + }, + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + }, + }, + needsUpdate: true, + finalLabels: map[string]string{ + kubeletapis.LabelHostname: "new-hostname", + kubeletapis.LabelZoneFailureDomain: "new-zone-failure-domain", + kubeletapis.LabelZoneRegion: "new-zone-region", + kubeletapis.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", + }, + }, + { + name: "make sure default labels are up to date", + initialNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + kubeletapis.LabelHostname: "new-hostname", + kubeletapis.LabelZoneFailureDomain: "new-zone-failure-domain", + kubeletapis.LabelZoneRegion: "new-zone-region", + kubeletapis.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", + }, + }, + }, + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + kubeletapis.LabelHostname: "old-hostname", + kubeletapis.LabelZoneFailureDomain: "old-zone-failure-domain", + kubeletapis.LabelZoneRegion: "old-zone-region", + kubeletapis.LabelInstanceType: "old-instance-type", + kubeletapis.LabelOS: "old-os", + kubeletapis.LabelArch: "old-arch", + }, + }, + }, + needsUpdate: true, + finalLabels: map[string]string{ + kubeletapis.LabelHostname: "new-hostname", + kubeletapis.LabelZoneFailureDomain: "new-zone-failure-domain", + kubeletapis.LabelZoneRegion: "new-zone-region", + kubeletapis.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", + }, + }, + { + name: "make sure existing labels do not get deleted", + initialNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + kubeletapis.LabelHostname: "new-hostname", + kubeletapis.LabelZoneFailureDomain: "new-zone-failure-domain", + kubeletapis.LabelZoneRegion: "new-zone-region", + kubeletapis.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", + }, + }, + }, + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + kubeletapis.LabelHostname: "new-hostname", + kubeletapis.LabelZoneFailureDomain: "new-zone-failure-domain", + kubeletapis.LabelZoneRegion: "new-zone-region", + kubeletapis.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", + "please-persist": "foo", + }, + }, + }, + needsUpdate: false, + finalLabels: map[string]string{ + kubeletapis.LabelHostname: "new-hostname", + kubeletapis.LabelZoneFailureDomain: "new-zone-failure-domain", + kubeletapis.LabelZoneRegion: "new-zone-region", + kubeletapis.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", + "please-persist": "foo", + }, + }, + { + name: "no update needed", + initialNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + kubeletapis.LabelHostname: "new-hostname", + kubeletapis.LabelZoneFailureDomain: "new-zone-failure-domain", + kubeletapis.LabelZoneRegion: "new-zone-region", + kubeletapis.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", + }, + }, + }, + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + kubeletapis.LabelHostname: "new-hostname", + kubeletapis.LabelZoneFailureDomain: "new-zone-failure-domain", + kubeletapis.LabelZoneRegion: "new-zone-region", + kubeletapis.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", + }, + }, + }, + needsUpdate: false, + finalLabels: map[string]string{ + kubeletapis.LabelHostname: "new-hostname", + kubeletapis.LabelZoneFailureDomain: "new-zone-failure-domain", + kubeletapis.LabelZoneRegion: "new-zone-region", + kubeletapis.LabelInstanceType: "new-instance-type", + kubeletapis.LabelOS: "new-os", + kubeletapis.LabelArch: "new-arch", + }, + }, + } + + for _, tc := range cases { + defer testKubelet.Cleanup() + kubelet := testKubelet.kubelet + + needsUpdate := kubelet.updateDefaultLabels(tc.initialNode, tc.existingNode) + assert.Equal(t, tc.needsUpdate, needsUpdate, tc.name) + assert.Equal(t, tc.finalLabels, tc.existingNode.Labels, tc.name) + } +}