diff --git a/pkg/kubelet/kubeletconfig/BUILD b/pkg/kubelet/kubeletconfig/BUILD index 03e386f8086..fd98a1cea86 100644 --- a/pkg/kubelet/kubeletconfig/BUILD +++ b/pkg/kubelet/kubeletconfig/BUILD @@ -1,9 +1,6 @@ package(default_visibility = ["//visibility:public"]) -load( - "@io_bazel_rules_go//go:def.bzl", - "go_library", -) +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", @@ -60,3 +57,15 @@ filegroup( ], tags = ["automanaged"], ) + +go_test( + name = "go_default_test", + srcs = ["controller_test.go"], + embed = [":go_default_library"], + deps = [ + "//pkg/kubelet/kubeletconfig/checkpoint:go_default_library", + "//pkg/kubelet/kubeletconfig/checkpoint/store:go_default_library", + "//pkg/kubelet/kubeletconfig/status:go_default_library", + "//staging/src/k8s.io/api/core/v1:go_default_library", + ], +) diff --git a/pkg/kubelet/kubeletconfig/checkpoint/store/fakestore.go b/pkg/kubelet/kubeletconfig/checkpoint/store/fakestore.go index 6e442cf2cc2..854294806b0 100644 --- a/pkg/kubelet/kubeletconfig/checkpoint/store/fakestore.go +++ b/pkg/kubelet/kubeletconfig/checkpoint/store/fakestore.go @@ -32,6 +32,11 @@ type fakeStore struct { var _ Store = (*fakeStore)(nil) +// NewFakeStore constructs a fake Store +func NewFakeStore() Store { + return &fakeStore{} +} + func (s *fakeStore) Initialize() error { return fmt.Errorf("Initialize method not supported") } diff --git a/pkg/kubelet/kubeletconfig/controller.go b/pkg/kubelet/kubeletconfig/controller.go index 0dcc36c7523..50b65a67a3a 100644 --- a/pkg/kubelet/kubeletconfig/controller.go +++ b/pkg/kubelet/kubeletconfig/controller.go @@ -309,7 +309,7 @@ func (cc *Controller) graduateAssignedToLastKnownGood() error { } // if the sources are equal, no need to change if assigned == lastKnownGood || - assigned != nil && lastKnownGood != nil && apiequality.Semantic.DeepEqual(assigned, lastKnownGood) { + assigned != nil && lastKnownGood != nil && apiequality.Semantic.DeepEqual(assigned.NodeConfigSource(), lastKnownGood.NodeConfigSource()) { return nil } // update last-known-good diff --git a/pkg/kubelet/kubeletconfig/controller_test.go b/pkg/kubelet/kubeletconfig/controller_test.go new file mode 100644 index 00000000000..037a5163d1d --- /dev/null +++ b/pkg/kubelet/kubeletconfig/controller_test.go @@ -0,0 +1,81 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kubeletconfig + +import ( + "testing" + + apiv1 "k8s.io/api/core/v1" + "k8s.io/kubernetes/pkg/kubelet/kubeletconfig/checkpoint" + "k8s.io/kubernetes/pkg/kubelet/kubeletconfig/checkpoint/store" + "k8s.io/kubernetes/pkg/kubelet/kubeletconfig/status" +) + +func TestGraduateAssignedToLastKnownGood(t *testing.T) { + realSource1, _, err := checkpoint.NewRemoteConfigSource(&apiv1.NodeConfigSource{ + ConfigMap: &apiv1.ConfigMapNodeConfigSource{ + Namespace: "foo", + Name: "1", + KubeletConfigKey: "kubelet", + }, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + realSource2, _, err := checkpoint.NewRemoteConfigSource(&apiv1.NodeConfigSource{ + ConfigMap: &apiv1.ConfigMapNodeConfigSource{ + Namespace: "foo", + Name: "2", + KubeletConfigKey: "kubelet", + }, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + cases := []struct { + name string + assigned checkpoint.RemoteConfigSource + lkg checkpoint.RemoteConfigSource + }{ + { + name: "nil lkg to non-nil lkg", + assigned: realSource1, + lkg: nil, + }, + { + name: "non-nil lkg to non-nil lkg", + assigned: realSource2, + lkg: realSource1, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + controller := &Controller{ + configStatus: status.NewNodeConfigStatus(), + checkpointStore: store.NewFakeStore(), + } + controller.checkpointStore.SetLastKnownGood(tc.lkg) + controller.checkpointStore.SetAssigned(tc.assigned) + if err := controller.graduateAssignedToLastKnownGood(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} diff --git a/test/e2e_node/dynamic_kubelet_config_test.go b/test/e2e_node/dynamic_kubelet_config_test.go index 60ad4956548..9071dd0913d 100644 --- a/test/e2e_node/dynamic_kubelet_config_test.go +++ b/test/e2e_node/dynamic_kubelet_config_test.go @@ -418,6 +418,70 @@ var _ = framework.KubeDescribe("[Feature:DynamicKubeletConfig][NodeFeature:Dynam }) }) + // previously, we missed a panic because we were not exercising this path + Context("update Node.Spec.ConfigSource: non-nil last-known-good to a new non-nil last-known-good", func() { + It(itDescription, func() { + var err error + // we base the "lkg" configmap off of the configuration from before the test + lkgKC := beforeKC.DeepCopy() + lkgConfigMap1 := newKubeletConfigMap("dynamic-kubelet-config-test-lkg-1", lkgKC) + lkgConfigMap1, err = f.ClientSet.CoreV1().ConfigMaps("kube-system").Create(lkgConfigMap1) + framework.ExpectNoError(err) + + lkgSource1 := &apiv1.NodeConfigSource{ConfigMap: &apiv1.ConfigMapNodeConfigSource{ + Namespace: lkgConfigMap1.Namespace, + Name: lkgConfigMap1.Name, + KubeletConfigKey: "kubelet", + }} + lkgStatus1 := lkgSource1.DeepCopy() + lkgStatus1.ConfigMap.UID = lkgConfigMap1.UID + lkgStatus1.ConfigMap.ResourceVersion = lkgConfigMap1.ResourceVersion + + lkgConfigMap2 := newKubeletConfigMap("dynamic-kubelet-config-test-lkg-2", lkgKC) + lkgConfigMap2, err = f.ClientSet.CoreV1().ConfigMaps("kube-system").Create(lkgConfigMap2) + framework.ExpectNoError(err) + + lkgSource2 := &apiv1.NodeConfigSource{ConfigMap: &apiv1.ConfigMapNodeConfigSource{ + Namespace: lkgConfigMap2.Namespace, + Name: lkgConfigMap2.Name, + KubeletConfigKey: "kubelet", + }} + lkgStatus2 := lkgSource2.DeepCopy() + lkgStatus2.ConfigMap.UID = lkgConfigMap2.UID + lkgStatus2.ConfigMap.ResourceVersion = lkgConfigMap2.ResourceVersion + + // cases + first := nodeConfigTestCase{ + desc: "last-known-good-1", + configSource: lkgSource1, + configMap: lkgConfigMap1, + expectConfigStatus: expectNodeConfigStatus{ + lastKnownGood: lkgStatus1, + }, + expectConfig: lkgKC, + event: true, + } + + second := nodeConfigTestCase{ + desc: "last-known-good-2", + configSource: lkgSource2, + configMap: lkgConfigMap2, + expectConfigStatus: expectNodeConfigStatus{ + lastKnownGood: lkgStatus2, + }, + expectConfig: lkgKC, + event: true, + } + + // Manually actuate this to ensure we wait for each case to become the last-known-good + const lkgDuration = 12 * time.Minute + By(fmt.Sprintf("setting initial state %q", first.desc)) + first.run(f, setConfigSourceFunc, true, lkgDuration) + By(fmt.Sprintf("from %q to %q", first.desc, second.desc)) + second.run(f, setConfigSourceFunc, true, lkgDuration) + }) + }) + // exposes resource leaks across config changes Context("update Node.Spec.ConfigSource: 100 update stress test:", func() { It(itDescription, func() {