From 808c9c62b3291f72ab50bdbca2b744a3e12d187a Mon Sep 17 00:00:00 2001 From: hxcGit Date: Fri, 26 Aug 2022 09:42:09 +0800 Subject: [PATCH 01/11] add auto pod upgrade controller for daemoset Signed-off-by: hxcGit --- .../app/controllermanager.go | 2 +- cmd/yurt-controller-manager/app/core.go | 14 + go.mod | 1 + .../podupgrade/pod_upgrade_controller.go | 340 +++++++++++++++++ .../podupgrade/pod_upgrade_controller_test.go | 343 ++++++++++++++++++ pkg/controller/podupgrade/util.go | 210 +++++++++++ pkg/controller/podupgrade/util_test.go | 45 +++ 7 files changed, 954 insertions(+), 1 deletion(-) create mode 100644 pkg/controller/podupgrade/pod_upgrade_controller.go create mode 100644 pkg/controller/podupgrade/pod_upgrade_controller_test.go create mode 100644 pkg/controller/podupgrade/util.go create mode 100644 pkg/controller/podupgrade/util_test.go diff --git a/cmd/yurt-controller-manager/app/controllermanager.go b/cmd/yurt-controller-manager/app/controllermanager.go index 0354dc0bf8c..b59fee63747 100644 --- a/cmd/yurt-controller-manager/app/controllermanager.go +++ b/cmd/yurt-controller-manager/app/controllermanager.go @@ -305,7 +305,7 @@ func NewControllerInitializers() map[string]InitFunc { controllers := map[string]InitFunc{} controllers["nodelifecycle"] = startNodeLifecycleController controllers["yurtcsrapprover"] = startYurtCSRApproverController - + controllers["podupgrade"] = startPodUpgradeController return controllers } diff --git a/cmd/yurt-controller-manager/app/core.go b/cmd/yurt-controller-manager/app/core.go index f84777b4d3c..6932783bcbe 100644 --- a/cmd/yurt-controller-manager/app/core.go +++ b/cmd/yurt-controller-manager/app/core.go @@ -27,6 +27,7 @@ import ( "github.com/openyurtio/openyurt/pkg/controller/certificates" lifecyclecontroller "github.com/openyurtio/openyurt/pkg/controller/nodelifecycle" + podupgradecontroller "github.com/openyurtio/openyurt/pkg/controller/podupgrade" ) func startNodeLifecycleController(ctx ControllerContext) (http.Handler, bool, error) { @@ -65,3 +66,16 @@ func startYurtCSRApproverController(ctx ControllerContext) (http.Handler, bool, return nil, true, nil } + +func startPodUpgradeController(ctx ControllerContext) (http.Handler, bool, error) { + podUpgradeCtrl := podupgradecontroller.NewController( + ctx.ClientBuilder.ClientOrDie("podUpgrade-controller"), + ctx.InformerFactory.Apps().V1().DaemonSets(), + ctx.InformerFactory.Core().V1().Nodes(), + ctx.InformerFactory.Core().V1().Pods(), + ) + + go podUpgradeCtrl.Run(2, ctx.Stop) + + return nil, true, nil +} diff --git a/go.mod b/go.mod index b7bd9e50103..4cc30637a49 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/Masterminds/semver/v3 v3.1.1 github.com/Microsoft/go-winio v0.4.15 github.com/aliyun/alibaba-cloud-sdk-go v1.61.579 + github.com/davecgh/go-spew v1.1.1 github.com/daviddengcn/go-colortext v1.0.0 github.com/dgrijalva/jwt-go v3.2.0+incompatible github.com/emicklei/go-restful v2.12.0+incompatible // indirect diff --git a/pkg/controller/podupgrade/pod_upgrade_controller.go b/pkg/controller/podupgrade/pod_upgrade_controller.go new file mode 100644 index 00000000000..a9bfe7f732d --- /dev/null +++ b/pkg/controller/podupgrade/pod_upgrade_controller.go @@ -0,0 +1,340 @@ +/* +Copyright 2022 The OpenYurt 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 podupgrade + +import ( + "context" + "fmt" + "reflect" + "time" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/wait" + appsinformers "k8s.io/client-go/informers/apps/v1" + coreinformers "k8s.io/client-go/informers/core/v1" + client "k8s.io/client-go/kubernetes" + appslisters "k8s.io/client-go/listers/apps/v1" + corelisters "k8s.io/client-go/listers/core/v1" + "k8s.io/client-go/tools/cache" + "k8s.io/client-go/util/workqueue" + "k8s.io/klog/v2" +) + +type Controller struct { + kubeclientset client.Interface + + daemonsetLister appslisters.DaemonSetLister + daemonsetSynced cache.InformerSynced + nodeLister corelisters.NodeLister + nodeSynced cache.InformerSynced + podLister corelisters.PodLister + podSynced cache.InformerSynced + + daemonsetWorkqueue workqueue.RateLimitingInterface + nodeWorkqueue workqueue.Interface +} + +func NewController(kc client.Interface, daemonsetInformer appsinformers.DaemonSetInformer, + nodeInformer coreinformers.NodeInformer, podInformer coreinformers.PodInformer) *Controller { + // TODO: Is eventBroadCaster needed? + ctrl := Controller{ + kubeclientset: kc, + daemonsetLister: daemonsetInformer.Lister(), + daemonsetSynced: daemonsetInformer.Informer().HasSynced, + + nodeLister: nodeInformer.Lister(), + nodeSynced: nodeInformer.Informer().HasSynced, + + podLister: podInformer.Lister(), + podSynced: podInformer.Informer().HasSynced, + + daemonsetWorkqueue: workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()), + nodeWorkqueue: workqueue.New(), + } + + daemonsetInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + UpdateFunc: func(old, new interface{}) { + newDS := new.(*appsv1.DaemonSet) + oldDS := old.(*appsv1.DaemonSet) + + // Only control daemonset with annotation "apps.openyurt.io/upgrade-strategy" + if !metav1.HasAnnotation(newDS.ObjectMeta, UpgradeAnnotation) || !metav1.HasAnnotation(oldDS.ObjectMeta, UpgradeAnnotation) { + return + } + + // TODO: change to compare generation and revision hash + if newDS.ResourceVersion == oldDS.ResourceVersion || reflect.DeepEqual(newDS.Spec.Template.Spec, oldDS.Spec.Template.Spec) { + return + } + + ctrl.enqueueDaemonset(new) + }, + }) + + nodeInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + UpdateFunc: func(old, new interface{}) { + oldNode := old.(*corev1.Node) + newNode := new.(*corev1.Node) + if !NodeReady(&oldNode.Status) && NodeReady(&newNode.Status) { + klog.Infof("node %q turn to ready", newNode.Name) + ctrl.nodeWorkqueue.Add(newNode.Name) + } + }, + }) + return &ctrl +} + +func (c *Controller) enqueueDaemonset(obj interface{}) { + var key string + var err error + if key, err = cache.MetaNamespaceKeyFunc(obj); err != nil { + utilruntime.HandleError(err) + return + } + + klog.Infof("Got udpate event: %v", key) + c.daemonsetWorkqueue.AddRateLimited(key) +} + +func (c *Controller) Run(threadiness int, stopCh <-chan struct{}) { + defer utilruntime.HandleCrash() + + klog.Info("Starting pod upgrade controller") + defer klog.Info("Shutting down pod upgrade controller") + defer c.daemonsetWorkqueue.ShutDown() + defer c.nodeWorkqueue.ShutDown() + + //synchronize the cache before starting to process events + if !cache.WaitForCacheSync(stopCh, c.daemonsetSynced, c.nodeSynced, + c.podSynced) { + klog.Error("sync podupgrade controller timeout") + } + + for i := 0; i < threadiness; i++ { + go wait.Until(c.runDaemonsetWorker, time.Second, stopCh) + } + + for i := 0; i < threadiness; i++ { + go wait.Until(c.runNodeWorker, time.Second, stopCh) + } + + <-stopCh +} + +func (c *Controller) runDaemonsetWorker() { + for { + obj, shutdown := c.daemonsetWorkqueue.Get() + if shutdown { + return + } + + if err := c.syncDaemonsetHandler(obj.(string)); err != nil { + utilruntime.HandleError(err) + } + c.daemonsetWorkqueue.Forget(obj) + c.daemonsetWorkqueue.Done(obj) + } +} + +func (c *Controller) runNodeWorker() { + for { + obj, shutdown := c.nodeWorkqueue.Get() + if shutdown { + return + } + + nodeName := obj.(string) + if err := c.syncNodeHandler(nodeName); err != nil { + utilruntime.HandleError(err) + } + + c.nodeWorkqueue.Done(nodeName) + } +} + +func (c *Controller) syncDaemonsetHandler(key string) error { + defer func() { + klog.V(4).Infof("Finish syncing pod upgrade request %q", key) + }() + + klog.V(4).Infof("Start handler pod upgrade request %q", key) + + namespace, name, err := cache.SplitMetaNamespaceKey(key) + if err != nil { + utilruntime.HandleError(fmt.Errorf("invalid resource key: %s", key)) + return nil + } + + // daemonset that need to be synced + ds, err := c.daemonsetLister.DaemonSets(namespace).Get(name) + if err != nil { + if apierrors.IsNotFound(err) { + return nil + } + return err + } + + pods, err := GetDaemonsetPods(c.podLister, ds) + if err != nil { + return err + } + + // recheck required annotation + v, ok := ds.Annotations[UpgradeAnnotation] + if !ok { + return fmt.Errorf("won't sync daemonset %q without annotation 'apps.openyurt.io/upgrade-strategy'", ds.Name) + } + + switch v { + case OTAUpgrade: + klog.Infof("OTA upgrade %v", ds.Name) + if err := c.checkOTAUpgrade(ds, pods); err != nil { + return err + } + + case AutoUpgrade: + klog.Infof("Auto upgrade %v", ds.Name) + if err := c.autoUpgrade(ds, pods); err != nil { + return err + } + default: + // error + return fmt.Errorf("unknown annotation type %v", v) + } + + return nil +} + +// checkOTAUpgrade compare every pod to its owner daemonset to check if pod is upgradable +// If pod is in line with the latest daemonset version, set annotation "apps.openyurt.io/pod-upgradable" to "true" +// while not, set annotation "apps.openyurt.io/pod-upgradable" to "false" +func (c *Controller) checkOTAUpgrade(ds *appsv1.DaemonSet, pods []*corev1.Pod) error { + for _, pod := range pods { + if err := SetPodUpgradeAnnotation(c.kubeclientset, ds, pod); err != nil { + return err + } + } + return nil +} + +// autoUpgrade perform pod upgrade operation when +// 1. pod is upgradable (using IsDaemonsetPodLatest to check) +// 2. pod node is ready +func (c *Controller) autoUpgrade(ds *appsv1.DaemonSet, pods []*corev1.Pod) error { + for _, pod := range pods { + latestOK, err := IsDaemonsetPodLatest(ds, pod) + if err != nil { + return err + } + + nodeOK, err := NodeReadyByName(c.kubeclientset, pod.Spec.NodeName) + if err != nil { + return err + } + + if !latestOK && nodeOK { + if err := c.kubeclientset.CoreV1().Pods(pod.Namespace). + Delete(context.TODO(), pod.Name, metav1.DeleteOptions{}); err != nil { + return err + } + klog.Infof("Auto upgrade pod %v/%v", ds.Name, pod.Name) + } + } + + return nil +} + +// syncNodeHandler delete the pod of daemonset that needs to be upgrade when node turns ready +func (c *Controller) syncNodeHandler(key string) error { + defer func() { + klog.V(4).Infof("Finish syncing pod upgrade request %q", key) + }() + + klog.V(4).Infof("Start handler pod upgrade request %q", key) + + node, err := c.nodeLister.Get(key) + if err != nil { + // If node not found, just ignore it. + if apierrors.IsNotFound(err) { + klog.V(5).Infof("Sync node %v not found", key) + return nil + } + return err + } + + // get all pod in this node + pods, err := GetNodePods(c.podLister, node) + if err != nil { + return err + } + + if err := c.upgradePodsWhenNodeReady(c.kubeclientset, pods); err != nil { + return err + } + + return nil +} + +// upgradePodsWhenNodeReady check pods in current node need to be upgraded +// Only workloads with annotation "apps.openyurt.io/upgrade-strategy" and updateStrategy "OnDelete" will be processed +func (c *Controller) upgradePodsWhenNodeReady(clientset client.Interface, pods []*corev1.Pod) error { + for _, pod := range pods { + owner := metav1.GetControllerOf(pod) + switch owner.Kind { + case DaemonSet: + ds, err := clientset.AppsV1().DaemonSets(pod.Namespace).Get(context.TODO(), owner.Name, metav1.GetOptions{}) + if err != nil { + return err + } + + // consider only the case with annotation "apps.openyurt.io/upgrade-strategy" + v, ok := ds.Annotations[UpgradeAnnotation] + // consider only the case with updateStrategy "OnDelete" + updateStrategyOK := (ds.Spec.UpdateStrategy.Type == appsv1.OnDeleteDaemonSetStrategyType) + if ok && updateStrategyOK { + switch v { + // set pod annotation when "apps.openyurt.io/upgrade-strategy=ota" + case OTAUpgrade: + if err := SetPodUpgradeAnnotation(clientset, ds, pod); err != nil { + return err + } + + // auto upgrade pod when "apps.openyurt.io/upgrade-strategy=auto" + case AutoUpgrade: + latestOK, err := IsDaemonsetPodLatest(ds, pod) + if err != nil { + return err + } + if !latestOK { + if err := clientset.CoreV1().Pods(pod.Namespace). + Delete(context.TODO(), pod.Name, metav1.DeleteOptions{}); err != nil { + return err + } + klog.Infof("Auto upgrade pod %v/%v", ds.Name, pod.Name) + } + } + } + default: + continue + } + } + return nil +} diff --git a/pkg/controller/podupgrade/pod_upgrade_controller_test.go b/pkg/controller/podupgrade/pod_upgrade_controller_test.go new file mode 100644 index 00000000000..59781f91e7d --- /dev/null +++ b/pkg/controller/podupgrade/pod_upgrade_controller_test.go @@ -0,0 +1,343 @@ +/* +Copyright 2022 The OpenYurt 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 podupgrade + +import ( + "context" + "reflect" + "testing" + + "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/uuid" + "k8s.io/apiserver/pkg/storage/names" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes/fake" + ktest "k8s.io/client-go/testing" + "k8s.io/client-go/tools/cache" +) + +var ( + simpleDaemonSetLabel = map[string]string{"foo": "bar"} + alwaysReady = func() bool { return true } +) + +var controllerKind = appsv1.SchemeGroupVersion.WithKind("DaemonSet") + +var ( + SyncDaemonset = "SyncDaemosnet" + SyncNode = "SyncNode" +) + +type fixture struct { + t *testing.T + client *fake.Clientset + podLister []*corev1.Pod + nodeLister []*corev1.Node + daemonsetLister []*appsv1.DaemonSet + actions []ktest.Action + objects []runtime.Object +} + +func newFixture(t *testing.T) *fixture { + return &fixture{ + t: t, + objects: []runtime.Object{}, + } +} + +func (f *fixture) newController() (*Controller, informers.SharedInformerFactory, informers.SharedInformerFactory) { + f.client = fake.NewSimpleClientset(f.objects...) + + dsInformer := informers.NewSharedInformerFactory(f.client, 0) + nodeInformer := informers.NewSharedInformerFactory(f.client, 0) + podInformer := informers.NewSharedInformerFactory(f.client, 0) + + for _, d := range f.daemonsetLister { + dsInformer.Apps().V1().DaemonSets().Informer().GetIndexer().Add(d) + } + for _, n := range f.nodeLister { + nodeInformer.Core().V1().Nodes().Informer().GetIndexer().Add(n) + } + for _, p := range f.podLister { + podInformer.Core().V1().Pods().Informer().GetIndexer().Add(p) + } + + c := NewController(f.client, dsInformer.Apps().V1().DaemonSets(), nodeInformer.Core().V1().Nodes(), + podInformer.Core().V1().Pods()) + + c.daemonsetSynced = alwaysReady + c.nodeSynced = alwaysReady + + return c, dsInformer, nodeInformer +} + +// run execute the controller logic +// kind=SyncDaemonset test daemonset update event +// kind=SyncNode test node ready event +func (f *fixture) run(key string, kind string) { + f.testController(key, kind, false) +} + +func (f *fixture) runExpectError(key string, kind string) { + f.testController(key, kind, true) +} + +func (f *fixture) testController(key string, kind string, expectError bool) { + c, dsInformer, nodeInformer := f.newController() + + stopCh := make(chan struct{}) + defer close(stopCh) + dsInformer.Start(stopCh) + nodeInformer.Start(stopCh) + + var err error + switch kind { + case SyncDaemonset: + err = c.syncDaemonsetHandler(key) + + case SyncNode: + err = c.syncNodeHandler(key) + } + + if !expectError && err != nil { + f.t.Errorf("error syncing: %v", err) + } else if expectError && err == nil { + f.t.Error("expected error syncing, got nil") + + } + + // make sure all the expected action occurred during controller sync process + for _, action := range f.actions { + findAndCheckAction(action, f.client.Actions(), f.t) + } +} + +// findAndCheckAction search all the given actions to find whether the expected action exists +func findAndCheckAction(expected ktest.Action, actions []ktest.Action, t *testing.T) { + for _, action := range actions { + if checkAction(expected, action) { + t.Logf("Check action %+v success", expected) + return + } + } + t.Errorf("Expected action %+v does not occur", expected) +} + +// checkAction verifies that expected and actual actions are equal +func checkAction(expected, actual ktest.Action) bool { + if !(expected.Matches(actual.GetVerb(), actual.GetResource().Resource)) || + reflect.TypeOf(actual) != reflect.TypeOf(expected) || + actual.GetSubresource() != expected.GetSubresource() { + return false + } + + switch a := actual.(type) { + case ktest.DeleteAction: + e, _ := expected.(ktest.DeleteActionImpl) + expName := e.GetName() + actualName := a.GetName() + if expName != actualName { + return false + } + } + + return true +} + +func (f *fixture) expectDeletePodAction(p *corev1.Pod) { + action := ktest.NewDeleteAction(schema.GroupVersionResource{Resource: "pods"}, p.Namespace, p.Name) + f.actions = append(f.actions, action) +} + +func newDaemonSet(name string, img string) *appsv1.DaemonSet { + two := int32(2) + return &appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{ + UID: uuid.NewUUID(), + Name: name, + Namespace: metav1.NamespaceDefault, + }, + Spec: appsv1.DaemonSetSpec{ + RevisionHistoryLimit: &two, + UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ + Type: appsv1.OnDeleteDaemonSetStrategyType, + }, + Selector: &metav1.LabelSelector{MatchLabels: simpleDaemonSetLabel}, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: simpleDaemonSetLabel, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: img}}, + }, + }, + }, + } +} + +func newPod(podName string, nodeName string, label map[string]string, ds *appsv1.DaemonSet) *corev1.Pod { + // Add hash unique label to the pod + newLabels := label + var podSpec corev1.PodSpec + // Copy pod spec from DaemonSet template, or use a default one if DaemonSet is nil + if ds != nil { + hash := ComputeHash(&ds.Spec.Template, ds.Status.CollisionCount) + newLabels = CloneAndAddLabel(label, appsv1.DefaultDaemonSetUniqueLabelKey, hash) + podSpec = ds.Spec.Template.Spec + } else { + podSpec = corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "foo/bar", + TerminationMessagePath: corev1.TerminationMessagePathDefault, + ImagePullPolicy: corev1.PullIfNotPresent, + }, + }, + } + } + + // Add node name to the pod + if len(nodeName) > 0 { + podSpec.NodeName = nodeName + } + + pod := &corev1.Pod{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + GenerateName: podName, + Labels: newLabels, + Namespace: metav1.NamespaceDefault, + }, + Spec: podSpec, + } + pod.Name = names.SimpleNameGenerator.GenerateName(podName) + if ds != nil { + pod.OwnerReferences = []metav1.OwnerReference{*metav1.NewControllerRef(ds, controllerKind)} + } + return pod +} + +func newNode(name string) *corev1.Node { + return &corev1.Node{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: metav1.NamespaceNone, + }, + Status: corev1.NodeStatus{ + Conditions: []corev1.NodeCondition{ + {Type: corev1.NodeReady, Status: corev1.ConditionTrue}, + }, + Allocatable: corev1.ResourceList{ + corev1.ResourcePods: resource.MustParse("100"), + }, + }, + } +} + +func setAutoUpgradeAnnotation(ds *appsv1.DaemonSet, annV string) { + metav1.SetMetaDataAnnotation(&ds.ObjectMeta, UpgradeAnnotation, annV) +} + +func getDaemonsetKey(ds *appsv1.DaemonSet, t *testing.T) string { + key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(ds) + if err != nil { + t.Errorf("Unexpected error getting key for foo %v: %v", ds.Name, err) + return "" + } + return key +} + +func TestAutoUpgradeWithDaemonsetUpdate(t *testing.T) { + f := newFixture(t) + ds := newDaemonSet("test-ds", "foo/bar:v1") + setAutoUpgradeAnnotation(ds, "auto") + node := newNode("test-node") + pod := newPod("test-pod", "test-node", simpleDaemonSetLabel, ds) + + ds.Spec.Template.Spec.Containers[0].Image = "foo/bar:v2" + + f.daemonsetLister = append(f.daemonsetLister, ds) + f.podLister = append(f.podLister, pod) + + f.objects = append(f.objects, ds, pod, node) + + f.expectDeletePodAction(pod) + + f.run(getDaemonsetKey(ds, t), SyncDaemonset) +} + +func TestAutoUpgradeWithNodeReady(t *testing.T) { + f := newFixture(t) + ds := newDaemonSet("test-ds", "foo/bar") + setAutoUpgradeAnnotation(ds, "auto") + node := newNode("test-node") + pod := newPod("test-pod", "test-node", simpleDaemonSetLabel, ds) + + ds.Spec.Template.Spec.Containers[0].Image = "foo/bar:v2" + + f.daemonsetLister = append(f.daemonsetLister, ds) + f.nodeLister = append(f.nodeLister, node) + f.podLister = append(f.podLister, pod) + + f.objects = append(f.objects, ds, pod, node) + + f.expectDeletePodAction(pod) + + f.run(node.Name, SyncNode) +} + +func TestOTAUpgrade(t *testing.T) { + f := newFixture(t) + ds := newDaemonSet("test-ds", "foo/bar:v1") + setAutoUpgradeAnnotation(ds, "ota") + oldPod := newPod("old-pod", "test-node", simpleDaemonSetLabel, ds) + + ds.Spec.Template.Spec.Containers[0].Image = "foo/bar:v2" + newPod := newPod("new-pod", "test-node", simpleDaemonSetLabel, ds) + + f.daemonsetLister = append(f.daemonsetLister, ds) + f.podLister = append(f.podLister, oldPod, newPod) + f.objects = append(f.objects, ds, oldPod, newPod) + + f.run(getDaemonsetKey(ds, t), SyncDaemonset) + + // check whether ota upgradable annotation set properly + oldPodGot, err := f.client.CoreV1().Pods(ds.Namespace).Get(context.TODO(), oldPod.Name, metav1.GetOptions{}) + if err != nil { + t.Errorf("get oldPod failed, %+v", err) + } + + newPodGot, err := f.client.CoreV1().Pods(ds.Namespace).Get(context.TODO(), newPod.Name, metav1.GetOptions{}) + if err != nil { + t.Errorf("get newPod failed, %+v", err) + } + + annOldPodGot, oldPodOK := oldPodGot.Annotations[PodUpgradableAnnotation] + assert.Equal(t, true, oldPodOK) + assert.Equal(t, "true", annOldPodGot) + + annNewPodGot, newPodOK := newPodGot.Annotations[PodUpgradableAnnotation] + assert.Equal(t, true, newPodOK) + assert.Equal(t, "false", annNewPodGot) +} diff --git a/pkg/controller/podupgrade/util.go b/pkg/controller/podupgrade/util.go new file mode 100644 index 00000000000..c9f2516f5ed --- /dev/null +++ b/pkg/controller/podupgrade/util.go @@ -0,0 +1,210 @@ +/* +Copyright 2022 The OpenYurt 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 podupgrade + +import ( + "context" + "encoding/binary" + "fmt" + "hash" + "hash/fnv" + "strconv" + + "github.com/davecgh/go-spew/spew" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + extensions "k8s.io/api/extensions/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/rand" + client "k8s.io/client-go/kubernetes" + corelisters "k8s.io/client-go/listers/core/v1" + "k8s.io/klog/v2" +) + +const ( + UpgradeAnnotation = "apps.openyurt.io/upgrade-strategy" + PodUpgradableAnnotation = "apps.openyurt.io/pod-upgradable" + + OTAUpgrade = "ota" + AutoUpgrade = "auto" + + DaemonSet = "DaemonSet" +) + +// GetDaemonsetPods get all pods belong to the given daemonset +func GetDaemonsetPods(podLister corelisters.PodLister, ds *appsv1.DaemonSet) ([]*corev1.Pod, error) { + dsPods := make([]*corev1.Pod, 0) + dsPodsNames := make([]string, 0) + pods, err := podLister.Pods(ds.Namespace).List(labels.Everything()) + if err != nil { + return nil, err + } + + for i, pod := range pods { + owner := metav1.GetControllerOf(pod) + if owner.UID == ds.UID { + dsPods = append(dsPods, pods[i]) + dsPodsNames = append(dsPodsNames, pod.Name) + } + } + + if len(dsPods) > 0 { + klog.V(4).Infof("Daemonset %v has pods %v", ds.Name, dsPodsNames) + } + return dsPods, nil +} + +// GetDaemonsetPods get all pods belong to the given daemonset +func GetNodePods(podLister corelisters.PodLister, node *corev1.Node) ([]*corev1.Pod, error) { + nodePods := make([]*corev1.Pod, 0) + nodePodsNames := make([]string, 0) + + pods, err := podLister.List(labels.Everything()) + if err != nil { + return nil, err + } + + for i, pod := range pods { + if pod.Spec.NodeName == node.Name { + nodePods = append(nodePods, pods[i]) + nodePodsNames = append(nodePodsNames, pod.Name) + } + } + + if len(nodePodsNames) > 0 { + klog.V(5).Infof("Daemonset %v has pods %v", node.Name, nodePodsNames) + } + return nodePods, nil +} + +// IsDaemonsetPodLatest check whether pod is latest by comparing its Spec with daemonset's +// If pod is latest, return true, otherwise return false +func IsDaemonsetPodLatest(ds *appsv1.DaemonSet, pod *corev1.Pod) (bool, error) { + hash := ComputeHash(&ds.Spec.Template, ds.Status.CollisionCount) + klog.V(4).Infof("compute hash: %v", hash) + generation, err := GetTemplateGeneration(ds) + if err != nil { + return false, err + } + + klog.V(5).Infof("daemonset %v revision hash is %v", ds.Name, hash) + klog.V(5).Infof("daemonset %v generation is %v", ds.Name, generation) + + templateMatches := generation != nil && pod.Labels[extensions.DaemonSetTemplateGenerationKey] == fmt.Sprint(generation) + hashMatches := len(hash) > 0 && pod.Labels[extensions.DefaultDaemonSetUniqueLabelKey] == hash + return hashMatches || templateMatches, nil +} + +// GetTemplateGeneration get annotation "deprecated.daemonset.template.generation" of the given daemonset +func GetTemplateGeneration(ds *appsv1.DaemonSet) (*int64, error) { + annotation, found := ds.Annotations[appsv1.DeprecatedTemplateGeneration] + if !found { + return nil, nil + } + generation, err := strconv.ParseInt(annotation, 10, 64) + if err != nil { + return nil, err + } + return &generation, nil +} + +func NodeReadyByName(client client.Interface, nodeName string) (bool, error) { + node, err := client.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{}) + if err != nil { + return false, err + } + + return NodeReady(&node.Status), nil +} + +// NodeReady check if the given node is ready +func NodeReady(nodeStatus *corev1.NodeStatus) bool { + for _, cond := range nodeStatus.Conditions { + if cond.Type == corev1.NodeReady { + return cond.Status == corev1.ConditionTrue + } + } + return false +} + +func SetPodUpgradeAnnotation(clientset client.Interface, ds *appsv1.DaemonSet, pod *corev1.Pod) error { + ok, err := IsDaemonsetPodLatest(ds, pod) + if err != nil { + return err + } + + var res bool + if !ok { + res = true + } + + metav1.SetMetaDataAnnotation(&pod.ObjectMeta, PodUpgradableAnnotation, strconv.FormatBool(res)) + if _, err := clientset.CoreV1().Pods(pod.Namespace).Update(context.TODO(), pod, metav1.UpdateOptions{}); err != nil { + return err + } + klog.Infof("set pod %q annotation apps.openyurt.io/pod-upgradable to %v", pod.Name, res) + + return nil +} + +// ComputeHash returns a hash value calculated from pod template and +// a collisionCount to avoid hash collision. The hash will be safe encoded to +// avoid bad words. +func ComputeHash(template *corev1.PodTemplateSpec, collisionCount *int32) string { + podTemplateSpecHasher := fnv.New32a() + DeepHashObject(podTemplateSpecHasher, *template) + + // Add collisionCount in the hash if it exists. + if collisionCount != nil { + collisionCountBytes := make([]byte, 8) + binary.LittleEndian.PutUint32(collisionCountBytes, uint32(*collisionCount)) + podTemplateSpecHasher.Write(collisionCountBytes) + } + + return rand.SafeEncodeString(fmt.Sprint(podTemplateSpecHasher.Sum32())) +} + +// DeepHashObject writes specified object to hash using the spew library +// which follows pointers and prints actual values of the nested objects +// ensuring the hash does not change when a pointer changes. +func DeepHashObject(hasher hash.Hash, objectToWrite interface{}) { + hasher.Reset() + printer := spew.ConfigState{ + Indent: " ", + SortKeys: true, + DisableMethods: true, + SpewKeys: true, + } + printer.Fprintf(hasher, "%#v", objectToWrite) +} + +// Clones the given map and returns a new map with the given key and value added. +// Returns the given map, if labelKey is empty. +func CloneAndAddLabel(labels map[string]string, labelKey, labelValue string) map[string]string { + if labelKey == "" { + // Don't need to add a label. + return labels + } + // Clone. + newLabels := map[string]string{} + for key, value := range labels { + newLabels[key] = value + } + newLabels[labelKey] = labelValue + return newLabels +} diff --git a/pkg/controller/podupgrade/util_test.go b/pkg/controller/podupgrade/util_test.go new file mode 100644 index 00000000000..2baace5cd16 --- /dev/null +++ b/pkg/controller/podupgrade/util_test.go @@ -0,0 +1,45 @@ +/* +Copyright 2022 The OpenYurt 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 podupgrade + +import ( + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes/fake" +) + +func TestGetNodePods(t *testing.T) { + // Note: fake client does not support filtering by field selector, just ignore + node := newNode("node1") + pod1 := newPod("test-pod1", "node1", simpleDaemonSetLabel, nil) + pod2 := newPod("test-pod2", "node2", simpleDaemonSetLabel, nil) + + expectPods := []*corev1.Pod{pod1} + clientset := fake.NewSimpleClientset(node, pod1, pod2) + podInformer := informers.NewSharedInformerFactory(clientset, 0) + + podInformer.Core().V1().Pods().Informer().GetIndexer().Add(pod1) + podInformer.Core().V1().Pods().Informer().GetIndexer().Add(pod2) + + gotPods, err := GetNodePods(podInformer.Core().V1().Pods().Lister(), node) + + assert.Equal(t, nil, err) + assert.Equal(t, expectPods, gotPods) +} From d740058fb611e823889cc39ef49995f9f797ac5a Mon Sep 17 00:00:00 2001 From: hxcGit Date: Tue, 30 Aug 2022 20:08:59 +0800 Subject: [PATCH 02/11] add unit tests for util Signed-off-by: hxcGit --- .../podupgrade/pod_upgrade_controller.go | 2 +- .../podupgrade/pod_upgrade_controller_test.go | 25 ++++----- pkg/controller/podupgrade/util.go | 24 ++++++--- pkg/controller/podupgrade/util_test.go | 53 ++++++++++++++++++- 4 files changed, 81 insertions(+), 23 deletions(-) diff --git a/pkg/controller/podupgrade/pod_upgrade_controller.go b/pkg/controller/podupgrade/pod_upgrade_controller.go index a9bfe7f732d..7a3714b931b 100644 --- a/pkg/controller/podupgrade/pod_upgrade_controller.go +++ b/pkg/controller/podupgrade/pod_upgrade_controller.go @@ -245,7 +245,7 @@ func (c *Controller) autoUpgrade(ds *appsv1.DaemonSet, pods []*corev1.Pod) error return err } - nodeOK, err := NodeReadyByName(c.kubeclientset, pod.Spec.NodeName) + nodeOK, err := NodeReadyByName(c.nodeLister, pod.Spec.NodeName) if err != nil { return err } diff --git a/pkg/controller/podupgrade/pod_upgrade_controller_test.go b/pkg/controller/podupgrade/pod_upgrade_controller_test.go index 59781f91e7d..01d9a18dac9 100644 --- a/pkg/controller/podupgrade/pod_upgrade_controller_test.go +++ b/pkg/controller/podupgrade/pod_upgrade_controller_test.go @@ -65,30 +65,27 @@ func newFixture(t *testing.T) *fixture { } } -func (f *fixture) newController() (*Controller, informers.SharedInformerFactory, informers.SharedInformerFactory) { +func (f *fixture) newController() (*Controller, informers.SharedInformerFactory) { f.client = fake.NewSimpleClientset(f.objects...) - dsInformer := informers.NewSharedInformerFactory(f.client, 0) - nodeInformer := informers.NewSharedInformerFactory(f.client, 0) - podInformer := informers.NewSharedInformerFactory(f.client, 0) - + sharedInformers := informers.NewSharedInformerFactory(f.client, 0) for _, d := range f.daemonsetLister { - dsInformer.Apps().V1().DaemonSets().Informer().GetIndexer().Add(d) + sharedInformers.Apps().V1().DaemonSets().Informer().GetIndexer().Add(d) } for _, n := range f.nodeLister { - nodeInformer.Core().V1().Nodes().Informer().GetIndexer().Add(n) + sharedInformers.Core().V1().Nodes().Informer().GetIndexer().Add(n) } for _, p := range f.podLister { - podInformer.Core().V1().Pods().Informer().GetIndexer().Add(p) + sharedInformers.Core().V1().Pods().Informer().GetIndexer().Add(p) } - c := NewController(f.client, dsInformer.Apps().V1().DaemonSets(), nodeInformer.Core().V1().Nodes(), - podInformer.Core().V1().Pods()) + c := NewController(f.client, sharedInformers.Apps().V1().DaemonSets(), sharedInformers.Core().V1().Nodes(), + sharedInformers.Core().V1().Pods()) c.daemonsetSynced = alwaysReady c.nodeSynced = alwaysReady - return c, dsInformer, nodeInformer + return c, sharedInformers } // run execute the controller logic @@ -103,12 +100,11 @@ func (f *fixture) runExpectError(key string, kind string) { } func (f *fixture) testController(key string, kind string, expectError bool) { - c, dsInformer, nodeInformer := f.newController() + c, sharedInformers := f.newController() stopCh := make(chan struct{}) defer close(stopCh) - dsInformer.Start(stopCh) - nodeInformer.Start(stopCh) + sharedInformers.Start(stopCh) var err error switch kind { @@ -278,6 +274,7 @@ func TestAutoUpgradeWithDaemonsetUpdate(t *testing.T) { ds.Spec.Template.Spec.Containers[0].Image = "foo/bar:v2" f.daemonsetLister = append(f.daemonsetLister, ds) + f.nodeLister = append(f.nodeLister, node) f.podLister = append(f.podLister, pod) f.objects = append(f.objects, ds, pod, node) diff --git a/pkg/controller/podupgrade/util.go b/pkg/controller/podupgrade/util.go index c9f2516f5ed..0efe749ddb9 100644 --- a/pkg/controller/podupgrade/util.go +++ b/pkg/controller/podupgrade/util.go @@ -36,16 +36,22 @@ import ( "k8s.io/klog/v2" ) +var DaemonSet = "DaemonSet" + const ( - UpgradeAnnotation = "apps.openyurt.io/upgrade-strategy" - PodUpgradableAnnotation = "apps.openyurt.io/pod-upgradable" + // UpgradeAnnotation is the annotation key used in daemonset spec to indicate + // which upgrade strategy is selected. Currently "ota" and "auto" are supported. + UpgradeAnnotation = "apps.openyurt.io/upgrade-strategy" OTAUpgrade = "ota" AutoUpgrade = "auto" - - DaemonSet = "DaemonSet" ) +// PodUpgradableAnnotation is the annotation key added to pods to indicate +// whether a new version is available for upgrade. +// This annotation will only be added if the upgrade strategy is "apps.openyurt.io/upgrade-strategy":"ota". +const PodUpgradableAnnotation = "apps.openyurt.io/pod-upgradable" + // GetDaemonsetPods get all pods belong to the given daemonset func GetDaemonsetPods(podLister corelisters.PodLister, ds *appsv1.DaemonSet) ([]*corev1.Pod, error) { dsPods := make([]*corev1.Pod, 0) @@ -57,6 +63,9 @@ func GetDaemonsetPods(podLister corelisters.PodLister, ds *appsv1.DaemonSet) ([] for i, pod := range pods { owner := metav1.GetControllerOf(pod) + if owner == nil { + continue + } if owner.UID == ds.UID { dsPods = append(dsPods, pods[i]) dsPodsNames = append(dsPodsNames, pod.Name) @@ -123,8 +132,9 @@ func GetTemplateGeneration(ds *appsv1.DaemonSet) (*int64, error) { return &generation, nil } -func NodeReadyByName(client client.Interface, nodeName string) (bool, error) { - node, err := client.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{}) +// NodeReadyByName check if the given node is ready +func NodeReadyByName(nodeList corelisters.NodeLister, nodeName string) (bool, error) { + node, err := nodeList.Get(nodeName) if err != nil { return false, err } @@ -132,7 +142,7 @@ func NodeReadyByName(client client.Interface, nodeName string) (bool, error) { return NodeReady(&node.Status), nil } -// NodeReady check if the given node is ready +// NodeReady check if the given node status is ready func NodeReady(nodeStatus *corev1.NodeStatus) bool { for _, cond := range nodeStatus.Conditions { if cond.Type == corev1.NodeReady { diff --git a/pkg/controller/podupgrade/util_test.go b/pkg/controller/podupgrade/util_test.go index 2baace5cd16..3a474adc486 100644 --- a/pkg/controller/podupgrade/util_test.go +++ b/pkg/controller/podupgrade/util_test.go @@ -20,13 +20,13 @@ import ( "testing" "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" ) func TestGetNodePods(t *testing.T) { - // Note: fake client does not support filtering by field selector, just ignore node := newNode("node1") pod1 := newPod("test-pod1", "node1", simpleDaemonSetLabel, nil) pod2 := newPod("test-pod2", "node2", simpleDaemonSetLabel, nil) @@ -43,3 +43,54 @@ func TestGetNodePods(t *testing.T) { assert.Equal(t, nil, err) assert.Equal(t, expectPods, gotPods) } + +func TestGetDaemonsetPods(t *testing.T) { + ds1 := newDaemonSet("daemosnet1", "foo/bar:v1") + + pod1 := newPod("pod1", "", simpleDaemonSetLabel, ds1) + pod2 := newPod("pod2", "", simpleDaemonSetLabel, nil) + + expectPods := []*corev1.Pod{pod1} + clientset := fake.NewSimpleClientset(ds1, pod1, pod2) + podInformer := informers.NewSharedInformerFactory(clientset, 0) + + podInformer.Core().V1().Pods().Informer().GetIndexer().Add(pod1) + podInformer.Core().V1().Pods().Informer().GetIndexer().Add(pod2) + + gotPods, err := GetDaemonsetPods(podInformer.Core().V1().Pods().Lister(), ds1) + + assert.Equal(t, nil, err) + assert.Equal(t, expectPods, gotPods) +} + +func TestIsDaemonsetPodLatest(t *testing.T) { + daemosnetV1 := newDaemonSet("daemonset", "foo/bar:v1") + daemosnetV2 := daemosnetV1.DeepCopy() + daemosnetV2.Spec.Template.Spec.Containers[0].Image = "foo/bar:v2" + + tests := []struct { + name string + ds *appsv1.DaemonSet + pod *corev1.Pod + wantLatest bool + }{ + { + name: "latest", + ds: daemosnetV1, + pod: newPod("pod", "", simpleDaemonSetLabel, daemosnetV1), + wantLatest: true, + }, + { + name: "not latest", + ds: daemosnetV2, + pod: newPod("pod", "", simpleDaemonSetLabel, daemosnetV1), + wantLatest: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotLatest, _ := IsDaemonsetPodLatest(tt.ds, tt.pod) + assert.Equal(t, tt.wantLatest, gotLatest) + }) + } +} From 855e47fbe057ba4bbcb147792af7508efd83f332 Mon Sep 17 00:00:00 2001 From: hxcGit Date: Fri, 2 Sep 2022 17:45:58 +0800 Subject: [PATCH 03/11] remove node workqueue Signed-off-by: hxcGit --- .../podupgrade/pod_upgrade_controller.go | 177 ++++++------------ .../podupgrade/pod_upgrade_controller_test.go | 77 ++++---- pkg/controller/podupgrade/util.go | 48 ++--- pkg/controller/podupgrade/util_test.go | 92 +++++++++ .../kubernetes/controller/controller_utils.go | 58 ++++++ 5 files changed, 256 insertions(+), 196 deletions(-) create mode 100644 pkg/util/kubernetes/controller/controller_utils.go diff --git a/pkg/controller/podupgrade/pod_upgrade_controller.go b/pkg/controller/podupgrade/pod_upgrade_controller.go index 7a3714b931b..4fb84e57508 100644 --- a/pkg/controller/podupgrade/pod_upgrade_controller.go +++ b/pkg/controller/podupgrade/pod_upgrade_controller.go @@ -19,7 +19,6 @@ package podupgrade import ( "context" "fmt" - "reflect" "time" appsv1 "k8s.io/api/apps/v1" @@ -36,6 +35,8 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" + + k8sutil "github.com/openyurtio/openyurt/pkg/util/kubernetes/controller" ) type Controller struct { @@ -48,13 +49,12 @@ type Controller struct { podLister corelisters.PodLister podSynced cache.InformerSynced - daemonsetWorkqueue workqueue.RateLimitingInterface - nodeWorkqueue workqueue.Interface + daemonsetWorkqueue workqueue.Interface } func NewController(kc client.Interface, daemonsetInformer appsinformers.DaemonSetInformer, nodeInformer coreinformers.NodeInformer, podInformer coreinformers.PodInformer) *Controller { - // TODO: Is eventBroadCaster needed? + ctrl := Controller{ kubeclientset: kc, daemonsetLister: daemonsetInformer.Lister(), @@ -66,8 +66,7 @@ func NewController(kc client.Interface, daemonsetInformer appsinformers.DaemonSe podLister: podInformer.Lister(), podSynced: podInformer.Informer().HasSynced, - daemonsetWorkqueue: workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()), - nodeWorkqueue: workqueue.New(), + daemonsetWorkqueue: workqueue.New(), } daemonsetInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ @@ -75,17 +74,27 @@ func NewController(kc client.Interface, daemonsetInformer appsinformers.DaemonSe newDS := new.(*appsv1.DaemonSet) oldDS := old.(*appsv1.DaemonSet) - // Only control daemonset with annotation "apps.openyurt.io/upgrade-strategy" - if !metav1.HasAnnotation(newDS.ObjectMeta, UpgradeAnnotation) || !metav1.HasAnnotation(oldDS.ObjectMeta, UpgradeAnnotation) { + // Only control daemonset meets prerequisites + if !checkPrerequisites(newDS) { + return + } + + // Only control daemonset with modification + if newDS.ResourceVersion == oldDS.ResourceVersion || + (k8sutil.ComputeHash(&newDS.Spec.Template, newDS.Status.CollisionCount) == + k8sutil.ComputeHash(&oldDS.Spec.Template, oldDS.Status.CollisionCount)) { return } - // TODO: change to compare generation and revision hash - if newDS.ResourceVersion == oldDS.ResourceVersion || reflect.DeepEqual(newDS.Spec.Template.Spec, oldDS.Spec.Template.Spec) { + var key string + var err error + if key, err = cache.MetaNamespaceKeyFunc(newDS); err != nil { + utilruntime.HandleError(err) return } - ctrl.enqueueDaemonset(new) + klog.Infof("Got daemonset udpate event: %v", key) + ctrl.daemonsetWorkqueue.Add(key) }, }) @@ -94,24 +103,51 @@ func NewController(kc client.Interface, daemonsetInformer appsinformers.DaemonSe oldNode := old.(*corev1.Node) newNode := new.(*corev1.Node) if !NodeReady(&oldNode.Status) && NodeReady(&newNode.Status) { - klog.Infof("node %q turn to ready", newNode.Name) - ctrl.nodeWorkqueue.Add(newNode.Name) + klog.Infof("Node %v turn to ready", newNode.Name) + + if err := ctrl.enqueueDaemonsetWhenNodeReady(newNode); err != nil { + klog.Errorf("Enqueue daemonset failed when node %v turns ready, %v", newNode.Name, err) + return + } } }, }) return &ctrl } -func (c *Controller) enqueueDaemonset(obj interface{}) { - var key string - var err error - if key, err = cache.MetaNamespaceKeyFunc(obj); err != nil { - utilruntime.HandleError(err) - return +// enqueueDaemonsetWhenNodeReady add daemonset key to daemonsetWorkqueue for further handling +func (c *Controller) enqueueDaemonsetWhenNodeReady(node *corev1.Node) error { + // TODO: Question-> is source lister up-to-date? + pods, err := GetNodePods(c.podLister, node) + if err != nil { + return err } - klog.Infof("Got udpate event: %v", key) - c.daemonsetWorkqueue.AddRateLimited(key) + for _, pod := range pods { + owner := metav1.GetControllerOf(pod) + switch owner.Kind { + case DaemonSet: + ds, err := c.daemonsetLister.DaemonSets(pod.Namespace).Get(owner.Name) + if err != nil { + if apierrors.IsNotFound(err) { + continue + } + return err + } + + if checkPrerequisites(ds) { + var key string + if key, err = cache.MetaNamespaceKeyFunc(ds); err != nil { + return err + } + + c.daemonsetWorkqueue.Add(key) + } + default: + continue + } + } + return nil } func (c *Controller) Run(threadiness int, stopCh <-chan struct{}) { @@ -120,7 +156,6 @@ func (c *Controller) Run(threadiness int, stopCh <-chan struct{}) { klog.Info("Starting pod upgrade controller") defer klog.Info("Shutting down pod upgrade controller") defer c.daemonsetWorkqueue.ShutDown() - defer c.nodeWorkqueue.ShutDown() //synchronize the cache before starting to process events if !cache.WaitForCacheSync(stopCh, c.daemonsetSynced, c.nodeSynced, @@ -132,10 +167,6 @@ func (c *Controller) Run(threadiness int, stopCh <-chan struct{}) { go wait.Until(c.runDaemonsetWorker, time.Second, stopCh) } - for i := 0; i < threadiness; i++ { - go wait.Until(c.runNodeWorker, time.Second, stopCh) - } - <-stopCh } @@ -149,27 +180,10 @@ func (c *Controller) runDaemonsetWorker() { if err := c.syncDaemonsetHandler(obj.(string)); err != nil { utilruntime.HandleError(err) } - c.daemonsetWorkqueue.Forget(obj) c.daemonsetWorkqueue.Done(obj) } } -func (c *Controller) runNodeWorker() { - for { - obj, shutdown := c.nodeWorkqueue.Get() - if shutdown { - return - } - - nodeName := obj.(string) - if err := c.syncNodeHandler(nodeName); err != nil { - utilruntime.HandleError(err) - } - - c.nodeWorkqueue.Done(nodeName) - } -} - func (c *Controller) syncDaemonsetHandler(key string) error { defer func() { klog.V(4).Infof("Finish syncing pod upgrade request %q", key) @@ -261,80 +275,3 @@ func (c *Controller) autoUpgrade(ds *appsv1.DaemonSet, pods []*corev1.Pod) error return nil } - -// syncNodeHandler delete the pod of daemonset that needs to be upgrade when node turns ready -func (c *Controller) syncNodeHandler(key string) error { - defer func() { - klog.V(4).Infof("Finish syncing pod upgrade request %q", key) - }() - - klog.V(4).Infof("Start handler pod upgrade request %q", key) - - node, err := c.nodeLister.Get(key) - if err != nil { - // If node not found, just ignore it. - if apierrors.IsNotFound(err) { - klog.V(5).Infof("Sync node %v not found", key) - return nil - } - return err - } - - // get all pod in this node - pods, err := GetNodePods(c.podLister, node) - if err != nil { - return err - } - - if err := c.upgradePodsWhenNodeReady(c.kubeclientset, pods); err != nil { - return err - } - - return nil -} - -// upgradePodsWhenNodeReady check pods in current node need to be upgraded -// Only workloads with annotation "apps.openyurt.io/upgrade-strategy" and updateStrategy "OnDelete" will be processed -func (c *Controller) upgradePodsWhenNodeReady(clientset client.Interface, pods []*corev1.Pod) error { - for _, pod := range pods { - owner := metav1.GetControllerOf(pod) - switch owner.Kind { - case DaemonSet: - ds, err := clientset.AppsV1().DaemonSets(pod.Namespace).Get(context.TODO(), owner.Name, metav1.GetOptions{}) - if err != nil { - return err - } - - // consider only the case with annotation "apps.openyurt.io/upgrade-strategy" - v, ok := ds.Annotations[UpgradeAnnotation] - // consider only the case with updateStrategy "OnDelete" - updateStrategyOK := (ds.Spec.UpdateStrategy.Type == appsv1.OnDeleteDaemonSetStrategyType) - if ok && updateStrategyOK { - switch v { - // set pod annotation when "apps.openyurt.io/upgrade-strategy=ota" - case OTAUpgrade: - if err := SetPodUpgradeAnnotation(clientset, ds, pod); err != nil { - return err - } - - // auto upgrade pod when "apps.openyurt.io/upgrade-strategy=auto" - case AutoUpgrade: - latestOK, err := IsDaemonsetPodLatest(ds, pod) - if err != nil { - return err - } - if !latestOK { - if err := clientset.CoreV1().Pods(pod.Namespace). - Delete(context.TODO(), pod.Name, metav1.DeleteOptions{}); err != nil { - return err - } - klog.Infof("Auto upgrade pod %v/%v", ds.Name, pod.Name) - } - } - } - default: - continue - } - } - return nil -} diff --git a/pkg/controller/podupgrade/pod_upgrade_controller_test.go b/pkg/controller/podupgrade/pod_upgrade_controller_test.go index 01d9a18dac9..7bde83979e1 100644 --- a/pkg/controller/podupgrade/pod_upgrade_controller_test.go +++ b/pkg/controller/podupgrade/pod_upgrade_controller_test.go @@ -34,11 +34,12 @@ import ( "k8s.io/client-go/kubernetes/fake" ktest "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" + + k8sutil "github.com/openyurtio/openyurt/pkg/util/kubernetes/controller" ) var ( simpleDaemonSetLabel = map[string]string{"foo": "bar"} - alwaysReady = func() bool { return true } ) var controllerKind = appsv1.SchemeGroupVersion.WithKind("DaemonSet") @@ -82,38 +83,26 @@ func (f *fixture) newController() (*Controller, informers.SharedInformerFactory) c := NewController(f.client, sharedInformers.Apps().V1().DaemonSets(), sharedInformers.Core().V1().Nodes(), sharedInformers.Core().V1().Pods()) - c.daemonsetSynced = alwaysReady - c.nodeSynced = alwaysReady - return c, sharedInformers } // run execute the controller logic -// kind=SyncDaemonset test daemonset update event -// kind=SyncNode test node ready event -func (f *fixture) run(key string, kind string) { - f.testController(key, kind, false) +func (f *fixture) run(key string) { + f.testController(key, false) } func (f *fixture) runExpectError(key string, kind string) { - f.testController(key, kind, true) + f.testController(key, true) } -func (f *fixture) testController(key string, kind string, expectError bool) { +func (f *fixture) testController(key string, expectError bool) { c, sharedInformers := f.newController() stopCh := make(chan struct{}) defer close(stopCh) sharedInformers.Start(stopCh) - var err error - switch kind { - case SyncDaemonset: - err = c.syncDaemonsetHandler(key) - - case SyncNode: - err = c.syncNodeHandler(key) - } + err := c.syncDaemonsetHandler(key) if !expectError && err != nil { f.t.Errorf("error syncing: %v", err) @@ -197,7 +186,7 @@ func newPod(podName string, nodeName string, label map[string]string, ds *appsv1 var podSpec corev1.PodSpec // Copy pod spec from DaemonSet template, or use a default one if DaemonSet is nil if ds != nil { - hash := ComputeHash(&ds.Spec.Template, ds.Status.CollisionCount) + hash := k8sutil.ComputeHash(&ds.Spec.Template, ds.Status.CollisionCount) newLabels = CloneAndAddLabel(label, appsv1.DefaultDaemonSetUniqueLabelKey, hash) podSpec = ds.Spec.Template.Spec } else { @@ -281,27 +270,7 @@ func TestAutoUpgradeWithDaemonsetUpdate(t *testing.T) { f.expectDeletePodAction(pod) - f.run(getDaemonsetKey(ds, t), SyncDaemonset) -} - -func TestAutoUpgradeWithNodeReady(t *testing.T) { - f := newFixture(t) - ds := newDaemonSet("test-ds", "foo/bar") - setAutoUpgradeAnnotation(ds, "auto") - node := newNode("test-node") - pod := newPod("test-pod", "test-node", simpleDaemonSetLabel, ds) - - ds.Spec.Template.Spec.Containers[0].Image = "foo/bar:v2" - - f.daemonsetLister = append(f.daemonsetLister, ds) - f.nodeLister = append(f.nodeLister, node) - f.podLister = append(f.podLister, pod) - - f.objects = append(f.objects, ds, pod, node) - - f.expectDeletePodAction(pod) - - f.run(node.Name, SyncNode) + f.run(getDaemonsetKey(ds, t)) } func TestOTAUpgrade(t *testing.T) { @@ -317,7 +286,7 @@ func TestOTAUpgrade(t *testing.T) { f.podLister = append(f.podLister, oldPod, newPod) f.objects = append(f.objects, ds, oldPod, newPod) - f.run(getDaemonsetKey(ds, t), SyncDaemonset) + f.run(getDaemonsetKey(ds, t)) // check whether ota upgradable annotation set properly oldPodGot, err := f.client.CoreV1().Pods(ds.Namespace).Get(context.TODO(), oldPod.Name, metav1.GetOptions{}) @@ -338,3 +307,29 @@ func TestOTAUpgrade(t *testing.T) { assert.Equal(t, true, newPodOK) assert.Equal(t, "false", annNewPodGot) } + +func TestController_enqueueDaemonsetWhenNodeReady(t *testing.T) { + f := newFixture(t) + node := newNode("node") + ds1 := newDaemonSet("ds1", "foo/bar:v1") + pod1 := newPod("pod1", "node", simpleDaemonSetLabel, ds1) + pod2 := newPod("pod2", "node", simpleDaemonSetLabel, ds1) + + ds2 := newDaemonSet("ds2", "foo/bar:v2") + pod3 := newPod("pod3", "node", simpleDaemonSetLabel, ds2) + pod4 := newPod("pod4", "node", simpleDaemonSetLabel, ds2) + + setAutoUpgradeAnnotation(ds1, "ota") + setAutoUpgradeAnnotation(ds2, "auto") + + f.daemonsetLister = append(f.daemonsetLister, ds1, ds2) + f.podLister = append(f.podLister, pod1, pod2, pod3, pod4) + f.objects = append(f.objects, node, ds1, ds2, pod1, pod2, pod3, pod4) + + c, _ := f.newController() + + c.enqueueDaemonsetWhenNodeReady(node) + + // enqueue "default/ds1" and "default/ds2" + assert.Equal(t, 2, c.daemonsetWorkqueue.Len()) +} diff --git a/pkg/controller/podupgrade/util.go b/pkg/controller/podupgrade/util.go index 0efe749ddb9..55fe78346e7 100644 --- a/pkg/controller/podupgrade/util.go +++ b/pkg/controller/podupgrade/util.go @@ -18,22 +18,19 @@ package podupgrade import ( "context" - "encoding/binary" "fmt" - "hash" - "hash/fnv" "strconv" - "github.com/davecgh/go-spew/spew" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/util/rand" client "k8s.io/client-go/kubernetes" corelisters "k8s.io/client-go/listers/core/v1" "k8s.io/klog/v2" + + k8sutil "github.com/openyurtio/openyurt/pkg/util/kubernetes/controller" ) var DaemonSet = "DaemonSet" @@ -96,7 +93,7 @@ func GetNodePods(podLister corelisters.PodLister, node *corev1.Node) ([]*corev1. } if len(nodePodsNames) > 0 { - klog.V(5).Infof("Daemonset %v has pods %v", node.Name, nodePodsNames) + klog.V(5).Infof("Node %v has pods %v", node.Name, nodePodsNames) } return nodePods, nil } @@ -104,7 +101,7 @@ func GetNodePods(podLister corelisters.PodLister, node *corev1.Node) ([]*corev1. // IsDaemonsetPodLatest check whether pod is latest by comparing its Spec with daemonset's // If pod is latest, return true, otherwise return false func IsDaemonsetPodLatest(ds *appsv1.DaemonSet, pod *corev1.Pod) (bool, error) { - hash := ComputeHash(&ds.Spec.Template, ds.Status.CollisionCount) + hash := k8sutil.ComputeHash(&ds.Spec.Template, ds.Status.CollisionCount) klog.V(4).Infof("compute hash: %v", hash) generation, err := GetTemplateGeneration(ds) if err != nil { @@ -152,6 +149,7 @@ func NodeReady(nodeStatus *corev1.NodeStatus) bool { return false } +// SetPodUpgradeAnnotation calculate and set annotation "apps.openyurt.io/pod-upgradable" to pod func SetPodUpgradeAnnotation(clientset client.Interface, ds *appsv1.DaemonSet, pod *corev1.Pod) error { ok, err := IsDaemonsetPodLatest(ds, pod) if err != nil { @@ -172,35 +170,15 @@ func SetPodUpgradeAnnotation(clientset client.Interface, ds *appsv1.DaemonSet, p return nil } -// ComputeHash returns a hash value calculated from pod template and -// a collisionCount to avoid hash collision. The hash will be safe encoded to -// avoid bad words. -func ComputeHash(template *corev1.PodTemplateSpec, collisionCount *int32) string { - podTemplateSpecHasher := fnv.New32a() - DeepHashObject(podTemplateSpecHasher, *template) - - // Add collisionCount in the hash if it exists. - if collisionCount != nil { - collisionCountBytes := make([]byte, 8) - binary.LittleEndian.PutUint32(collisionCountBytes, uint32(*collisionCount)) - podTemplateSpecHasher.Write(collisionCountBytes) +// checkPrerequisites checks that daemonset meets two conditions +// 1. annotation "apps.openyurt.io/upgrade-strategy"="auto" or "ota" +// 2. update strategy is "OnDelete" +func checkPrerequisites(ds *appsv1.DaemonSet) bool { + v, ok := ds.Annotations[UpgradeAnnotation] + if !ok || (v != "auto" && v != "ota") { + return false } - - return rand.SafeEncodeString(fmt.Sprint(podTemplateSpecHasher.Sum32())) -} - -// DeepHashObject writes specified object to hash using the spew library -// which follows pointers and prints actual values of the nested objects -// ensuring the hash does not change when a pointer changes. -func DeepHashObject(hasher hash.Hash, objectToWrite interface{}) { - hasher.Reset() - printer := spew.ConfigState{ - Indent: " ", - SortKeys: true, - DisableMethods: true, - SpewKeys: true, - } - printer.Fprintf(hasher, "%#v", objectToWrite) + return ds.Spec.UpdateStrategy.Type == appsv1.OnDeleteDaemonSetStrategyType } // Clones the given map and returns a new map with the given key and value added. diff --git a/pkg/controller/podupgrade/util_test.go b/pkg/controller/podupgrade/util_test.go index 3a474adc486..f63f9a275ac 100644 --- a/pkg/controller/podupgrade/util_test.go +++ b/pkg/controller/podupgrade/util_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" ) @@ -94,3 +95,94 @@ func TestIsDaemonsetPodLatest(t *testing.T) { }) } } + +func Test_checkPrerequisites(t *testing.T) { + tests := []struct { + name string + ds *appsv1.DaemonSet + want bool + }{ + { + name: "satisfied-ota", + ds: &appsv1.DaemonSet{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + "apps.openyurt.io/upgrade-strategy": "ota", + }, + }, + Spec: appsv1.DaemonSetSpec{ + UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ + Type: appsv1.OnDeleteDaemonSetStrategyType, + }, + }, + }, + want: true, + }, + { + name: "satisfied-auto", + ds: &appsv1.DaemonSet{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + "apps.openyurt.io/upgrade-strategy": "auto", + }, + }, + Spec: appsv1.DaemonSetSpec{ + UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ + Type: appsv1.OnDeleteDaemonSetStrategyType, + }, + }, + }, + want: true, + }, + { + name: "unsatisfied-other", + ds: &appsv1.DaemonSet{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + "apps.openyurt.io/upgrade-strategy": "other", + }, + }, + Spec: appsv1.DaemonSetSpec{ + UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ + Type: appsv1.OnDeleteDaemonSetStrategyType, + }, + }, + }, + want: false, + }, + { + name: "unsatisfied-without-ann", + ds: &appsv1.DaemonSet{ + Spec: appsv1.DaemonSetSpec{ + UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ + Type: appsv1.OnDeleteDaemonSetStrategyType, + }, + }, + }, + want: false, + }, + { + name: "unsatisfied-without-updateStrategy", + ds: &appsv1.DaemonSet{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + "apps.openyurt.io/upgrade-strategy": "other", + }, + }, + }, + want: false, + }, + { + name: "unsatisfied-without-both", + ds: &appsv1.DaemonSet{}, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := checkPrerequisites(tt.ds); got != tt.want { + t.Errorf("checkPrerequisites() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/util/kubernetes/controller/controller_utils.go b/pkg/util/kubernetes/controller/controller_utils.go new file mode 100644 index 00000000000..18239357d35 --- /dev/null +++ b/pkg/util/kubernetes/controller/controller_utils.go @@ -0,0 +1,58 @@ +/* +Copyright 2014 The Kubernetes Authors. +Copyright 2022 The OpenYurt 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 controller + +import ( + "encoding/binary" + "fmt" + "hash" + "hash/fnv" + + "github.com/davecgh/go-spew/spew" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/rand" +) + +// ComputeHash returns a hash value calculated from pod template and +// a collisionCount to avoid hash collision. The hash will be safe encoded to +// avoid bad words. +func ComputeHash(template *v1.PodTemplateSpec, collisionCount *int32) string { + podTemplateSpecHasher := fnv.New32a() + DeepHashObject(podTemplateSpecHasher, *template) + + // Add collisionCount in the hash if it exists. + if collisionCount != nil { + collisionCountBytes := make([]byte, 8) + binary.LittleEndian.PutUint32(collisionCountBytes, uint32(*collisionCount)) + podTemplateSpecHasher.Write(collisionCountBytes) + } + + return rand.SafeEncodeString(fmt.Sprint(podTemplateSpecHasher.Sum32())) +} + +// DeepHashObject writes specified object to hash using the spew library +// which follows pointers and prints actual values of the nested objects +// ensuring the hash does not change when a pointer changes. +func DeepHashObject(hasher hash.Hash, objectToWrite interface{}) { + hasher.Reset() + printer := spew.ConfigState{ + Indent: " ", + SortKeys: true, + DisableMethods: true, + SpewKeys: true, + } + printer.Fprintf(hasher, "%#v", objectToWrite) +} From 79abddc55897b5202b7fefa7dd5995c904c0a79b Mon Sep 17 00:00:00 2001 From: hxcGit Date: Tue, 6 Sep 2022 20:39:51 +0800 Subject: [PATCH 04/11] add maxUnavailable limitation Signed-off-by: hxcGit --- .../kubernetes/util/controller_util.go | 647 ++++++++++++++++++ .../kubernetes/util/daemonset_util.go | 63 ++ .../podupgrade/kubernetes/util/pod_util.go | 83 +++ .../podupgrade/pod_upgrade_controller.go | 301 ++++++-- pkg/controller/podupgrade/util.go | 38 +- pkg/controller/podupgrade/util_test.go | 20 +- 6 files changed, 1078 insertions(+), 74 deletions(-) create mode 100644 pkg/controller/podupgrade/kubernetes/util/controller_util.go create mode 100644 pkg/controller/podupgrade/kubernetes/util/daemonset_util.go create mode 100644 pkg/controller/podupgrade/kubernetes/util/pod_util.go diff --git a/pkg/controller/podupgrade/kubernetes/util/controller_util.go b/pkg/controller/podupgrade/kubernetes/util/controller_util.go new file mode 100644 index 00000000000..655aa0688b8 --- /dev/null +++ b/pkg/controller/podupgrade/kubernetes/util/controller_util.go @@ -0,0 +1,647 @@ +/* +Copyright 2014 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 util + +import ( + "context" + "encoding/binary" + "fmt" + "hash" + "hash/fnv" + "sync" + "sync/atomic" + "time" + + "github.com/davecgh/go-spew/spew" + v1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/rand" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/wait" + clientset "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/cache" + "k8s.io/client-go/tools/record" + "k8s.io/utils/clock" + + "k8s.io/klog/v2" +) + +const ( + // If a watch drops a delete event for a pod, it'll take this long + // before a dormant controller waiting for those packets is woken up anyway. It is + // specifically targeted at the case where some problem prevents an update + // of expectations, without it the controller could stay asleep forever. This should + // be set based on the expected latency of watch events. + // + // Currently a controller can service (create *and* observe the watch events for said + // creation) about 10 pods a second, so it takes about 1 min to service + // 500 pods. Just creation is limited to 20qps, and watching happens with ~10-30s + // latency/pod at the scale of 3000 pods over 100 nodes. + ExpectationsTimeout = 5 * time.Minute + // When batching pod creates, SlowStartInitialBatchSize is the size of the + // initial batch. The size of each successive batch is twice the size of + // the previous batch. For example, for a value of 1, batch sizes would be + // 1, 2, 4, 8, ... and for a value of 10, batch sizes would be + // 10, 20, 40, 80, ... Setting the value higher means that quota denials + // will result in more doomed API calls and associated event spam. Setting + // the value lower will result in more API call round trip periods for + // large batches. + // + // Given a number of pods to start "N": + // The number of doomed calls per sync once quota is exceeded is given by: + // min(N,SlowStartInitialBatchSize) + // The number of batches is given by: + // 1+floor(log_2(ceil(N/SlowStartInitialBatchSize))) + SlowStartInitialBatchSize = 1 +) + +var UpdateTaintBackoff = wait.Backoff{ + Steps: 5, + Duration: 100 * time.Millisecond, + Jitter: 1.0, +} + +var UpdateLabelBackoff = wait.Backoff{ + Steps: 5, + Duration: 100 * time.Millisecond, + Jitter: 1.0, +} + +var ( + KeyFunc = cache.DeletionHandlingMetaNamespaceKeyFunc +) + +type ResyncPeriodFunc func() time.Duration + +// Returns 0 for resyncPeriod in case resyncing is not needed. +func NoResyncPeriodFunc() time.Duration { + return 0 +} + +// StaticResyncPeriodFunc returns the resync period specified +func StaticResyncPeriodFunc(resyncPeriod time.Duration) ResyncPeriodFunc { + return func() time.Duration { + return resyncPeriod + } +} + +// Expectations are a way for controllers to tell the controller manager what they expect. eg: +// ControllerExpectations: { +// controller1: expects 2 adds in 2 minutes +// controller2: expects 2 dels in 2 minutes +// controller3: expects -1 adds in 2 minutes => controller3's expectations have already been met +// } +// +// Implementation: +// ControlleeExpectation = pair of atomic counters to track controllee's creation/deletion +// ControllerExpectationsStore = TTLStore + a ControlleeExpectation per controller +// +// * Once set expectations can only be lowered +// * A controller isn't synced till its expectations are either fulfilled, or expire +// * Controllers that don't set expectations will get woken up for every matching controllee + +// ExpKeyFunc to parse out the key from a ControlleeExpectation +var ExpKeyFunc = func(obj interface{}) (string, error) { + if e, ok := obj.(*ControlleeExpectations); ok { + return e.key, nil + } + return "", fmt.Errorf("could not find key for obj %#v", obj) +} + +// ControllerExpectationsInterface is an interface that allows users to set and wait on expectations. +// Only abstracted out for testing. +// Warning: if using KeyFunc it is not safe to use a single ControllerExpectationsInterface with different +// types of controllers, because the keys might conflict across types. +type ControllerExpectationsInterface interface { + GetExpectations(controllerKey string) (*ControlleeExpectations, bool, error) + SatisfiedExpectations(controllerKey string) bool + DeleteExpectations(controllerKey string) + SetExpectations(controllerKey string, add, del int) error + ExpectCreations(controllerKey string, adds int) error + ExpectDeletions(controllerKey string, dels int) error + CreationObserved(controllerKey string) + DeletionObserved(controllerKey string) + RaiseExpectations(controllerKey string, add, del int) + LowerExpectations(controllerKey string, add, del int) +} + +// ControllerExpectations is a cache mapping controllers to what they expect to see before being woken up for a sync. +type ControllerExpectations struct { + cache.Store +} + +// GetExpectations returns the ControlleeExpectations of the given controller. +func (r *ControllerExpectations) GetExpectations(controllerKey string) (*ControlleeExpectations, bool, error) { + exp, exists, err := r.GetByKey(controllerKey) + if err == nil && exists { + return exp.(*ControlleeExpectations), true, nil + } + return nil, false, err +} + +// DeleteExpectations deletes the expectations of the given controller from the TTLStore. +func (r *ControllerExpectations) DeleteExpectations(controllerKey string) { + if exp, exists, err := r.GetByKey(controllerKey); err == nil && exists { + if err := r.Delete(exp); err != nil { + klog.V(2).Infof("Error deleting expectations for controller %v: %v", controllerKey, err) + } + } +} + +// SatisfiedExpectations returns true if the required adds/dels for the given controller have been observed. +// Add/del counts are established by the controller at sync time, and updated as controllees are observed by the controller +// manager. +func (r *ControllerExpectations) SatisfiedExpectations(controllerKey string) bool { + if exp, exists, err := r.GetExpectations(controllerKey); exists { + if exp.Fulfilled() { + klog.V(4).Infof("Controller expectations fulfilled %#v", exp) + return true + } else if exp.isExpired() { + klog.V(4).Infof("Controller expectations expired %#v", exp) + return true + } else { + klog.V(4).Infof("Controller still waiting on expectations %#v", exp) + return false + } + } else if err != nil { + klog.V(2).Infof("Error encountered while checking expectations %#v, forcing sync", err) + } else { + // When a new controller is created, it doesn't have expectations. + // When it doesn't see expected watch events for > TTL, the expectations expire. + // - In this case it wakes up, creates/deletes controllees, and sets expectations again. + // When it has satisfied expectations and no controllees need to be created/destroyed > TTL, the expectations expire. + // - In this case it continues without setting expectations till it needs to create/delete controllees. + klog.V(4).Infof("Controller %v either never recorded expectations, or the ttl expired.", controllerKey) + } + // Trigger a sync if we either encountered and error (which shouldn't happen since we're + // getting from local store) or this controller hasn't established expectations. + return true +} + +// TODO: Extend ExpirationCache to support explicit expiration. +// TODO: Make this possible to disable in tests. +// TODO: Support injection of clock. +func (exp *ControlleeExpectations) isExpired() bool { + return clock.RealClock{}.Since(exp.timestamp) > ExpectationsTimeout +} + +// SetExpectations registers new expectations for the given controller. Forgets existing expectations. +func (r *ControllerExpectations) SetExpectations(controllerKey string, add, del int) error { + exp := &ControlleeExpectations{add: int64(add), del: int64(del), key: controllerKey, timestamp: clock.RealClock{}.Now()} + klog.V(4).Infof("Setting expectations %#v", exp) + return r.Add(exp) +} + +func (r *ControllerExpectations) ExpectCreations(controllerKey string, adds int) error { + return r.SetExpectations(controllerKey, adds, 0) +} + +func (r *ControllerExpectations) ExpectDeletions(controllerKey string, dels int) error { + return r.SetExpectations(controllerKey, 0, dels) +} + +// Decrements the expectation counts of the given controller. +func (r *ControllerExpectations) LowerExpectations(controllerKey string, add, del int) { + if exp, exists, err := r.GetExpectations(controllerKey); err == nil && exists { + exp.Add(int64(-add), int64(-del)) + // The expectations might've been modified since the update on the previous line. + klog.V(4).Infof("Lowered expectations %#v", exp) + } +} + +// Increments the expectation counts of the given controller. +func (r *ControllerExpectations) RaiseExpectations(controllerKey string, add, del int) { + if exp, exists, err := r.GetExpectations(controllerKey); err == nil && exists { + exp.Add(int64(add), int64(del)) + // The expectations might've been modified since the update on the previous line. + klog.V(4).Infof("Raised expectations %#v", exp) + } +} + +// CreationObserved atomically decrements the `add` expectation count of the given controller. +func (r *ControllerExpectations) CreationObserved(controllerKey string) { + r.LowerExpectations(controllerKey, 1, 0) +} + +// DeletionObserved atomically decrements the `del` expectation count of the given controller. +func (r *ControllerExpectations) DeletionObserved(controllerKey string) { + r.LowerExpectations(controllerKey, 0, 1) +} + +// ControlleeExpectations track controllee creates/deletes. +type ControlleeExpectations struct { + // Important: Since these two int64 fields are using sync/atomic, they have to be at the top of the struct due to a bug on 32-bit platforms + // See: https://golang.org/pkg/sync/atomic/ for more information + add int64 + del int64 + key string + timestamp time.Time +} + +// Add increments the add and del counters. +func (e *ControlleeExpectations) Add(add, del int64) { + atomic.AddInt64(&e.add, add) + atomic.AddInt64(&e.del, del) +} + +// Fulfilled returns true if this expectation has been fulfilled. +func (e *ControlleeExpectations) Fulfilled() bool { + // TODO: think about why this line being atomic doesn't matter + return atomic.LoadInt64(&e.add) <= 0 && atomic.LoadInt64(&e.del) <= 0 +} + +// GetExpectations returns the add and del expectations of the controllee. +func (e *ControlleeExpectations) GetExpectations() (int64, int64) { + return atomic.LoadInt64(&e.add), atomic.LoadInt64(&e.del) +} + +// NewControllerExpectations returns a store for ControllerExpectations. +func NewControllerExpectations() *ControllerExpectations { + return &ControllerExpectations{cache.NewStore(ExpKeyFunc)} +} + +// UIDSetKeyFunc to parse out the key from a UIDSet. +var UIDSetKeyFunc = func(obj interface{}) (string, error) { + if u, ok := obj.(*UIDSet); ok { + return u.key, nil + } + return "", fmt.Errorf("could not find key for obj %#v", obj) +} + +// UIDSet holds a key and a set of UIDs. Used by the +// UIDTrackingControllerExpectations to remember which UID it has seen/still +// waiting for. +type UIDSet struct { + sets.String + key string +} + +// UIDTrackingControllerExpectations tracks the UID of the pods it deletes. +// This cache is needed over plain old expectations to safely handle graceful +// deletion. The desired behavior is to treat an update that sets the +// DeletionTimestamp on an object as a delete. To do so consistently, one needs +// to remember the expected deletes so they aren't double counted. +// TODO: Track creates as well (#22599) +type UIDTrackingControllerExpectations struct { + ControllerExpectationsInterface + // TODO: There is a much nicer way to do this that involves a single store, + // a lock per entry, and a ControlleeExpectationsInterface type. + uidStoreLock sync.Mutex + // Store used for the UIDs associated with any expectation tracked via the + // ControllerExpectationsInterface. + uidStore cache.Store +} + +// GetUIDs is a convenience method to avoid exposing the set of expected uids. +// The returned set is not thread safe, all modifications must be made holding +// the uidStoreLock. +func (u *UIDTrackingControllerExpectations) GetUIDs(controllerKey string) sets.String { + if uid, exists, err := u.uidStore.GetByKey(controllerKey); err == nil && exists { + return uid.(*UIDSet).String + } + return nil +} + +// ExpectDeletions records expectations for the given deleteKeys, against the given controller. +func (u *UIDTrackingControllerExpectations) ExpectDeletions(rcKey string, deletedKeys []string) error { + expectedUIDs := sets.NewString() + for _, k := range deletedKeys { + expectedUIDs.Insert(k) + } + klog.V(4).Infof("Controller %v waiting on deletions for: %+v", rcKey, deletedKeys) + u.uidStoreLock.Lock() + defer u.uidStoreLock.Unlock() + + if existing := u.GetUIDs(rcKey); existing != nil && existing.Len() != 0 { + klog.Errorf("Clobbering existing delete keys: %+v", existing) + } + if err := u.uidStore.Add(&UIDSet{expectedUIDs, rcKey}); err != nil { + return err + } + return u.ControllerExpectationsInterface.ExpectDeletions(rcKey, expectedUIDs.Len()) +} + +// DeletionObserved records the given deleteKey as a deletion, for the given rc. +func (u *UIDTrackingControllerExpectations) DeletionObserved(rcKey, deleteKey string) { + u.uidStoreLock.Lock() + defer u.uidStoreLock.Unlock() + + uids := u.GetUIDs(rcKey) + if uids != nil && uids.Has(deleteKey) { + klog.V(4).Infof("Controller %v received delete for pod %v", rcKey, deleteKey) + u.ControllerExpectationsInterface.DeletionObserved(rcKey) + uids.Delete(deleteKey) + } +} + +// DeleteExpectations deletes the UID set and invokes DeleteExpectations on the +// underlying ControllerExpectationsInterface. +func (u *UIDTrackingControllerExpectations) DeleteExpectations(rcKey string) { + u.uidStoreLock.Lock() + defer u.uidStoreLock.Unlock() + + u.ControllerExpectationsInterface.DeleteExpectations(rcKey) + if uidExp, exists, err := u.uidStore.GetByKey(rcKey); err == nil && exists { + if err := u.uidStore.Delete(uidExp); err != nil { + klog.V(2).Infof("Error deleting uid expectations for controller %v: %v", rcKey, err) + } + } +} + +// NewUIDTrackingControllerExpectations returns a wrapper around +// ControllerExpectations that is aware of deleteKeys. +func NewUIDTrackingControllerExpectations(ce ControllerExpectationsInterface) *UIDTrackingControllerExpectations { + return &UIDTrackingControllerExpectations{ControllerExpectationsInterface: ce, uidStore: cache.NewStore(UIDSetKeyFunc)} +} + +// Reasons for pod events +const ( + // FailedCreatePodReason is added in an event and in a replica set condition + // when a pod for a replica set is failed to be created. + FailedCreatePodReason = "FailedCreate" + // SuccessfulCreatePodReason is added in an event when a pod for a replica set + // is successfully created. + SuccessfulCreatePodReason = "SuccessfulCreate" + // FailedDeletePodReason is added in an event and in a replica set condition + // when a pod for a replica set is failed to be deleted. + FailedDeletePodReason = "FailedDelete" + // SuccessfulDeletePodReason is added in an event when a pod for a replica set + // is successfully deleted. + SuccessfulDeletePodReason = "SuccessfulDelete" +) + +// ComputeHash returns a hash value calculated from pod template and +// a collisionCount to avoid hash collision. The hash will be safe encoded to +// avoid bad words. +func ComputeHash(template *v1.PodTemplateSpec, collisionCount *int32) string { + podTemplateSpecHasher := fnv.New32a() + DeepHashObject(podTemplateSpecHasher, *template) + + // Add collisionCount in the hash if it exists. + if collisionCount != nil { + collisionCountBytes := make([]byte, 8) + binary.LittleEndian.PutUint32(collisionCountBytes, uint32(*collisionCount)) + podTemplateSpecHasher.Write(collisionCountBytes) + } + + return rand.SafeEncodeString(fmt.Sprint(podTemplateSpecHasher.Sum32())) +} + +// DeepHashObject writes specified object to hash using the spew library +// which follows pointers and prints actual values of the nested objects +// ensuring the hash does not change when a pointer changes. +func DeepHashObject(hasher hash.Hash, objectToWrite interface{}) { + hasher.Reset() + printer := spew.ConfigState{ + Indent: " ", + SortKeys: true, + DisableMethods: true, + SpewKeys: true, + } + printer.Fprintf(hasher, "%#v", objectToWrite) +} + +// PodControlInterface is an interface that knows how to add or delete pods +// created as an interface to allow testing. +type PodControlInterface interface { + // CreatePods creates new pods according to the spec, and sets object as the pod's controller. + CreatePods(ctx context.Context, namespace string, template *v1.PodTemplateSpec, object runtime.Object, controllerRef *metav1.OwnerReference) error + // CreatePodsWithGenerateName creates new pods according to the spec, sets object as the pod's controller and sets pod's generateName. + CreatePodsWithGenerateName(ctx context.Context, namespace string, template *v1.PodTemplateSpec, object runtime.Object, controllerRef *metav1.OwnerReference, generateName string) error + // DeletePod deletes the pod identified by podID. + DeletePod(ctx context.Context, namespace string, podID string, object runtime.Object) error + // PatchPod patches the pod. + PatchPod(ctx context.Context, namespace, name string, data []byte) error +} + +// RealPodControl is the default implementation of PodControlInterface. +type RealPodControl struct { + KubeClient clientset.Interface + Recorder record.EventRecorder +} + +var _ PodControlInterface = &RealPodControl{} + +func getPodsLabelSet(template *v1.PodTemplateSpec) labels.Set { + desiredLabels := make(labels.Set) + for k, v := range template.Labels { + desiredLabels[k] = v + } + return desiredLabels +} + +func getPodsFinalizers(template *v1.PodTemplateSpec) []string { + desiredFinalizers := make([]string, len(template.Finalizers)) + copy(desiredFinalizers, template.Finalizers) + return desiredFinalizers +} + +func getPodsAnnotationSet(template *v1.PodTemplateSpec) labels.Set { + desiredAnnotations := make(labels.Set) + for k, v := range template.Annotations { + desiredAnnotations[k] = v + } + return desiredAnnotations +} + +func getPodsPrefix(controllerName string) string { + // use the dash (if the name isn't too long) to make the pod name a bit prettier + prefix := fmt.Sprintf("%s-", controllerName) + if len(apimachineryvalidation.NameIsDNSSubdomain(prefix, true)) != 0 { + prefix = controllerName + } + return prefix +} + +func validateControllerRef(controllerRef *metav1.OwnerReference) error { + if controllerRef == nil { + return fmt.Errorf("controllerRef is nil") + } + if len(controllerRef.APIVersion) == 0 { + return fmt.Errorf("controllerRef has empty APIVersion") + } + if len(controllerRef.Kind) == 0 { + return fmt.Errorf("controllerRef has empty Kind") + } + if controllerRef.Controller == nil || !*controllerRef.Controller { + return fmt.Errorf("controllerRef.Controller is not set to true") + } + if controllerRef.BlockOwnerDeletion == nil || !*controllerRef.BlockOwnerDeletion { + return fmt.Errorf("controllerRef.BlockOwnerDeletion is not set") + } + return nil +} + +func (r RealPodControl) CreatePods(ctx context.Context, namespace string, template *v1.PodTemplateSpec, controllerObject runtime.Object, controllerRef *metav1.OwnerReference) error { + return r.CreatePodsWithGenerateName(ctx, namespace, template, controllerObject, controllerRef, "") +} + +func (r RealPodControl) CreatePodsWithGenerateName(ctx context.Context, namespace string, template *v1.PodTemplateSpec, controllerObject runtime.Object, controllerRef *metav1.OwnerReference, generateName string) error { + if err := validateControllerRef(controllerRef); err != nil { + return err + } + pod, err := GetPodFromTemplate(template, controllerObject, controllerRef) + if err != nil { + return err + } + if len(generateName) > 0 { + pod.ObjectMeta.GenerateName = generateName + } + return r.createPods(ctx, namespace, pod, controllerObject) +} + +func (r RealPodControl) PatchPod(ctx context.Context, namespace, name string, data []byte) error { + _, err := r.KubeClient.CoreV1().Pods(namespace).Patch(ctx, name, types.StrategicMergePatchType, data, metav1.PatchOptions{}) + return err +} + +func GetPodFromTemplate(template *v1.PodTemplateSpec, parentObject runtime.Object, controllerRef *metav1.OwnerReference) (*v1.Pod, error) { + desiredLabels := getPodsLabelSet(template) + desiredFinalizers := getPodsFinalizers(template) + desiredAnnotations := getPodsAnnotationSet(template) + accessor, err := meta.Accessor(parentObject) + if err != nil { + return nil, fmt.Errorf("parentObject does not have ObjectMeta, %v", err) + } + prefix := getPodsPrefix(accessor.GetName()) + + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: desiredLabels, + Annotations: desiredAnnotations, + GenerateName: prefix, + Finalizers: desiredFinalizers, + }, + } + if controllerRef != nil { + pod.OwnerReferences = append(pod.OwnerReferences, *controllerRef) + } + pod.Spec = *template.Spec.DeepCopy() + return pod, nil +} + +func (r RealPodControl) createPods(ctx context.Context, namespace string, pod *v1.Pod, object runtime.Object) error { + if len(labels.Set(pod.Labels)) == 0 { + return fmt.Errorf("unable to create pods, no labels") + } + newPod, err := r.KubeClient.CoreV1().Pods(namespace).Create(ctx, pod, metav1.CreateOptions{}) + if err != nil { + // only send an event if the namespace isn't terminating + if !apierrors.HasStatusCause(err, v1.NamespaceTerminatingCause) { + r.Recorder.Eventf(object, v1.EventTypeWarning, FailedCreatePodReason, "Error creating: %v", err) + } + return err + } + accessor, err := meta.Accessor(object) + if err != nil { + klog.Errorf("parentObject does not have ObjectMeta, %v", err) + return nil + } + klog.V(4).Infof("Controller %v created pod %v", accessor.GetName(), newPod.Name) + r.Recorder.Eventf(object, v1.EventTypeNormal, SuccessfulCreatePodReason, "Created pod: %v", newPod.Name) + + return nil +} + +func (r RealPodControl) DeletePod(ctx context.Context, namespace string, podID string, object runtime.Object) error { + accessor, err := meta.Accessor(object) + if err != nil { + return fmt.Errorf("object does not have ObjectMeta, %v", err) + } + klog.V(2).InfoS("Deleting pod", "controller", accessor.GetName(), "pod", klog.KRef(namespace, podID)) + if err := r.KubeClient.CoreV1().Pods(namespace).Delete(ctx, podID, metav1.DeleteOptions{}); err != nil { + if apierrors.IsNotFound(err) { + klog.V(4).Infof("pod %v/%v has already been deleted.", namespace, podID) + return err + } + r.Recorder.Eventf(object, v1.EventTypeWarning, FailedDeletePodReason, "Error deleting: %v", err) + return fmt.Errorf("unable to delete pods: %v", err) + } + r.Recorder.Eventf(object, v1.EventTypeNormal, SuccessfulDeletePodReason, "Deleted pod: %v", podID) + + return nil +} + +type FakePodControl struct { + sync.Mutex + Templates []v1.PodTemplateSpec + ControllerRefs []metav1.OwnerReference + DeletePodName []string + Patches [][]byte + Err error + CreateLimit int + CreateCallCount int +} + +var _ PodControlInterface = &FakePodControl{} + +func (f *FakePodControl) PatchPod(ctx context.Context, namespace, name string, data []byte) error { + f.Lock() + defer f.Unlock() + f.Patches = append(f.Patches, data) + if f.Err != nil { + return f.Err + } + return nil +} + +func (f *FakePodControl) CreatePods(ctx context.Context, namespace string, spec *v1.PodTemplateSpec, object runtime.Object, controllerRef *metav1.OwnerReference) error { + return f.CreatePodsWithGenerateName(ctx, namespace, spec, object, controllerRef, "") +} + +func (f *FakePodControl) CreatePodsWithGenerateName(ctx context.Context, namespace string, spec *v1.PodTemplateSpec, object runtime.Object, controllerRef *metav1.OwnerReference, generateNamePrefix string) error { + f.Lock() + defer f.Unlock() + f.CreateCallCount++ + if f.CreateLimit != 0 && f.CreateCallCount > f.CreateLimit { + return fmt.Errorf("not creating pod, limit %d already reached (create call %d)", f.CreateLimit, f.CreateCallCount) + } + spec.GenerateName = generateNamePrefix + f.Templates = append(f.Templates, *spec) + f.ControllerRefs = append(f.ControllerRefs, *controllerRef) + if f.Err != nil { + return f.Err + } + return nil +} + +func (f *FakePodControl) DeletePod(ctx context.Context, namespace string, podID string, object runtime.Object) error { + f.Lock() + defer f.Unlock() + f.DeletePodName = append(f.DeletePodName, podID) + if f.Err != nil { + return f.Err + } + return nil +} + +func (f *FakePodControl) Clear() { + f.Lock() + defer f.Unlock() + f.DeletePodName = []string{} + f.Templates = []v1.PodTemplateSpec{} + f.ControllerRefs = []metav1.OwnerReference{} + f.Patches = [][]byte{} + f.CreateLimit = 0 + f.CreateCallCount = 0 +} diff --git a/pkg/controller/podupgrade/kubernetes/util/daemonset_util.go b/pkg/controller/podupgrade/kubernetes/util/daemonset_util.go new file mode 100644 index 00000000000..75e7300ee76 --- /dev/null +++ b/pkg/controller/podupgrade/kubernetes/util/daemonset_util.go @@ -0,0 +1,63 @@ +/* +Copyright 2017 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 util + +import ( + "fmt" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// GetTargetNodeName get the target node name of DaemonSet pods. If `.spec.NodeName` is not empty (nil), +// return `.spec.NodeName`; otherwise, retrieve node name of pending pods from NodeAffinity. Return error +// if failed to retrieve node name from `.spec.NodeName` and NodeAffinity. +func GetTargetNodeName(pod *v1.Pod) (string, error) { + if len(pod.Spec.NodeName) != 0 { + return pod.Spec.NodeName, nil + } + + // Retrieve node name of unscheduled pods from NodeAffinity + if pod.Spec.Affinity == nil || + pod.Spec.Affinity.NodeAffinity == nil || + pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution == nil { + return "", fmt.Errorf("no spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution for pod %s/%s", + pod.Namespace, pod.Name) + } + + terms := pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms + if len(terms) < 1 { + return "", fmt.Errorf("no nodeSelectorTerms in requiredDuringSchedulingIgnoredDuringExecution of pod %s/%s", + pod.Namespace, pod.Name) + } + + for _, term := range terms { + for _, exp := range term.MatchFields { + if exp.Key == metav1.ObjectNameField && + exp.Operator == v1.NodeSelectorOpIn { + if len(exp.Values) != 1 { + return "", fmt.Errorf("the matchFields value of '%s' is not unique for pod %s/%s", + metav1.ObjectNameField, pod.Namespace, pod.Name) + } + + return exp.Values[0], nil + } + } + } + + return "", fmt.Errorf("no node name found for pod %s/%s", pod.Namespace, pod.Name) +} diff --git a/pkg/controller/podupgrade/kubernetes/util/pod_util.go b/pkg/controller/podupgrade/kubernetes/util/pod_util.go new file mode 100644 index 00000000000..73d123501ca --- /dev/null +++ b/pkg/controller/podupgrade/kubernetes/util/pod_util.go @@ -0,0 +1,83 @@ +/* +Copyright 2015 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 util + +import ( + "time" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// IsPodAvailable returns true if a pod is available; false otherwise. +// Precondition for an available pod is that it must be ready. On top +// of that, there are two cases when a pod can be considered available: +// 1. minReadySeconds == 0, or +// 2. LastTransitionTime (is set) + minReadySeconds < current time +func IsPodAvailable(pod *v1.Pod, minReadySeconds int32, now metav1.Time) bool { + if !IsPodReady(pod) { + return false + } + + c := GetPodReadyCondition(pod.Status) + minReadySecondsDuration := time.Duration(minReadySeconds) * time.Second + if minReadySeconds == 0 || (!c.LastTransitionTime.IsZero() && c.LastTransitionTime.Add(minReadySecondsDuration).Before(now.Time)) { + return true + } + return false +} + +// IsPodReady returns true if a pod is ready; false otherwise. +func IsPodReady(pod *v1.Pod) bool { + return IsPodReadyConditionTrue(pod.Status) +} + +// IsPodReadyConditionTrue returns true if a pod is ready; false otherwise. +func IsPodReadyConditionTrue(status v1.PodStatus) bool { + condition := GetPodReadyCondition(status) + return condition != nil && condition.Status == v1.ConditionTrue +} + +// GetPodReadyCondition extracts the pod ready condition from the given status and returns that. +// Returns nil if the condition is not present. +func GetPodReadyCondition(status v1.PodStatus) *v1.PodCondition { + _, condition := GetPodCondition(&status, v1.PodReady) + return condition +} + +// GetPodCondition extracts the provided condition from the given status and returns that. +// Returns nil and -1 if the condition is not present, and the index of the located condition. +func GetPodCondition(status *v1.PodStatus, conditionType v1.PodConditionType) (int, *v1.PodCondition) { + if status == nil { + return -1, nil + } + return GetPodConditionFromList(status.Conditions, conditionType) +} + +// GetPodConditionFromList extracts the provided condition from the given list of condition and +// returns the index of the condition and the condition. Returns -1 and nil if the condition is not present. +func GetPodConditionFromList(conditions []v1.PodCondition, conditionType v1.PodConditionType) (int, *v1.PodCondition) { + if conditions == nil { + return -1, nil + } + for i := range conditions { + if conditions[i].Type == conditionType { + return i, &conditions[i] + } + } + return -1, nil +} diff --git a/pkg/controller/podupgrade/pod_upgrade_controller.go b/pkg/controller/podupgrade/pod_upgrade_controller.go index 4fb84e57508..1b879f88f6e 100644 --- a/pkg/controller/podupgrade/pod_upgrade_controller.go +++ b/pkg/controller/podupgrade/pod_upgrade_controller.go @@ -19,12 +19,14 @@ package podupgrade import ( "context" "fmt" + "sync" "time" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilerrors "k8s.io/apimachinery/pkg/util/errors" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" appsinformers "k8s.io/client-go/informers/apps/v1" @@ -36,9 +38,18 @@ import ( "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" - k8sutil "github.com/openyurtio/openyurt/pkg/util/kubernetes/controller" + k8sutil "github.com/openyurtio/openyurt/pkg/controller/podupgrade/kubernetes/util" ) +const ( + // BurstReplicas is a rate limiter for booting pods on a lot of pods. + // The value of 250 is chosen b/c values that are too high can cause registry DoS issues. + BurstReplicas = 250 +) + +// controllerKind contains the schema.GroupVersionKind for this controller type. +var controllerKind = appsv1.SchemeGroupVersion.WithKind("DaemonSet") + type Controller struct { kubeclientset client.Interface @@ -50,6 +61,8 @@ type Controller struct { podSynced cache.InformerSynced daemonsetWorkqueue workqueue.Interface + + expectations k8sutil.ControllerExpectationsInterface } func NewController(kc client.Interface, daemonsetInformer appsinformers.DaemonSetInformer, @@ -67,6 +80,7 @@ func NewController(kc client.Interface, daemonsetInformer appsinformers.DaemonSe podSynced: podInformer.Informer().HasSynced, daemonsetWorkqueue: workqueue.New(), + expectations: k8sutil.NewControllerExpectations(), } daemonsetInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ @@ -79,42 +93,100 @@ func NewController(kc client.Interface, daemonsetInformer appsinformers.DaemonSe return } - // Only control daemonset with modification - if newDS.ResourceVersion == oldDS.ResourceVersion || - (k8sutil.ComputeHash(&newDS.Spec.Template, newDS.Status.CollisionCount) == - k8sutil.ComputeHash(&oldDS.Spec.Template, oldDS.Status.CollisionCount)) { - return - } - - var key string - var err error - if key, err = cache.MetaNamespaceKeyFunc(newDS); err != nil { - utilruntime.HandleError(err) + if newDS.ResourceVersion == oldDS.ResourceVersion || oldDS.Status.CurrentNumberScheduled != newDS.Status.DesiredNumberScheduled { return } - klog.Infof("Got daemonset udpate event: %v", key) - ctrl.daemonsetWorkqueue.Add(key) + klog.V(5).Infof("Got daemonset udpate event: %v", newDS.Name) + ctrl.enqueueDaemonSet(newDS) }, }) - nodeInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ - UpdateFunc: func(old, new interface{}) { - oldNode := old.(*corev1.Node) - newNode := new.(*corev1.Node) - if !NodeReady(&oldNode.Status) && NodeReady(&newNode.Status) { - klog.Infof("Node %v turn to ready", newNode.Name) - - if err := ctrl.enqueueDaemonsetWhenNodeReady(newNode); err != nil { - klog.Errorf("Enqueue daemonset failed when node %v turns ready, %v", newNode.Name, err) - return - } - } - }, + // Watch for deletion of pods. The reason we watch is that we don't want a daemon set to delete + // more pods until all the effects (expectations) of a daemon set's delete have been observed. + podInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + DeleteFunc: ctrl.deletePod, }) return &ctrl } +func (c *Controller) enqueueDaemonSet(ds *appsv1.DaemonSet) { + key, err := cache.MetaNamespaceKeyFunc(ds) + if err != nil { + utilruntime.HandleError(fmt.Errorf("Couldn't get key for object %#v: %v", ds, err)) + return + } + + klog.V(5).Infof("Daemonset %v queued", key) + c.daemonsetWorkqueue.Add(key) +} + +func (c *Controller) deletePod(obj interface{}) { + pod, ok := obj.(*corev1.Pod) + // When a delete is dropped, the relist will notice a pod in the store not + // in the list, leading to the insertion of a tombstone object which contains + // the deleted key/value. Note that this value might be stale. If the pod + // changed labels the new daemonset will not be woken up till the periodic + // resync. + + if !ok { + tombstone, ok := obj.(cache.DeletedFinalStateUnknown) + if !ok { + utilruntime.HandleError(fmt.Errorf("couldn't get object from tombstone %#v", obj)) + return + } + pod, ok = tombstone.Obj.(*corev1.Pod) + if !ok { + utilruntime.HandleError(fmt.Errorf("tombstone contained object that is not a pod %#v", obj)) + return + } + } + + klog.V(5).Infof("Daemonset pod %s deleted.", pod.Name) + + controllerRef := metav1.GetControllerOf(pod) + if controllerRef == nil { + // No controller should care about orphans being deleted. + return + } + ds := c.resolveControllerRef(pod.Namespace, controllerRef) + if ds == nil { + return + } + + // Only care daemonset meets prerequisites + if !checkPrerequisites(ds) { + return + } + dsKey, err := cache.MetaNamespaceKeyFunc(ds) + if err != nil { + return + } + + c.expectations.DeletionObserved(dsKey) +} + +// resolveControllerRef returns the controller referenced by a ControllerRef, +// or nil if the ControllerRef could not be resolved to a matching controller +// of the correct Kind. +func (c *Controller) resolveControllerRef(namespace string, controllerRef *metav1.OwnerReference) *appsv1.DaemonSet { + // We can't look up by UID, so look up by Name and then verify UID. + // Don't even try to look up by Name if it's the wrong Kind. + if controllerRef.Kind != controllerKind.Kind { + return nil + } + ds, err := c.daemonsetLister.DaemonSets(namespace).Get(controllerRef.Name) + if err != nil { + return nil + } + if ds.UID != controllerRef.UID { + // The controller we found with this Name is not the same one that the + // ControllerRef points to. + return nil + } + return ds +} + // enqueueDaemonsetWhenNodeReady add daemonset key to daemonsetWorkqueue for further handling func (c *Controller) enqueueDaemonsetWhenNodeReady(node *corev1.Node) error { // TODO: Question-> is source lister up-to-date? @@ -201,11 +273,22 @@ func (c *Controller) syncDaemonsetHandler(key string) error { ds, err := c.daemonsetLister.DaemonSets(namespace).Get(name) if err != nil { if apierrors.IsNotFound(err) { + c.expectations.DeleteExpectations(key) return nil } return err } + if ds.DeletionTimestamp != nil { + return nil + } + + // Only process daemonset that meets expectations + // Otherwise, wait native daemonset controller reconciling + if !c.expectations.SatisfiedExpectations(key) { + return nil + } + pods, err := GetDaemonsetPods(c.podLister, ds) if err != nil { return err @@ -219,18 +302,15 @@ func (c *Controller) syncDaemonsetHandler(key string) error { switch v { case OTAUpgrade: - klog.Infof("OTA upgrade %v", ds.Name) if err := c.checkOTAUpgrade(ds, pods); err != nil { return err } case AutoUpgrade: - klog.Infof("Auto upgrade %v", ds.Name) - if err := c.autoUpgrade(ds, pods); err != nil { + if err := c.autoUpgrade(ds); err != nil { return err } default: - // error return fmt.Errorf("unknown annotation type %v", v) } @@ -249,29 +329,156 @@ func (c *Controller) checkOTAUpgrade(ds *appsv1.DaemonSet, pods []*corev1.Pod) e return nil } -// autoUpgrade perform pod upgrade operation when -// 1. pod is upgradable (using IsDaemonsetPodLatest to check) -// 2. pod node is ready -func (c *Controller) autoUpgrade(ds *appsv1.DaemonSet, pods []*corev1.Pod) error { - for _, pod := range pods { - latestOK, err := IsDaemonsetPodLatest(ds, pod) +// autoUpgrade identifies the set of old pods to delete +func (c *Controller) autoUpgrade(ds *appsv1.DaemonSet) error { + nodeToDaemonPods, err := c.getNodesToDaemonPods(ds) + if err != nil { + return fmt.Errorf("couldn't get node to daemon pod mapping for daemon set %q: %v", ds.Name, err) + } + + // TODO(hxc) + // calculate maxUnavailable specified by user, default is xxx?? + // maxUnavailable := xxx + maxUnavailable := 1 + + var numUnavailable int + var allowedReplacementPods []string + var candidatePodsToDelete []string + + for nodeName, pods := range nodeToDaemonPods { + // check if node is ready, ignore not-ready node + ready, err := NodeReadyByName(c.nodeLister, nodeName) if err != nil { - return err + return fmt.Errorf("couldn't check node %q ready status, %v", nodeName, err) + } + if !ready { + continue + } + + newPod, oldPod, ok := findUpdatedPodsOnNode(ds, pods) + if !ok { + // let the manage loop clean up this node, and treat it as an unavailable node + klog.V(3).Infof("DaemonSet %s/%s has excess pods on node %s, skipping to allow the core loop to process", ds.Namespace, ds.Name, nodeName) + numUnavailable++ + continue + } + switch { + case oldPod == nil && newPod == nil, oldPod != nil && newPod != nil: + // the manage loop will handle creating or deleting the appropriate pod, consider this unavailable + numUnavailable++ + case newPod != nil: + // this pod is up to date, check its availability + if !k8sutil.IsPodAvailable(newPod, ds.Spec.MinReadySeconds, metav1.Time{Time: time.Now()}) { + // an unavailable new pod is counted against maxUnavailable + numUnavailable++ + } + default: + // this pod is old, it is an update candidate + switch { + case !k8sutil.IsPodAvailable(oldPod, ds.Spec.MinReadySeconds, metav1.Time{Time: time.Now()}): + // the old pod isn't available, so it needs to be replaced + klog.V(5).Infof("DaemonSet %s/%s pod %s on node %s is out of date and not available, allowing replacement", ds.Namespace, ds.Name, oldPod.Name, nodeName) + // record the replacement + if allowedReplacementPods == nil { + allowedReplacementPods = make([]string, 0, len(nodeToDaemonPods)) + } + allowedReplacementPods = append(allowedReplacementPods, oldPod.Name) + case numUnavailable >= maxUnavailable: + // no point considering any other candidates + continue + default: + klog.V(5).Infof("DaemonSet %s/%s pod %s on node %s is out of date, this is a candidate to replace", ds.Namespace, ds.Name, oldPod.Name, nodeName) + // record the candidate + if candidatePodsToDelete == nil { + candidatePodsToDelete = make([]string, 0, maxUnavailable) + } + candidatePodsToDelete = append(candidatePodsToDelete, oldPod.Name) + } } + } + // use any of the candidates we can, including the allowedReplacemnntPods + klog.V(5).Infof("DaemonSet %s/%s allowing %d replacements, up to %d unavailable, %d are unavailable, %d candidates", ds.Namespace, ds.Name, len(allowedReplacementPods), maxUnavailable, numUnavailable, len(candidatePodsToDelete)) + remainingUnavailable := maxUnavailable - numUnavailable + if remainingUnavailable < 0 { + remainingUnavailable = 0 + } + if max := len(candidatePodsToDelete); remainingUnavailable > max { + remainingUnavailable = max + } + oldPodsToDelete := append(allowedReplacementPods, candidatePodsToDelete[:remainingUnavailable]...) - nodeOK, err := NodeReadyByName(c.nodeLister, pod.Spec.NodeName) + return c.syncPodsOnNodes(ds, oldPodsToDelete) +} + +func (c *Controller) getNodesToDaemonPods(ds *appsv1.DaemonSet) (map[string][]*corev1.Pod, error) { + // TODO(hxc): ignore adopt/orphan pod, just deal with pods in podLister + pods, err := GetDaemonsetPods(c.podLister, ds) + if err != nil { + return nil, err + } + + // Group Pods by Node name. + nodeToDaemonPods := make(map[string][]*corev1.Pod) + for _, pod := range pods { + nodeName, err := k8sutil.GetTargetNodeName(pod) if err != nil { - return err + klog.Warningf("Failed to get target node name of Pod %v/%v in DaemonSet %v/%v", + pod.Namespace, pod.Name, ds.Namespace, ds.Name) + continue } - if !latestOK && nodeOK { - if err := c.kubeclientset.CoreV1().Pods(pod.Namespace). - Delete(context.TODO(), pod.Name, metav1.DeleteOptions{}); err != nil { - return err + nodeToDaemonPods[nodeName] = append(nodeToDaemonPods[nodeName], pod) + } + + return nodeToDaemonPods, nil +} + +// syncPodsOnNodes deletes pods on the given nodes +// returns slice with errors if any +func (c *Controller) syncPodsOnNodes(ds *appsv1.DaemonSet, podsToDelete []string) error { + // We need to set expectations before deleting pods to avoid race conditions. + dsKey, err := cache.MetaNamespaceKeyFunc(ds) + if err != nil { + return fmt.Errorf("couldn't get key for object %#v: %v", ds, err) + } + + deleteDiff := len(podsToDelete) + + if deleteDiff > BurstReplicas { + deleteDiff = BurstReplicas + } + + c.expectations.SetExpectations(dsKey, 0, deleteDiff) + + // error channel to communicate back failures, make the buffer big enough to avoid any blocking + errCh := make(chan error, deleteDiff) + + // delete pods process + klog.V(4).Infof("Pods to delete for daemon set %s: %+v, deleting %d", ds.Name, podsToDelete, deleteDiff) + deleteWait := sync.WaitGroup{} + deleteWait.Add(deleteDiff) + for i := 0; i < deleteDiff; i++ { + go func(ix int) { + defer deleteWait.Done() + if err := c.kubeclientset.CoreV1().Pods(ds.Namespace). + Delete(context.TODO(), podsToDelete[ix], metav1.DeleteOptions{}); err != nil { + c.expectations.DeletionObserved(dsKey) + if !apierrors.IsNotFound(err) { + klog.V(2).Infof("Failed deletion, decremented expectations for set %q/%q", ds.Namespace, ds.Name) + errCh <- err + utilruntime.HandleError(err) + } } - klog.Infof("Auto upgrade pod %v/%v", ds.Name, pod.Name) - } + klog.Infof("Auto upgrade pod %v/%v", ds.Name, podsToDelete[ix]) + }(i) } + deleteWait.Wait() - return nil + // collect errors if any for proper reporting/retry logic in the controller + errors := []error{} + close(errCh) + for err := range errCh { + errors = append(errors, err) + } + return utilerrors.NewAggregate(errors) } diff --git a/pkg/controller/podupgrade/util.go b/pkg/controller/podupgrade/util.go index 55fe78346e7..c6cd7e1b2e8 100644 --- a/pkg/controller/podupgrade/util.go +++ b/pkg/controller/podupgrade/util.go @@ -30,7 +30,7 @@ import ( corelisters "k8s.io/client-go/listers/core/v1" "k8s.io/klog/v2" - k8sutil "github.com/openyurtio/openyurt/pkg/util/kubernetes/controller" + k8sutil "github.com/openyurtio/openyurt/pkg/controller/podupgrade/kubernetes/util" ) var DaemonSet = "DaemonSet" @@ -100,12 +100,12 @@ func GetNodePods(podLister corelisters.PodLister, node *corev1.Node) ([]*corev1. // IsDaemonsetPodLatest check whether pod is latest by comparing its Spec with daemonset's // If pod is latest, return true, otherwise return false -func IsDaemonsetPodLatest(ds *appsv1.DaemonSet, pod *corev1.Pod) (bool, error) { +func IsDaemonsetPodLatest(ds *appsv1.DaemonSet, pod *corev1.Pod) bool { hash := k8sutil.ComputeHash(&ds.Spec.Template, ds.Status.CollisionCount) klog.V(4).Infof("compute hash: %v", hash) generation, err := GetTemplateGeneration(ds) if err != nil { - return false, err + generation = nil } klog.V(5).Infof("daemonset %v revision hash is %v", ds.Name, hash) @@ -113,7 +113,7 @@ func IsDaemonsetPodLatest(ds *appsv1.DaemonSet, pod *corev1.Pod) (bool, error) { templateMatches := generation != nil && pod.Labels[extensions.DaemonSetTemplateGenerationKey] == fmt.Sprint(generation) hashMatches := len(hash) > 0 && pod.Labels[extensions.DefaultDaemonSetUniqueLabelKey] == hash - return hashMatches || templateMatches, nil + return hashMatches || templateMatches } // GetTemplateGeneration get annotation "deprecated.daemonset.template.generation" of the given daemonset @@ -151,10 +151,7 @@ func NodeReady(nodeStatus *corev1.NodeStatus) bool { // SetPodUpgradeAnnotation calculate and set annotation "apps.openyurt.io/pod-upgradable" to pod func SetPodUpgradeAnnotation(clientset client.Interface, ds *appsv1.DaemonSet, pod *corev1.Pod) error { - ok, err := IsDaemonsetPodLatest(ds, pod) - if err != nil { - return err - } + ok := IsDaemonsetPodLatest(ds, pod) var res bool if !ok { @@ -196,3 +193,28 @@ func CloneAndAddLabel(labels map[string]string, labelKey, labelValue string) map newLabels[labelKey] = labelValue return newLabels } + +// findUpdatedPodsOnNode looks at non-deleted pods on a given node and returns true if there +// is at most one of each old and new pods, or false if there are multiples. We can skip +// processing the particular node in those scenarios and let the manage loop prune the +// excess pods for our next time around. +func findUpdatedPodsOnNode(ds *appsv1.DaemonSet, podsOnNode []*corev1.Pod) (newPod, oldPod *corev1.Pod, ok bool) { + for _, pod := range podsOnNode { + if pod.DeletionTimestamp != nil { + continue + } + + if IsDaemonsetPodLatest(ds, pod) { + if newPod != nil { + return nil, nil, false + } + newPod = pod + } else { + if oldPod != nil { + return nil, nil, false + } + oldPod = pod + } + } + return newPod, oldPod, true +} diff --git a/pkg/controller/podupgrade/util_test.go b/pkg/controller/podupgrade/util_test.go index f63f9a275ac..c1343e0ba8b 100644 --- a/pkg/controller/podupgrade/util_test.go +++ b/pkg/controller/podupgrade/util_test.go @@ -27,24 +27,6 @@ import ( "k8s.io/client-go/kubernetes/fake" ) -func TestGetNodePods(t *testing.T) { - node := newNode("node1") - pod1 := newPod("test-pod1", "node1", simpleDaemonSetLabel, nil) - pod2 := newPod("test-pod2", "node2", simpleDaemonSetLabel, nil) - - expectPods := []*corev1.Pod{pod1} - clientset := fake.NewSimpleClientset(node, pod1, pod2) - podInformer := informers.NewSharedInformerFactory(clientset, 0) - - podInformer.Core().V1().Pods().Informer().GetIndexer().Add(pod1) - podInformer.Core().V1().Pods().Informer().GetIndexer().Add(pod2) - - gotPods, err := GetNodePods(podInformer.Core().V1().Pods().Lister(), node) - - assert.Equal(t, nil, err) - assert.Equal(t, expectPods, gotPods) -} - func TestGetDaemonsetPods(t *testing.T) { ds1 := newDaemonSet("daemosnet1", "foo/bar:v1") @@ -90,7 +72,7 @@ func TestIsDaemonsetPodLatest(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gotLatest, _ := IsDaemonsetPodLatest(tt.ds, tt.pod) + gotLatest := IsDaemonsetPodLatest(tt.ds, tt.pod) assert.Equal(t, tt.wantLatest, gotLatest) }) } From b31933e4cb6d5150466f20d1d65366ea98f42f80 Mon Sep 17 00:00:00 2001 From: hxcGit Date: Wed, 7 Sep 2022 17:23:29 +0800 Subject: [PATCH 05/11] add unit tests case Signed-off-by: hxcGit --- cmd/yurt-controller-manager/app/core.go | 4 +- .../kubernetes}/controller_util.go | 5 +- .../kubernetes}/daemonset_util.go | 2 +- .../kubernetes}/pod_util.go | 2 +- .../pod_updater_controller.go} | 106 +++-- .../pod_updater_controller_test.go | 445 ++++++++++++++++++ .../util.go | 43 +- .../util_test.go | 2 +- .../podupgrade/pod_upgrade_controller_test.go | 335 ------------- 9 files changed, 516 insertions(+), 428 deletions(-) rename pkg/controller/{podupgrade/kubernetes/util => podupdatercontroller/kubernetes}/controller_util.go (99%) rename pkg/controller/{podupgrade/kubernetes/util => podupdatercontroller/kubernetes}/daemonset_util.go (99%) rename pkg/controller/{podupgrade/kubernetes/util => podupdatercontroller/kubernetes}/pod_util.go (99%) rename pkg/controller/{podupgrade/pod_upgrade_controller.go => podupdatercontroller/pod_updater_controller.go} (84%) create mode 100644 pkg/controller/podupdatercontroller/pod_updater_controller_test.go rename pkg/controller/{podupgrade => podupdatercontroller}/util.go (80%) rename pkg/controller/{podupgrade => podupdatercontroller}/util_test.go (99%) delete mode 100644 pkg/controller/podupgrade/pod_upgrade_controller_test.go diff --git a/cmd/yurt-controller-manager/app/core.go b/cmd/yurt-controller-manager/app/core.go index 6932783bcbe..88eadd68b97 100644 --- a/cmd/yurt-controller-manager/app/core.go +++ b/cmd/yurt-controller-manager/app/core.go @@ -27,7 +27,7 @@ import ( "github.com/openyurtio/openyurt/pkg/controller/certificates" lifecyclecontroller "github.com/openyurtio/openyurt/pkg/controller/nodelifecycle" - podupgradecontroller "github.com/openyurtio/openyurt/pkg/controller/podupgrade" + podupdater "github.com/openyurtio/openyurt/pkg/controller/podupdater" ) func startNodeLifecycleController(ctx ControllerContext) (http.Handler, bool, error) { @@ -68,7 +68,7 @@ func startYurtCSRApproverController(ctx ControllerContext) (http.Handler, bool, } func startPodUpgradeController(ctx ControllerContext) (http.Handler, bool, error) { - podUpgradeCtrl := podupgradecontroller.NewController( + podUpgradeCtrl := podupdater.NewController( ctx.ClientBuilder.ClientOrDie("podUpgrade-controller"), ctx.InformerFactory.Apps().V1().DaemonSets(), ctx.InformerFactory.Core().V1().Nodes(), diff --git a/pkg/controller/podupgrade/kubernetes/util/controller_util.go b/pkg/controller/podupdatercontroller/kubernetes/controller_util.go similarity index 99% rename from pkg/controller/podupgrade/kubernetes/util/controller_util.go rename to pkg/controller/podupdatercontroller/kubernetes/controller_util.go index 655aa0688b8..574d3e0c518 100644 --- a/pkg/controller/podupgrade/kubernetes/util/controller_util.go +++ b/pkg/controller/podupdatercontroller/kubernetes/controller_util.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package util +package kubernetes import ( "context" @@ -41,9 +41,8 @@ import ( clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" - "k8s.io/utils/clock" - "k8s.io/klog/v2" + "k8s.io/utils/clock" ) const ( diff --git a/pkg/controller/podupgrade/kubernetes/util/daemonset_util.go b/pkg/controller/podupdatercontroller/kubernetes/daemonset_util.go similarity index 99% rename from pkg/controller/podupgrade/kubernetes/util/daemonset_util.go rename to pkg/controller/podupdatercontroller/kubernetes/daemonset_util.go index 75e7300ee76..a7f4630da9f 100644 --- a/pkg/controller/podupgrade/kubernetes/util/daemonset_util.go +++ b/pkg/controller/podupdatercontroller/kubernetes/daemonset_util.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package util +package kubernetes import ( "fmt" diff --git a/pkg/controller/podupgrade/kubernetes/util/pod_util.go b/pkg/controller/podupdatercontroller/kubernetes/pod_util.go similarity index 99% rename from pkg/controller/podupgrade/kubernetes/util/pod_util.go rename to pkg/controller/podupdatercontroller/kubernetes/pod_util.go index 73d123501ca..dd50ab3bfaa 100644 --- a/pkg/controller/podupgrade/kubernetes/util/pod_util.go +++ b/pkg/controller/podupdatercontroller/kubernetes/pod_util.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package util +package kubernetes import ( "time" diff --git a/pkg/controller/podupgrade/pod_upgrade_controller.go b/pkg/controller/podupdatercontroller/pod_updater_controller.go similarity index 84% rename from pkg/controller/podupgrade/pod_upgrade_controller.go rename to pkg/controller/podupdatercontroller/pod_updater_controller.go index 1b879f88f6e..cce13bb3baa 100644 --- a/pkg/controller/podupgrade/pod_upgrade_controller.go +++ b/pkg/controller/podupdatercontroller/pod_updater_controller.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package podupgrade +package podupdater import ( "context" @@ -27,20 +27,40 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" + intstrutil "k8s.io/apimachinery/pkg/util/intstr" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" appsinformers "k8s.io/client-go/informers/apps/v1" coreinformers "k8s.io/client-go/informers/core/v1" client "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/scheme" + v1core "k8s.io/client-go/kubernetes/typed/core/v1" appslisters "k8s.io/client-go/listers/apps/v1" corelisters "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" + "k8s.io/client-go/tools/record" "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" - k8sutil "github.com/openyurtio/openyurt/pkg/controller/podupgrade/kubernetes/util" + k8sutil "github.com/openyurtio/openyurt/pkg/controller/podupdater/kubernetes" ) +const ( + // UpgradeAnnotation is the annotation key used in daemonset spec to indicate + // which upgrade strategy is selected. Currently "ota" and "auto" are supported. + UpgradeAnnotation = "apps.openyurt.io/upgrade-strategy" + + OTAUpgrade = "ota" + AutoUpgrade = "auto" +) + +// PodUpgradableAnnotation is the annotation key added to pods to indicate +// whether a new version is available for upgrade. +// This annotation will only be added if the upgrade strategy is "apps.openyurt.io/upgrade-strategy":"ota". +const PodUpgradableAnnotation = "apps.openyurt.io/pod-upgradable" + +const MaxUnavailableAnnotation = "apps.openyurt.io/max-unavailable" + const ( // BurstReplicas is a rate limiter for booting pods on a lot of pods. // The value of 250 is chosen b/c values that are too high can cause registry DoS issues. @@ -52,6 +72,7 @@ var controllerKind = appsv1.SchemeGroupVersion.WithKind("DaemonSet") type Controller struct { kubeclientset client.Interface + podControl k8sutil.PodControlInterface daemonsetLister appslisters.DaemonSetLister daemonsetSynced cache.InformerSynced @@ -68,8 +89,17 @@ type Controller struct { func NewController(kc client.Interface, daemonsetInformer appsinformers.DaemonSetInformer, nodeInformer coreinformers.NodeInformer, podInformer coreinformers.PodInformer) *Controller { + eventBroadcaster := record.NewBroadcaster() + eventBroadcaster.StartStructuredLogging(0) + eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: kc.CoreV1().Events("")}) + ctrl := Controller{ - kubeclientset: kc, + kubeclientset: kc, + podControl: k8sutil.RealPodControl{ + KubeClient: kc, + Recorder: eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: "podUpdater"}), + }, + daemonsetLister: daemonsetInformer.Lister(), daemonsetSynced: daemonsetInformer.Informer().HasSynced, @@ -187,41 +217,6 @@ func (c *Controller) resolveControllerRef(namespace string, controllerRef *metav return ds } -// enqueueDaemonsetWhenNodeReady add daemonset key to daemonsetWorkqueue for further handling -func (c *Controller) enqueueDaemonsetWhenNodeReady(node *corev1.Node) error { - // TODO: Question-> is source lister up-to-date? - pods, err := GetNodePods(c.podLister, node) - if err != nil { - return err - } - - for _, pod := range pods { - owner := metav1.GetControllerOf(pod) - switch owner.Kind { - case DaemonSet: - ds, err := c.daemonsetLister.DaemonSets(pod.Namespace).Get(owner.Name) - if err != nil { - if apierrors.IsNotFound(err) { - continue - } - return err - } - - if checkPrerequisites(ds) { - var key string - if key, err = cache.MetaNamespaceKeyFunc(ds); err != nil { - return err - } - - c.daemonsetWorkqueue.Add(key) - } - default: - continue - } - } - return nil -} - func (c *Controller) Run(threadiness int, stopCh <-chan struct{}) { defer utilruntime.HandleCrash() @@ -336,10 +331,11 @@ func (c *Controller) autoUpgrade(ds *appsv1.DaemonSet) error { return fmt.Errorf("couldn't get node to daemon pod mapping for daemon set %q: %v", ds.Name, err) } - // TODO(hxc) - // calculate maxUnavailable specified by user, default is xxx?? - // maxUnavailable := xxx - maxUnavailable := 1 + // TODO(hxc): calculate maxUnavailable specified by user, default is 1 + maxUnavailable, err := c.maxUnavailableCounts(ds, nodeToDaemonPods) + if err != nil { + return fmt.Errorf("couldn't get maxUnavailable number for daemon set %q: %v", ds.Name, err) + } var numUnavailable int var allowedReplacementPods []string @@ -460,8 +456,7 @@ func (c *Controller) syncPodsOnNodes(ds *appsv1.DaemonSet, podsToDelete []string for i := 0; i < deleteDiff; i++ { go func(ix int) { defer deleteWait.Done() - if err := c.kubeclientset.CoreV1().Pods(ds.Namespace). - Delete(context.TODO(), podsToDelete[ix], metav1.DeleteOptions{}); err != nil { + if err := c.podControl.DeletePod(context.TODO(), ds.Namespace, podsToDelete[ix], ds); err != nil { c.expectations.DeletionObserved(dsKey) if !apierrors.IsNotFound(err) { klog.V(2).Infof("Failed deletion, decremented expectations for set %q/%q", ds.Namespace, ds.Name) @@ -482,3 +477,26 @@ func (c *Controller) syncPodsOnNodes(ds *appsv1.DaemonSet, podsToDelete []string } return utilerrors.NewAggregate(errors) } + +// maxUnavailableCounts calculates the true number of allowed unavailable +func (c *Controller) maxUnavailableCounts(ds *appsv1.DaemonSet, nodeToDaemonPods map[string][]*corev1.Pod) (int, error) { + // if annotation is not set, use default value one + v, ok := ds.Annotations[MaxUnavailableAnnotation] + if !ok { + return 1, nil + } + + intstrv := intstrutil.Parse(v) + maxUnavailable, err := intstrutil.GetScaledValueFromIntOrPercent(&intstrv, len(nodeToDaemonPods), true) + if err != nil { + return -1, fmt.Errorf("invalid value for MaxUnavailable: %v", err) + } + + // if the daemonset returned with an impossible configuration, obey the default of unavailable=1 + if maxUnavailable == 0 { + klog.Warningf("DaemonSet %s/%s is not configured for unavailability, defaulting to accepting unavailability", ds.Namespace, ds.Name) + maxUnavailable = 1 + } + klog.V(5).Infof("DaemonSet %s/%s, maxUnavailable: %d", ds.Namespace, ds.Name, maxUnavailable) + return maxUnavailable, nil +} diff --git a/pkg/controller/podupdatercontroller/pod_updater_controller_test.go b/pkg/controller/podupdatercontroller/pod_updater_controller_test.go new file mode 100644 index 00000000000..f656c375ab1 --- /dev/null +++ b/pkg/controller/podupdatercontroller/pod_updater_controller_test.go @@ -0,0 +1,445 @@ +/* +Copyright 2022 The OpenYurt 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 podupdater + +import ( + "context" + "fmt" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/uuid" + "k8s.io/apiserver/pkg/storage/names" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/cache" + "k8s.io/client-go/tools/record" + + k8sutil "github.com/openyurtio/openyurt/pkg/controller/podupdater/kubernetes" +) + +var ( + simpleDaemonSetLabel = map[string]string{"foo": "bar"} + alwaysReady = func() bool { return true } +) + +// ---------------------------------------------------------------------------------------------------------------- +// ----------------------------------------------------new Object-------------------------------------------------- +// ---------------------------------------------------------------------------------------------------------------- + +func newDaemonSet(name string, img string) *appsv1.DaemonSet { + two := int32(2) + return &appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{ + UID: uuid.NewUUID(), + Name: name, + Namespace: metav1.NamespaceDefault, + }, + Spec: appsv1.DaemonSetSpec{ + RevisionHistoryLimit: &two, + // UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ + // Type: appsv1.OnDeleteDaemonSetStrategyType, + // }, + Selector: &metav1.LabelSelector{MatchLabels: simpleDaemonSetLabel}, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: simpleDaemonSetLabel, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: img}}, + }, + }, + }, + } +} + +func newPod(podName string, nodeName string, label map[string]string, ds *appsv1.DaemonSet) *corev1.Pod { + // Add hash unique label to the pod + newLabels := label + var podSpec corev1.PodSpec + // Copy pod spec from DaemonSet template, or use a default one if DaemonSet is nil + if ds != nil { + hash := k8sutil.ComputeHash(&ds.Spec.Template, ds.Status.CollisionCount) + newLabels = CloneAndAddLabel(label, appsv1.DefaultDaemonSetUniqueLabelKey, hash) + podSpec = ds.Spec.Template.Spec + } else { + podSpec = corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "foo/bar", + TerminationMessagePath: corev1.TerminationMessagePathDefault, + ImagePullPolicy: corev1.PullIfNotPresent, + }, + }, + } + } + + // Add node name to the pod + if len(nodeName) > 0 { + podSpec.NodeName = nodeName + } + + pod := &corev1.Pod{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + GenerateName: podName, + Labels: newLabels, + Namespace: metav1.NamespaceDefault, + }, + Spec: podSpec, + } + pod.Name = names.SimpleNameGenerator.GenerateName(podName) + if ds != nil { + pod.OwnerReferences = []metav1.OwnerReference{*metav1.NewControllerRef(ds, controllerKind)} + } + return pod +} + +func newNode(name string, ready bool) *corev1.Node { + cond := corev1.NodeCondition{ + Type: corev1.NodeReady, + Status: corev1.ConditionTrue, + } + if !ready { + cond.Status = corev1.ConditionFalse + } + + return &corev1.Node{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: metav1.NamespaceNone, + }, + Status: corev1.NodeStatus{ + Conditions: []corev1.NodeCondition{ + cond, + }, + Allocatable: corev1.ResourceList{ + corev1.ResourcePods: resource.MustParse("100"), + }, + }, + } +} + +// func newNotReadyNode(name string) *corev1.Node { +// return &corev1.Node{ +// TypeMeta: metav1.TypeMeta{APIVersion: "v1"}, +// ObjectMeta: metav1.ObjectMeta{ +// Name: name, +// Namespace: metav1.NamespaceNone, +// }, +// Status: corev1.NodeStatus{ +// Conditions: []corev1.NodeCondition{ +// {Type: corev1.NodeReady, Status: corev1.ConditionFalse}, +// }, +// Allocatable: corev1.ResourceList{ +// corev1.ResourcePods: resource.MustParse("100"), +// }, +// }, +// } +// } + +// ---------------------------------------------------------------------------------------------------------------- +// --------------------------------------------------fakeController------------------------------------------------ +// ---------------------------------------------------------------------------------------------------------------- +type fakeController struct { + *Controller + + dsStore cache.Store + nodeStore cache.Store + podStore cache.Store + + fakeRecorder *record.FakeRecorder +} + +// ---------------------------------------------------------------------------------------------------------------- +// --------------------------------------------------fakePodControl------------------------------------------------ +// ---------------------------------------------------------------------------------------------------------------- +type fakePodControl struct { + sync.Mutex + *k8sutil.FakePodControl + podStore cache.Store + podIDMap map[string]*v1.Pod + expectations k8sutil.ControllerExpectationsInterface +} + +func newFakePodControl() *fakePodControl { + podIDMap := make(map[string]*v1.Pod) + return &fakePodControl{ + FakePodControl: &k8sutil.FakePodControl{}, + podIDMap: podIDMap, + } +} + +func (f *fakePodControl) DeletePod(ctx context.Context, namespace string, podID string, object runtime.Object) error { + f.Lock() + defer f.Unlock() + if err := f.FakePodControl.DeletePod(ctx, namespace, podID, object); err != nil { + return fmt.Errorf("failed to delete pod %q", podID) + } + pod, ok := f.podIDMap[podID] + if !ok { + return fmt.Errorf("pod %q does not exist", podID) + } + f.podStore.Delete(pod) + delete(f.podIDMap, podID) + + ds := object.(*appsv1.DaemonSet) + dsKey, _ := cache.MetaNamespaceKeyFunc(ds) + f.expectations.DeletionObserved(dsKey) + + return nil +} + +func newTest(initialObjests ...runtime.Object) (*fakeController, *fakePodControl) { + clientset := fake.NewSimpleClientset(initialObjests...) + informerFactory := informers.NewSharedInformerFactory(clientset, 0) + + c := NewController( + clientset, + informerFactory.Apps().V1().DaemonSets(), + informerFactory.Core().V1().Nodes(), + informerFactory.Core().V1().Pods(), + ) + + fakeRecorder := record.NewFakeRecorder(100) + + c.daemonsetSynced = alwaysReady + c.nodeSynced = alwaysReady + c.podSynced = alwaysReady + + podControl := newFakePodControl() + c.podControl = podControl + podControl.podStore = informerFactory.Core().V1().Pods().Informer().GetStore() + + fakeCtrl := &fakeController{ + c, + informerFactory.Apps().V1().DaemonSets().Informer().GetStore(), + informerFactory.Core().V1().Nodes().Informer().GetStore(), + informerFactory.Core().V1().Pods().Informer().GetStore(), + fakeRecorder, + } + + podControl.expectations = c.expectations + return fakeCtrl, podControl +} + +// ---------------------------------------------------------------------------------------------------------------- +// --------------------------------------------------Expectations-------------------------------------------------- +// ---------------------------------------------------------------------------------------------------------------- + +func expectSyncDaemonSets(t *testing.T, fakeCtrl *fakeController, ds *appsv1.DaemonSet, podControl *fakePodControl, expectedDeletes int) error { + // t.Helper() + key, err := cache.MetaNamespaceKeyFunc(ds) + if err != nil { + return err + } + + err = fakeCtrl.syncDaemonsetHandler(key) + if err != nil { + return err + } + + err = validateSyncDaemonSets(fakeCtrl, podControl, expectedDeletes) + if err != nil { + return err + } + return nil +} + +// clearExpectations copies the FakePodControl to PodStore and clears the delete expectations. +// func clearExpectations(t *testing.T, fakeCtrl *fakeController, ds *appsv1.DaemonSet, fakePodControl *fakePodControl) { +// fakePodControl.Clear() + +// key, err := cache.MetaNamespaceKeyFunc(ds) +// if err != nil { +// t.Errorf("Could not get key for daemon.") +// return +// } +// fakeCtrl.expectations.DeleteExpectations(key) +// } + +// ---------------------------------------------------------------------------------------------------------------- +// -------------------------------------------------------util----------------------------------------------------- +// ---------------------------------------------------------------------------------------------------------------- + +func setAutoUpgradeAnnotation(ds *appsv1.DaemonSet) { + metav1.SetMetaDataAnnotation(&ds.ObjectMeta, UpgradeAnnotation, AutoUpgrade) +} + +func setOnDelete(ds *appsv1.DaemonSet) { + ds.Spec.UpdateStrategy = appsv1.DaemonSetUpdateStrategy{ + Type: appsv1.OnDeleteDaemonSetStrategyType, + } +} + +// validateSyncDaemonSets check whether the number of deleted pod and events meet expectations +func validateSyncDaemonSets(fakeCtrl *fakeController, fakePodControl *fakePodControl, expectedDeletes int) error { + if len(fakePodControl.DeletePodName) != expectedDeletes { + return fmt.Errorf("Unexpected number of deletes. Expected %d, got %v\n", expectedDeletes, fakePodControl.DeletePodName) + } + return nil +} + +func addNodesWithPods(f *fakePodControl, nodeStore cache.Store, podStore cache.Store, + startIndex, numNodes int, ds *appsv1.DaemonSet, ready bool) ([]*corev1.Node, error) { + nodes := make([]*corev1.Node, 0) + + for i := startIndex; i < startIndex+numNodes; i++ { + var nodeName string + switch ready { + case true: + nodeName = fmt.Sprintf("node-ready-%d", i) + case false: + nodeName = fmt.Sprintf("node-not-ready-%d", i) + + } + + node := newNode(nodeName, ready) + err := nodeStore.Add(node) + if err != nil { + return nil, err + } + nodes = append(nodes, node) + + podName := fmt.Sprintf("pod-%d", i) + pod := newPod(podName, nodeName, simpleDaemonSetLabel, ds) + err = podStore.Add(pod) + if err != nil { + return nil, err + } + f.podIDMap[pod.Name] = pod + } + return nodes, nil +} + +// ---------------------------------------------------------------------------------------------------------------- +// ----------------------------------------------------Test Cases-------------------------------------------------- +// ---------------------------------------------------------------------------------------------------------------- + +type tCase struct { + name string + onDelete bool + strategy string + nodeNum int + readyNodeNum int + maxUnavailable int + turnReady bool +} + +// DaemonSets should place onto NotReady nodes +func TestNotReadyNodeDaemonDoesLaunchPod(t *testing.T) { + + tcases := []tCase{ + { + name: "success", + onDelete: true, + strategy: "auto", + nodeNum: 3, + readyNodeNum: 3, + maxUnavailable: 1, + turnReady: false, + }, + { + name: "success with 1 node not-ready", + onDelete: true, + strategy: "auto", + nodeNum: 3, + readyNodeNum: 2, + maxUnavailable: 1, + turnReady: false, + }, + { + name: "success with 2 nodes not-ready", + onDelete: true, + strategy: "auto", + nodeNum: 3, + readyNodeNum: 1, + maxUnavailable: 1, + turnReady: false, + }, + { + name: "success with 2 nodes not-ready, then turn ready", + onDelete: true, + strategy: "auto", + nodeNum: 3, + readyNodeNum: 1, + maxUnavailable: 1, + turnReady: true, + }, + } + + for _, tcase := range tcases { + t.Log(tcase.name) + ds := newDaemonSet("ds", "foo/bar:v1") + if tcase.onDelete { + setOnDelete(ds) + } + switch tcase.strategy { + case AutoUpgrade: + setAutoUpgradeAnnotation(ds) + } + + fakeCtrl, podControl := newTest(ds) + + // add ready nodes and its pods + _, err := addNodesWithPods(podControl, fakeCtrl.nodeStore, + fakeCtrl.podStore, 1, tcase.readyNodeNum, ds, true) + if err != nil { + t.Fatal(err) + } + + // add not-ready nodes and its pods + notReadyNodes, err := addNodesWithPods(podControl, fakeCtrl.nodeStore, + fakeCtrl.podStore, 1, tcase.nodeNum-tcase.readyNodeNum, ds, false) + if err != nil { + t.Fatal(err) + } + + ds.Spec.Template.Spec.Containers[0].Image = "foo/bar:v2" + + err = fakeCtrl.dsStore.Add(ds) + if err != nil { + t.Fatal(err) + } + + err = expectSyncDaemonSets(t, fakeCtrl, ds, podControl, tcase.readyNodeNum) + assert.Equal(t, nil, err) + + if tcase.turnReady { + fakeCtrl.podControl.(*fakePodControl).Clear() + for _, node := range notReadyNodes { + node.Status.Conditions = []corev1.NodeCondition{ + {Type: corev1.NodeReady, Status: corev1.ConditionTrue}, + } + if err := fakeCtrl.nodeStore.Update(node); err != nil { + t.Fatal(err) + } + } + + err = expectSyncDaemonSets(t, fakeCtrl, ds, podControl, tcase.nodeNum-tcase.readyNodeNum) + assert.Equal(t, nil, err) + } + } +} diff --git a/pkg/controller/podupgrade/util.go b/pkg/controller/podupdatercontroller/util.go similarity index 80% rename from pkg/controller/podupgrade/util.go rename to pkg/controller/podupdatercontroller/util.go index c6cd7e1b2e8..c3952ddba9c 100644 --- a/pkg/controller/podupgrade/util.go +++ b/pkg/controller/podupdatercontroller/util.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package podupgrade +package podupdater import ( "context" @@ -30,25 +30,9 @@ import ( corelisters "k8s.io/client-go/listers/core/v1" "k8s.io/klog/v2" - k8sutil "github.com/openyurtio/openyurt/pkg/controller/podupgrade/kubernetes/util" + k8sutil "github.com/openyurtio/openyurt/pkg/controller/podupdater/kubernetes" ) -var DaemonSet = "DaemonSet" - -const ( - // UpgradeAnnotation is the annotation key used in daemonset spec to indicate - // which upgrade strategy is selected. Currently "ota" and "auto" are supported. - UpgradeAnnotation = "apps.openyurt.io/upgrade-strategy" - - OTAUpgrade = "ota" - AutoUpgrade = "auto" -) - -// PodUpgradableAnnotation is the annotation key added to pods to indicate -// whether a new version is available for upgrade. -// This annotation will only be added if the upgrade strategy is "apps.openyurt.io/upgrade-strategy":"ota". -const PodUpgradableAnnotation = "apps.openyurt.io/pod-upgradable" - // GetDaemonsetPods get all pods belong to the given daemonset func GetDaemonsetPods(podLister corelisters.PodLister, ds *appsv1.DaemonSet) ([]*corev1.Pod, error) { dsPods := make([]*corev1.Pod, 0) @@ -75,29 +59,6 @@ func GetDaemonsetPods(podLister corelisters.PodLister, ds *appsv1.DaemonSet) ([] return dsPods, nil } -// GetDaemonsetPods get all pods belong to the given daemonset -func GetNodePods(podLister corelisters.PodLister, node *corev1.Node) ([]*corev1.Pod, error) { - nodePods := make([]*corev1.Pod, 0) - nodePodsNames := make([]string, 0) - - pods, err := podLister.List(labels.Everything()) - if err != nil { - return nil, err - } - - for i, pod := range pods { - if pod.Spec.NodeName == node.Name { - nodePods = append(nodePods, pods[i]) - nodePodsNames = append(nodePodsNames, pod.Name) - } - } - - if len(nodePodsNames) > 0 { - klog.V(5).Infof("Node %v has pods %v", node.Name, nodePodsNames) - } - return nodePods, nil -} - // IsDaemonsetPodLatest check whether pod is latest by comparing its Spec with daemonset's // If pod is latest, return true, otherwise return false func IsDaemonsetPodLatest(ds *appsv1.DaemonSet, pod *corev1.Pod) bool { diff --git a/pkg/controller/podupgrade/util_test.go b/pkg/controller/podupdatercontroller/util_test.go similarity index 99% rename from pkg/controller/podupgrade/util_test.go rename to pkg/controller/podupdatercontroller/util_test.go index c1343e0ba8b..a18b197ebb5 100644 --- a/pkg/controller/podupgrade/util_test.go +++ b/pkg/controller/podupdatercontroller/util_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package podupgrade +package podupdater import ( "testing" diff --git a/pkg/controller/podupgrade/pod_upgrade_controller_test.go b/pkg/controller/podupgrade/pod_upgrade_controller_test.go deleted file mode 100644 index 7bde83979e1..00000000000 --- a/pkg/controller/podupgrade/pod_upgrade_controller_test.go +++ /dev/null @@ -1,335 +0,0 @@ -/* -Copyright 2022 The OpenYurt 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 podupgrade - -import ( - "context" - "reflect" - "testing" - - "github.com/stretchr/testify/assert" - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/uuid" - "k8s.io/apiserver/pkg/storage/names" - "k8s.io/client-go/informers" - "k8s.io/client-go/kubernetes/fake" - ktest "k8s.io/client-go/testing" - "k8s.io/client-go/tools/cache" - - k8sutil "github.com/openyurtio/openyurt/pkg/util/kubernetes/controller" -) - -var ( - simpleDaemonSetLabel = map[string]string{"foo": "bar"} -) - -var controllerKind = appsv1.SchemeGroupVersion.WithKind("DaemonSet") - -var ( - SyncDaemonset = "SyncDaemosnet" - SyncNode = "SyncNode" -) - -type fixture struct { - t *testing.T - client *fake.Clientset - podLister []*corev1.Pod - nodeLister []*corev1.Node - daemonsetLister []*appsv1.DaemonSet - actions []ktest.Action - objects []runtime.Object -} - -func newFixture(t *testing.T) *fixture { - return &fixture{ - t: t, - objects: []runtime.Object{}, - } -} - -func (f *fixture) newController() (*Controller, informers.SharedInformerFactory) { - f.client = fake.NewSimpleClientset(f.objects...) - - sharedInformers := informers.NewSharedInformerFactory(f.client, 0) - for _, d := range f.daemonsetLister { - sharedInformers.Apps().V1().DaemonSets().Informer().GetIndexer().Add(d) - } - for _, n := range f.nodeLister { - sharedInformers.Core().V1().Nodes().Informer().GetIndexer().Add(n) - } - for _, p := range f.podLister { - sharedInformers.Core().V1().Pods().Informer().GetIndexer().Add(p) - } - - c := NewController(f.client, sharedInformers.Apps().V1().DaemonSets(), sharedInformers.Core().V1().Nodes(), - sharedInformers.Core().V1().Pods()) - - return c, sharedInformers -} - -// run execute the controller logic -func (f *fixture) run(key string) { - f.testController(key, false) -} - -func (f *fixture) runExpectError(key string, kind string) { - f.testController(key, true) -} - -func (f *fixture) testController(key string, expectError bool) { - c, sharedInformers := f.newController() - - stopCh := make(chan struct{}) - defer close(stopCh) - sharedInformers.Start(stopCh) - - err := c.syncDaemonsetHandler(key) - - if !expectError && err != nil { - f.t.Errorf("error syncing: %v", err) - } else if expectError && err == nil { - f.t.Error("expected error syncing, got nil") - - } - - // make sure all the expected action occurred during controller sync process - for _, action := range f.actions { - findAndCheckAction(action, f.client.Actions(), f.t) - } -} - -// findAndCheckAction search all the given actions to find whether the expected action exists -func findAndCheckAction(expected ktest.Action, actions []ktest.Action, t *testing.T) { - for _, action := range actions { - if checkAction(expected, action) { - t.Logf("Check action %+v success", expected) - return - } - } - t.Errorf("Expected action %+v does not occur", expected) -} - -// checkAction verifies that expected and actual actions are equal -func checkAction(expected, actual ktest.Action) bool { - if !(expected.Matches(actual.GetVerb(), actual.GetResource().Resource)) || - reflect.TypeOf(actual) != reflect.TypeOf(expected) || - actual.GetSubresource() != expected.GetSubresource() { - return false - } - - switch a := actual.(type) { - case ktest.DeleteAction: - e, _ := expected.(ktest.DeleteActionImpl) - expName := e.GetName() - actualName := a.GetName() - if expName != actualName { - return false - } - } - - return true -} - -func (f *fixture) expectDeletePodAction(p *corev1.Pod) { - action := ktest.NewDeleteAction(schema.GroupVersionResource{Resource: "pods"}, p.Namespace, p.Name) - f.actions = append(f.actions, action) -} - -func newDaemonSet(name string, img string) *appsv1.DaemonSet { - two := int32(2) - return &appsv1.DaemonSet{ - ObjectMeta: metav1.ObjectMeta{ - UID: uuid.NewUUID(), - Name: name, - Namespace: metav1.NamespaceDefault, - }, - Spec: appsv1.DaemonSetSpec{ - RevisionHistoryLimit: &two, - UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ - Type: appsv1.OnDeleteDaemonSetStrategyType, - }, - Selector: &metav1.LabelSelector{MatchLabels: simpleDaemonSetLabel}, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: simpleDaemonSetLabel, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{{Image: img}}, - }, - }, - }, - } -} - -func newPod(podName string, nodeName string, label map[string]string, ds *appsv1.DaemonSet) *corev1.Pod { - // Add hash unique label to the pod - newLabels := label - var podSpec corev1.PodSpec - // Copy pod spec from DaemonSet template, or use a default one if DaemonSet is nil - if ds != nil { - hash := k8sutil.ComputeHash(&ds.Spec.Template, ds.Status.CollisionCount) - newLabels = CloneAndAddLabel(label, appsv1.DefaultDaemonSetUniqueLabelKey, hash) - podSpec = ds.Spec.Template.Spec - } else { - podSpec = corev1.PodSpec{ - Containers: []corev1.Container{ - { - Image: "foo/bar", - TerminationMessagePath: corev1.TerminationMessagePathDefault, - ImagePullPolicy: corev1.PullIfNotPresent, - }, - }, - } - } - - // Add node name to the pod - if len(nodeName) > 0 { - podSpec.NodeName = nodeName - } - - pod := &corev1.Pod{ - TypeMeta: metav1.TypeMeta{APIVersion: "v1"}, - ObjectMeta: metav1.ObjectMeta{ - GenerateName: podName, - Labels: newLabels, - Namespace: metav1.NamespaceDefault, - }, - Spec: podSpec, - } - pod.Name = names.SimpleNameGenerator.GenerateName(podName) - if ds != nil { - pod.OwnerReferences = []metav1.OwnerReference{*metav1.NewControllerRef(ds, controllerKind)} - } - return pod -} - -func newNode(name string) *corev1.Node { - return &corev1.Node{ - TypeMeta: metav1.TypeMeta{APIVersion: "v1"}, - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: metav1.NamespaceNone, - }, - Status: corev1.NodeStatus{ - Conditions: []corev1.NodeCondition{ - {Type: corev1.NodeReady, Status: corev1.ConditionTrue}, - }, - Allocatable: corev1.ResourceList{ - corev1.ResourcePods: resource.MustParse("100"), - }, - }, - } -} - -func setAutoUpgradeAnnotation(ds *appsv1.DaemonSet, annV string) { - metav1.SetMetaDataAnnotation(&ds.ObjectMeta, UpgradeAnnotation, annV) -} - -func getDaemonsetKey(ds *appsv1.DaemonSet, t *testing.T) string { - key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(ds) - if err != nil { - t.Errorf("Unexpected error getting key for foo %v: %v", ds.Name, err) - return "" - } - return key -} - -func TestAutoUpgradeWithDaemonsetUpdate(t *testing.T) { - f := newFixture(t) - ds := newDaemonSet("test-ds", "foo/bar:v1") - setAutoUpgradeAnnotation(ds, "auto") - node := newNode("test-node") - pod := newPod("test-pod", "test-node", simpleDaemonSetLabel, ds) - - ds.Spec.Template.Spec.Containers[0].Image = "foo/bar:v2" - - f.daemonsetLister = append(f.daemonsetLister, ds) - f.nodeLister = append(f.nodeLister, node) - f.podLister = append(f.podLister, pod) - - f.objects = append(f.objects, ds, pod, node) - - f.expectDeletePodAction(pod) - - f.run(getDaemonsetKey(ds, t)) -} - -func TestOTAUpgrade(t *testing.T) { - f := newFixture(t) - ds := newDaemonSet("test-ds", "foo/bar:v1") - setAutoUpgradeAnnotation(ds, "ota") - oldPod := newPod("old-pod", "test-node", simpleDaemonSetLabel, ds) - - ds.Spec.Template.Spec.Containers[0].Image = "foo/bar:v2" - newPod := newPod("new-pod", "test-node", simpleDaemonSetLabel, ds) - - f.daemonsetLister = append(f.daemonsetLister, ds) - f.podLister = append(f.podLister, oldPod, newPod) - f.objects = append(f.objects, ds, oldPod, newPod) - - f.run(getDaemonsetKey(ds, t)) - - // check whether ota upgradable annotation set properly - oldPodGot, err := f.client.CoreV1().Pods(ds.Namespace).Get(context.TODO(), oldPod.Name, metav1.GetOptions{}) - if err != nil { - t.Errorf("get oldPod failed, %+v", err) - } - - newPodGot, err := f.client.CoreV1().Pods(ds.Namespace).Get(context.TODO(), newPod.Name, metav1.GetOptions{}) - if err != nil { - t.Errorf("get newPod failed, %+v", err) - } - - annOldPodGot, oldPodOK := oldPodGot.Annotations[PodUpgradableAnnotation] - assert.Equal(t, true, oldPodOK) - assert.Equal(t, "true", annOldPodGot) - - annNewPodGot, newPodOK := newPodGot.Annotations[PodUpgradableAnnotation] - assert.Equal(t, true, newPodOK) - assert.Equal(t, "false", annNewPodGot) -} - -func TestController_enqueueDaemonsetWhenNodeReady(t *testing.T) { - f := newFixture(t) - node := newNode("node") - ds1 := newDaemonSet("ds1", "foo/bar:v1") - pod1 := newPod("pod1", "node", simpleDaemonSetLabel, ds1) - pod2 := newPod("pod2", "node", simpleDaemonSetLabel, ds1) - - ds2 := newDaemonSet("ds2", "foo/bar:v2") - pod3 := newPod("pod3", "node", simpleDaemonSetLabel, ds2) - pod4 := newPod("pod4", "node", simpleDaemonSetLabel, ds2) - - setAutoUpgradeAnnotation(ds1, "ota") - setAutoUpgradeAnnotation(ds2, "auto") - - f.daemonsetLister = append(f.daemonsetLister, ds1, ds2) - f.podLister = append(f.podLister, pod1, pod2, pod3, pod4) - f.objects = append(f.objects, node, ds1, ds2, pod1, pod2, pod3, pod4) - - c, _ := f.newController() - - c.enqueueDaemonsetWhenNodeReady(node) - - // enqueue "default/ds1" and "default/ds2" - assert.Equal(t, 2, c.daemonsetWorkqueue.Len()) -} From 22e2fef6a12efd66a0ec1c428b7ba9b07602595c Mon Sep 17 00:00:00 2001 From: hxcGit Date: Wed, 7 Sep 2022 20:11:58 +0800 Subject: [PATCH 06/11] fix upper case Signed-off-by: hxcGit --- .../kubernetes/controller_util.go | 0 .../kubernetes/daemonset_util.go | 0 .../{podupdatercontroller => podupdater}/kubernetes/pod_util.go | 0 .../pod_updater_controller.go | 0 .../pod_updater_controller_test.go | 0 pkg/controller/{podupdatercontroller => podupdater}/util.go | 0 pkg/controller/{podupdatercontroller => podupdater}/util_test.go | 0 7 files changed, 0 insertions(+), 0 deletions(-) rename pkg/controller/{podupdatercontroller => podupdater}/kubernetes/controller_util.go (100%) rename pkg/controller/{podupdatercontroller => podupdater}/kubernetes/daemonset_util.go (100%) rename pkg/controller/{podupdatercontroller => podupdater}/kubernetes/pod_util.go (100%) rename pkg/controller/{podupdatercontroller => podupdater}/pod_updater_controller.go (100%) rename pkg/controller/{podupdatercontroller => podupdater}/pod_updater_controller_test.go (100%) rename pkg/controller/{podupdatercontroller => podupdater}/util.go (100%) rename pkg/controller/{podupdatercontroller => podupdater}/util_test.go (100%) diff --git a/pkg/controller/podupdatercontroller/kubernetes/controller_util.go b/pkg/controller/podupdater/kubernetes/controller_util.go similarity index 100% rename from pkg/controller/podupdatercontroller/kubernetes/controller_util.go rename to pkg/controller/podupdater/kubernetes/controller_util.go diff --git a/pkg/controller/podupdatercontroller/kubernetes/daemonset_util.go b/pkg/controller/podupdater/kubernetes/daemonset_util.go similarity index 100% rename from pkg/controller/podupdatercontroller/kubernetes/daemonset_util.go rename to pkg/controller/podupdater/kubernetes/daemonset_util.go diff --git a/pkg/controller/podupdatercontroller/kubernetes/pod_util.go b/pkg/controller/podupdater/kubernetes/pod_util.go similarity index 100% rename from pkg/controller/podupdatercontroller/kubernetes/pod_util.go rename to pkg/controller/podupdater/kubernetes/pod_util.go diff --git a/pkg/controller/podupdatercontroller/pod_updater_controller.go b/pkg/controller/podupdater/pod_updater_controller.go similarity index 100% rename from pkg/controller/podupdatercontroller/pod_updater_controller.go rename to pkg/controller/podupdater/pod_updater_controller.go diff --git a/pkg/controller/podupdatercontroller/pod_updater_controller_test.go b/pkg/controller/podupdater/pod_updater_controller_test.go similarity index 100% rename from pkg/controller/podupdatercontroller/pod_updater_controller_test.go rename to pkg/controller/podupdater/pod_updater_controller_test.go diff --git a/pkg/controller/podupdatercontroller/util.go b/pkg/controller/podupdater/util.go similarity index 100% rename from pkg/controller/podupdatercontroller/util.go rename to pkg/controller/podupdater/util.go diff --git a/pkg/controller/podupdatercontroller/util_test.go b/pkg/controller/podupdater/util_test.go similarity index 100% rename from pkg/controller/podupdatercontroller/util_test.go rename to pkg/controller/podupdater/util_test.go From 16979d8d10c87f6ba36ce07511e2391c04af6d10 Mon Sep 17 00:00:00 2001 From: hxcGit Date: Fri, 9 Sep 2022 16:03:54 +0800 Subject: [PATCH 07/11] add unit tests for k8s utils Signed-off-by: hxcGit --- .../app/controllermanager.go | 2 +- cmd/yurt-controller-manager/app/core.go | 8 +- .../controller-manager.go | 3 - .../daemon_pod_updater_controller.go} | 74 ++--- .../daemon_pod_updater_controller_test.go} | 170 ++++++----- .../kubernetes/controller_utils.go} | 46 --- .../kubernetes/controller_utils_test.go | 282 ++++++++++++++++++ .../kubernetes/pod_util.go | 0 .../kubernetes/pod_util_test.go | 80 +++++ .../{podupdater => daemonpodupdater}/util.go | 62 +++- .../util_test.go | 10 +- .../podupdater/kubernetes/daemonset_util.go | 63 ---- 12 files changed, 560 insertions(+), 240 deletions(-) rename pkg/controller/{podupdater/pod_updater_controller.go => daemonpodupdater/daemon_pod_updater_controller.go} (86%) rename pkg/controller/{podupdater/pod_updater_controller_test.go => daemonpodupdater/daemon_pod_updater_controller_test.go} (74%) rename pkg/controller/{podupdater/kubernetes/controller_util.go => daemonpodupdater/kubernetes/controller_utils.go} (93%) create mode 100644 pkg/controller/daemonpodupdater/kubernetes/controller_utils_test.go rename pkg/controller/{podupdater => daemonpodupdater}/kubernetes/pod_util.go (100%) create mode 100644 pkg/controller/daemonpodupdater/kubernetes/pod_util_test.go rename pkg/controller/{podupdater => daemonpodupdater}/util.go (69%) rename pkg/controller/{podupdater => daemonpodupdater}/util_test.go (94%) delete mode 100644 pkg/controller/podupdater/kubernetes/daemonset_util.go diff --git a/cmd/yurt-controller-manager/app/controllermanager.go b/cmd/yurt-controller-manager/app/controllermanager.go index b59fee63747..9fea3d8ca9c 100644 --- a/cmd/yurt-controller-manager/app/controllermanager.go +++ b/cmd/yurt-controller-manager/app/controllermanager.go @@ -305,7 +305,7 @@ func NewControllerInitializers() map[string]InitFunc { controllers := map[string]InitFunc{} controllers["nodelifecycle"] = startNodeLifecycleController controllers["yurtcsrapprover"] = startYurtCSRApproverController - controllers["podupgrade"] = startPodUpgradeController + controllers["daemonpodupdater"] = startDaemonPodUpdaterController return controllers } diff --git a/cmd/yurt-controller-manager/app/core.go b/cmd/yurt-controller-manager/app/core.go index 88eadd68b97..76f344b1d70 100644 --- a/cmd/yurt-controller-manager/app/core.go +++ b/cmd/yurt-controller-manager/app/core.go @@ -26,8 +26,8 @@ import ( "time" "github.com/openyurtio/openyurt/pkg/controller/certificates" + daemonpodupdater "github.com/openyurtio/openyurt/pkg/controller/daemonpodupdater" lifecyclecontroller "github.com/openyurtio/openyurt/pkg/controller/nodelifecycle" - podupdater "github.com/openyurtio/openyurt/pkg/controller/podupdater" ) func startNodeLifecycleController(ctx ControllerContext) (http.Handler, bool, error) { @@ -67,15 +67,15 @@ func startYurtCSRApproverController(ctx ControllerContext) (http.Handler, bool, return nil, true, nil } -func startPodUpgradeController(ctx ControllerContext) (http.Handler, bool, error) { - podUpgradeCtrl := podupdater.NewController( +func startDaemonPodUpdaterController(ctx ControllerContext) (http.Handler, bool, error) { + daemonPodUpdaterCtrl := daemonpodupdater.NewController( ctx.ClientBuilder.ClientOrDie("podUpgrade-controller"), ctx.InformerFactory.Apps().V1().DaemonSets(), ctx.InformerFactory.Core().V1().Nodes(), ctx.InformerFactory.Core().V1().Pods(), ) - go podUpgradeCtrl.Run(2, ctx.Stop) + go daemonPodUpdaterCtrl.Run(2, ctx.Stop) return nil, true, nil } diff --git a/cmd/yurt-controller-manager/controller-manager.go b/cmd/yurt-controller-manager/controller-manager.go index d3f30d5392b..e3b77ddf474 100644 --- a/cmd/yurt-controller-manager/controller-manager.go +++ b/cmd/yurt-controller-manager/controller-manager.go @@ -26,13 +26,10 @@ import ( "time" "k8s.io/component-base/logs" - // for JSON log format registration _ "k8s.io/component-base/logs/json/register" - // load all the prometheus client-go plugin _ "k8s.io/component-base/metrics/prometheus/clientgo" - // for version metric registration _ "k8s.io/component-base/metrics/prometheus/version" diff --git a/pkg/controller/podupdater/pod_updater_controller.go b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go similarity index 86% rename from pkg/controller/podupdater/pod_updater_controller.go rename to pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go index cce13bb3baa..52a2095e242 100644 --- a/pkg/controller/podupdater/pod_updater_controller.go +++ b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package podupdater +package daemonpodupdater import ( "context" @@ -42,26 +42,26 @@ import ( "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" - k8sutil "github.com/openyurtio/openyurt/pkg/controller/podupdater/kubernetes" + k8sutil "github.com/openyurtio/openyurt/pkg/controller/daemonpodupdater/kubernetes" ) const ( - // UpgradeAnnotation is the annotation key used in daemonset spec to indicate - // which upgrade strategy is selected. Currently "ota" and "auto" are supported. - UpgradeAnnotation = "apps.openyurt.io/upgrade-strategy" + // UpdateAnnotation is the annotation key used in daemonset spec to indicate + // which update strategy is selected. Currently, "ota" and "auto" are supported. + UpdateAnnotation = "apps.openyurt.io/update-strategy" - OTAUpgrade = "ota" - AutoUpgrade = "auto" -) + OTAUpdate = "ota" + AutoUpdate = "auto" -// PodUpgradableAnnotation is the annotation key added to pods to indicate -// whether a new version is available for upgrade. -// This annotation will only be added if the upgrade strategy is "apps.openyurt.io/upgrade-strategy":"ota". -const PodUpgradableAnnotation = "apps.openyurt.io/pod-upgradable" + // PodUpdatableAnnotation is the annotation key added to pods to indicate + // whether a new version is available for update. + // This annotation will only be added if the update strategy is "apps.openyurt.io/update-strategy":"ota". + PodUpdatableAnnotation = "apps.openyurt.io/pod-updatable" -const MaxUnavailableAnnotation = "apps.openyurt.io/max-unavailable" + // MaxUnavailableAnnotation is the annotation key added to daemonset to indicate + // the max unavailable pods number. It's used with "apps.openyurt.io/update-strategy=auto". + MaxUnavailableAnnotation = "apps.openyurt.io/max-unavailable" -const ( // BurstReplicas is a rate limiter for booting pods on a lot of pods. // The value of 250 is chosen b/c values that are too high can cause registry DoS issues. BurstReplicas = 250 @@ -143,7 +143,7 @@ func NewController(kc client.Interface, daemonsetInformer appsinformers.DaemonSe func (c *Controller) enqueueDaemonSet(ds *appsv1.DaemonSet) { key, err := cache.MetaNamespaceKeyFunc(ds) if err != nil { - utilruntime.HandleError(fmt.Errorf("Couldn't get key for object %#v: %v", ds, err)) + utilruntime.HandleError(fmt.Errorf("couldn't get key for object %#v: %v", ds, err)) return } @@ -153,7 +153,7 @@ func (c *Controller) enqueueDaemonSet(ds *appsv1.DaemonSet) { func (c *Controller) deletePod(obj interface{}) { pod, ok := obj.(*corev1.Pod) - // When a delete is dropped, the relist will notice a pod in the store not + // When a deletion is dropped, the relist will notice a pod in the store not // in the list, leading to the insertion of a tombstone object which contains // the deleted key/value. Note that this value might be stale. If the pod // changed labels the new daemonset will not be woken up till the periodic @@ -220,14 +220,14 @@ func (c *Controller) resolveControllerRef(namespace string, controllerRef *metav func (c *Controller) Run(threadiness int, stopCh <-chan struct{}) { defer utilruntime.HandleCrash() - klog.Info("Starting pod upgrade controller") - defer klog.Info("Shutting down pod upgrade controller") + klog.Info("Starting daemonPodUpdater controller") + defer klog.Info("Shutting down daemonPodUpdater controller") defer c.daemonsetWorkqueue.ShutDown() //synchronize the cache before starting to process events if !cache.WaitForCacheSync(stopCh, c.daemonsetSynced, c.nodeSynced, c.podSynced) { - klog.Error("sync podupgrade controller timeout") + klog.Error("sync daemonPodUpdater controller timeout") } for i := 0; i < threadiness; i++ { @@ -253,10 +253,10 @@ func (c *Controller) runDaemonsetWorker() { func (c *Controller) syncDaemonsetHandler(key string) error { defer func() { - klog.V(4).Infof("Finish syncing pod upgrade request %q", key) + klog.V(4).Infof("Finish syncing daemonPodUpdater request %q", key) }() - klog.V(4).Infof("Start handler pod upgrade request %q", key) + klog.V(4).Infof("Start handler daemonPodUpdater request %q", key) namespace, name, err := cache.SplitMetaNamespaceKey(key) if err != nil { @@ -290,19 +290,19 @@ func (c *Controller) syncDaemonsetHandler(key string) error { } // recheck required annotation - v, ok := ds.Annotations[UpgradeAnnotation] + v, ok := ds.Annotations[UpdateAnnotation] if !ok { - return fmt.Errorf("won't sync daemonset %q without annotation 'apps.openyurt.io/upgrade-strategy'", ds.Name) + return fmt.Errorf("won't sync daemonset %q without annotation 'apps.openyurt.io/update-strategy'", ds.Name) } switch v { - case OTAUpgrade: - if err := c.checkOTAUpgrade(ds, pods); err != nil { + case OTAUpdate: + if err := c.checkOTAUpdate(ds, pods); err != nil { return err } - case AutoUpgrade: - if err := c.autoUpgrade(ds); err != nil { + case AutoUpdate: + if err := c.autoUpdate(ds); err != nil { return err } default: @@ -312,20 +312,20 @@ func (c *Controller) syncDaemonsetHandler(key string) error { return nil } -// checkOTAUpgrade compare every pod to its owner daemonset to check if pod is upgradable -// If pod is in line with the latest daemonset version, set annotation "apps.openyurt.io/pod-upgradable" to "true" -// while not, set annotation "apps.openyurt.io/pod-upgradable" to "false" -func (c *Controller) checkOTAUpgrade(ds *appsv1.DaemonSet, pods []*corev1.Pod) error { +// checkOTAUpdate compare every pod to its owner daemonset to check if pod is updatable +// If pod is in line with the latest daemonset version, set annotation "apps.openyurt.io/pod-updatable" to "true" +// while not, set annotation "apps.openyurt.io/pod-updatable" to "false" +func (c *Controller) checkOTAUpdate(ds *appsv1.DaemonSet, pods []*corev1.Pod) error { for _, pod := range pods { - if err := SetPodUpgradeAnnotation(c.kubeclientset, ds, pod); err != nil { + if err := SetPodUpdateAnnotation(c.kubeclientset, ds, pod); err != nil { return err } } return nil } -// autoUpgrade identifies the set of old pods to delete -func (c *Controller) autoUpgrade(ds *appsv1.DaemonSet) error { +// autoUpdate identifies the set of old pods to delete +func (c *Controller) autoUpdate(ds *appsv1.DaemonSet) error { nodeToDaemonPods, err := c.getNodesToDaemonPods(ds) if err != nil { return fmt.Errorf("couldn't get node to daemon pod mapping for daemon set %q: %v", ds.Name, err) @@ -363,7 +363,7 @@ func (c *Controller) autoUpgrade(ds *appsv1.DaemonSet) error { // the manage loop will handle creating or deleting the appropriate pod, consider this unavailable numUnavailable++ case newPod != nil: - // this pod is up to date, check its availability + // this pod is up-to-date, check its availability if !k8sutil.IsPodAvailable(newPod, ds.Spec.MinReadySeconds, metav1.Time{Time: time.Now()}) { // an unavailable new pod is counted against maxUnavailable numUnavailable++ @@ -416,7 +416,7 @@ func (c *Controller) getNodesToDaemonPods(ds *appsv1.DaemonSet) (map[string][]*c // Group Pods by Node name. nodeToDaemonPods := make(map[string][]*corev1.Pod) for _, pod := range pods { - nodeName, err := k8sutil.GetTargetNodeName(pod) + nodeName, err := GetTargetNodeName(pod) if err != nil { klog.Warningf("Failed to get target node name of Pod %v/%v in DaemonSet %v/%v", pod.Namespace, pod.Name, ds.Namespace, ds.Name) @@ -464,7 +464,7 @@ func (c *Controller) syncPodsOnNodes(ds *appsv1.DaemonSet, podsToDelete []string utilruntime.HandleError(err) } } - klog.Infof("Auto upgrade pod %v/%v", ds.Name, podsToDelete[ix]) + klog.Infof("Auto update pod %v/%v", ds.Name, podsToDelete[ix]) }(i) } deleteWait.Wait() diff --git a/pkg/controller/podupdater/pod_updater_controller_test.go b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller_test.go similarity index 74% rename from pkg/controller/podupdater/pod_updater_controller_test.go rename to pkg/controller/daemonpodupdater/daemon_pod_updater_controller_test.go index f656c375ab1..fdb73feeafb 100644 --- a/pkg/controller/podupdater/pod_updater_controller_test.go +++ b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package podupdater +package daemonpodupdater import ( "context" @@ -22,21 +22,25 @@ import ( "sync" "testing" - "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + intstrutil "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/apiserver/pkg/storage/names" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/cache" - "k8s.io/client-go/tools/record" - k8sutil "github.com/openyurtio/openyurt/pkg/controller/podupdater/kubernetes" + k8sutil "github.com/openyurtio/openyurt/pkg/controller/daemonpodupdater/kubernetes" +) + +const ( + DefaultMaxUnavailable = "1" + CoupleMaxUnavailable = "2" ) var ( @@ -58,10 +62,7 @@ func newDaemonSet(name string, img string) *appsv1.DaemonSet { }, Spec: appsv1.DaemonSetSpec{ RevisionHistoryLimit: &two, - // UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ - // Type: appsv1.OnDeleteDaemonSetStrategyType, - // }, - Selector: &metav1.LabelSelector{MatchLabels: simpleDaemonSetLabel}, + Selector: &metav1.LabelSelector{MatchLabels: simpleDaemonSetLabel}, Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: simpleDaemonSetLabel, @@ -108,6 +109,14 @@ func newPod(podName string, nodeName string, label map[string]string, ds *appsv1 Namespace: metav1.NamespaceDefault, }, Spec: podSpec, + Status: corev1.PodStatus{ + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }, + }, + }, } pod.Name = names.SimpleNameGenerator.GenerateName(podName) if ds != nil { @@ -142,24 +151,6 @@ func newNode(name string, ready bool) *corev1.Node { } } -// func newNotReadyNode(name string) *corev1.Node { -// return &corev1.Node{ -// TypeMeta: metav1.TypeMeta{APIVersion: "v1"}, -// ObjectMeta: metav1.ObjectMeta{ -// Name: name, -// Namespace: metav1.NamespaceNone, -// }, -// Status: corev1.NodeStatus{ -// Conditions: []corev1.NodeCondition{ -// {Type: corev1.NodeReady, Status: corev1.ConditionFalse}, -// }, -// Allocatable: corev1.ResourceList{ -// corev1.ResourcePods: resource.MustParse("100"), -// }, -// }, -// } -// } - // ---------------------------------------------------------------------------------------------------------------- // --------------------------------------------------fakeController------------------------------------------------ // ---------------------------------------------------------------------------------------------------------------- @@ -169,8 +160,6 @@ type fakeController struct { dsStore cache.Store nodeStore cache.Store podStore cache.Store - - fakeRecorder *record.FakeRecorder } // ---------------------------------------------------------------------------------------------------------------- @@ -223,8 +212,6 @@ func newTest(initialObjests ...runtime.Object) (*fakeController, *fakePodControl informerFactory.Core().V1().Pods(), ) - fakeRecorder := record.NewFakeRecorder(100) - c.daemonsetSynced = alwaysReady c.nodeSynced = alwaysReady c.podSynced = alwaysReady @@ -238,7 +225,6 @@ func newTest(initialObjests ...runtime.Object) (*fakeController, *fakePodControl informerFactory.Apps().V1().DaemonSets().Informer().GetStore(), informerFactory.Core().V1().Nodes().Informer().GetStore(), informerFactory.Core().V1().Pods().Informer().GetStore(), - fakeRecorder, } podControl.expectations = c.expectations @@ -249,23 +235,37 @@ func newTest(initialObjests ...runtime.Object) (*fakeController, *fakePodControl // --------------------------------------------------Expectations-------------------------------------------------- // ---------------------------------------------------------------------------------------------------------------- -func expectSyncDaemonSets(t *testing.T, fakeCtrl *fakeController, ds *appsv1.DaemonSet, podControl *fakePodControl, expectedDeletes int) error { - // t.Helper() +func expectSyncDaemonSets(t *testing.T, tcase tCase, fakeCtrl *fakeController, ds *appsv1.DaemonSet, + podControl *fakePodControl, expectedDeletes int) { key, err := cache.MetaNamespaceKeyFunc(ds) if err != nil { - return err + t.Fatal(err) } - err = fakeCtrl.syncDaemonsetHandler(key) + intstrv := intstrutil.Parse(tcase.maxUnavailable) + maxUnavailable, err := intstrutil.GetScaledValueFromIntOrPercent(&intstrv, tcase.nodeNum, true) if err != nil { - return err + t.Fatal(err) + } + // Execute test case + round := expectedDeletes / maxUnavailable + for round >= 0 { + err = fakeCtrl.syncDaemonsetHandler(key) + if err != nil { + t.Fatalf("Test %q does not passed, got syncDaemonsetHandler error %v", tcase.name, err) + } + round-- + } + + // Validate deleted pods number + if !tcase.wantDelete { + return } - err = validateSyncDaemonSets(fakeCtrl, podControl, expectedDeletes) + err = validateSyncDaemonSets(podControl, expectedDeletes) if err != nil { - return err + t.Fatalf("Test %q does not passed, %v", tcase.name, err) } - return nil } // clearExpectations copies the FakePodControl to PodStore and clears the delete expectations. @@ -284,8 +284,12 @@ func expectSyncDaemonSets(t *testing.T, fakeCtrl *fakeController, ds *appsv1.Dae // -------------------------------------------------------util----------------------------------------------------- // ---------------------------------------------------------------------------------------------------------------- -func setAutoUpgradeAnnotation(ds *appsv1.DaemonSet) { - metav1.SetMetaDataAnnotation(&ds.ObjectMeta, UpgradeAnnotation, AutoUpgrade) +func setAutoUpdateAnnotation(ds *appsv1.DaemonSet) { + metav1.SetMetaDataAnnotation(&ds.ObjectMeta, UpdateAnnotation, AutoUpdate) +} + +func setMaxUnavailableAnnotation(ds *appsv1.DaemonSet, v string) { + metav1.SetMetaDataAnnotation(&ds.ObjectMeta, MaxUnavailableAnnotation, v) } func setOnDelete(ds *appsv1.DaemonSet) { @@ -295,15 +299,14 @@ func setOnDelete(ds *appsv1.DaemonSet) { } // validateSyncDaemonSets check whether the number of deleted pod and events meet expectations -func validateSyncDaemonSets(fakeCtrl *fakeController, fakePodControl *fakePodControl, expectedDeletes int) error { +func validateSyncDaemonSets(fakePodControl *fakePodControl, expectedDeletes int) error { if len(fakePodControl.DeletePodName) != expectedDeletes { return fmt.Errorf("Unexpected number of deletes. Expected %d, got %v\n", expectedDeletes, fakePodControl.DeletePodName) } return nil } -func addNodesWithPods(f *fakePodControl, nodeStore cache.Store, podStore cache.Store, - startIndex, numNodes int, ds *appsv1.DaemonSet, ready bool) ([]*corev1.Node, error) { +func addNodesWithPods(fakeCtrl *fakeController, f *fakePodControl, startIndex, numNodes int, ds *appsv1.DaemonSet, ready bool) ([]*corev1.Node, error) { nodes := make([]*corev1.Node, 0) for i := startIndex; i < startIndex+numNodes; i++ { @@ -313,19 +316,18 @@ func addNodesWithPods(f *fakePodControl, nodeStore cache.Store, podStore cache.S nodeName = fmt.Sprintf("node-ready-%d", i) case false: nodeName = fmt.Sprintf("node-not-ready-%d", i) - } node := newNode(nodeName, ready) - err := nodeStore.Add(node) + err := fakeCtrl.nodeStore.Add(node) if err != nil { return nil, err } nodes = append(nodes, node) - podName := fmt.Sprintf("pod-%d", i) - pod := newPod(podName, nodeName, simpleDaemonSetLabel, ds) - err = podStore.Add(pod) + podPrefix := fmt.Sprintf("pod-%d", i) + pod := newPod(podPrefix, nodeName, simpleDaemonSetLabel, ds) + err = fakeCtrl.podStore.Add(pod) if err != nil { return nil, err } @@ -344,22 +346,54 @@ type tCase struct { strategy string nodeNum int readyNodeNum int - maxUnavailable int + maxUnavailable string turnReady bool + wantDelete bool } // DaemonSets should place onto NotReady nodes -func TestNotReadyNodeDaemonDoesLaunchPod(t *testing.T) { +func TestDaemonsetPodUpdater(t *testing.T) { tcases := []tCase{ + { + name: "failed with not OnDelete strategy", + onDelete: false, + strategy: "auto", + nodeNum: 3, + readyNodeNum: 3, + maxUnavailable: DefaultMaxUnavailable, + turnReady: false, + wantDelete: false, + }, { name: "success", onDelete: true, strategy: "auto", nodeNum: 3, readyNodeNum: 3, - maxUnavailable: 1, + maxUnavailable: DefaultMaxUnavailable, + turnReady: false, + wantDelete: true, + }, + { + name: "success with maxUnavailable is 2", + onDelete: true, + strategy: "auto", + nodeNum: 3, + readyNodeNum: 3, + maxUnavailable: CoupleMaxUnavailable, + turnReady: false, + wantDelete: true, + }, + { + name: "success with maxUnavailable is 50%", + onDelete: true, + strategy: "auto", + nodeNum: 3, + readyNodeNum: 3, + maxUnavailable: "50%", turnReady: false, + wantDelete: true, }, { name: "success with 1 node not-ready", @@ -367,8 +401,9 @@ func TestNotReadyNodeDaemonDoesLaunchPod(t *testing.T) { strategy: "auto", nodeNum: 3, readyNodeNum: 2, - maxUnavailable: 1, + maxUnavailable: DefaultMaxUnavailable, turnReady: false, + wantDelete: true, }, { name: "success with 2 nodes not-ready", @@ -376,8 +411,9 @@ func TestNotReadyNodeDaemonDoesLaunchPod(t *testing.T) { strategy: "auto", nodeNum: 3, readyNodeNum: 1, - maxUnavailable: 1, + maxUnavailable: DefaultMaxUnavailable, turnReady: false, + wantDelete: true, }, { name: "success with 2 nodes not-ready, then turn ready", @@ -385,47 +421,48 @@ func TestNotReadyNodeDaemonDoesLaunchPod(t *testing.T) { strategy: "auto", nodeNum: 3, readyNodeNum: 1, - maxUnavailable: 1, + maxUnavailable: DefaultMaxUnavailable, turnReady: true, + wantDelete: true, }, } for _, tcase := range tcases { - t.Log(tcase.name) + t.Logf("Current test case is %q", tcase.name) ds := newDaemonSet("ds", "foo/bar:v1") if tcase.onDelete { setOnDelete(ds) } + setMaxUnavailableAnnotation(ds, tcase.maxUnavailable) switch tcase.strategy { - case AutoUpgrade: - setAutoUpgradeAnnotation(ds) + case AutoUpdate: + setAutoUpdateAnnotation(ds) } fakeCtrl, podControl := newTest(ds) // add ready nodes and its pods - _, err := addNodesWithPods(podControl, fakeCtrl.nodeStore, - fakeCtrl.podStore, 1, tcase.readyNodeNum, ds, true) + _, err := addNodesWithPods(fakeCtrl, podControl, 1, tcase.readyNodeNum, ds, true) if err != nil { t.Fatal(err) } // add not-ready nodes and its pods - notReadyNodes, err := addNodesWithPods(podControl, fakeCtrl.nodeStore, - fakeCtrl.podStore, 1, tcase.nodeNum-tcase.readyNodeNum, ds, false) + notReadyNodes, err := addNodesWithPods(fakeCtrl, podControl, tcase.readyNodeNum+1, tcase.nodeNum-tcase.readyNodeNum, ds, + false) if err != nil { t.Fatal(err) } + // Update daemonset specification ds.Spec.Template.Spec.Containers[0].Image = "foo/bar:v2" - err = fakeCtrl.dsStore.Add(ds) if err != nil { t.Fatal(err) } - err = expectSyncDaemonSets(t, fakeCtrl, ds, podControl, tcase.readyNodeNum) - assert.Equal(t, nil, err) + // Check test case + expectSyncDaemonSets(t, tcase, fakeCtrl, ds, podControl, tcase.readyNodeNum) if tcase.turnReady { fakeCtrl.podControl.(*fakePodControl).Clear() @@ -438,8 +475,7 @@ func TestNotReadyNodeDaemonDoesLaunchPod(t *testing.T) { } } - err = expectSyncDaemonSets(t, fakeCtrl, ds, podControl, tcase.nodeNum-tcase.readyNodeNum) - assert.Equal(t, nil, err) + expectSyncDaemonSets(t, tcase, fakeCtrl, ds, podControl, tcase.nodeNum-tcase.readyNodeNum) } } } diff --git a/pkg/controller/podupdater/kubernetes/controller_util.go b/pkg/controller/daemonpodupdater/kubernetes/controller_utils.go similarity index 93% rename from pkg/controller/podupdater/kubernetes/controller_util.go rename to pkg/controller/daemonpodupdater/kubernetes/controller_utils.go index 574d3e0c518..42ea916f9c5 100644 --- a/pkg/controller/podupdater/kubernetes/controller_util.go +++ b/pkg/controller/daemonpodupdater/kubernetes/controller_utils.go @@ -37,7 +37,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/rand" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" @@ -57,53 +56,14 @@ const ( // 500 pods. Just creation is limited to 20qps, and watching happens with ~10-30s // latency/pod at the scale of 3000 pods over 100 nodes. ExpectationsTimeout = 5 * time.Minute - // When batching pod creates, SlowStartInitialBatchSize is the size of the - // initial batch. The size of each successive batch is twice the size of - // the previous batch. For example, for a value of 1, batch sizes would be - // 1, 2, 4, 8, ... and for a value of 10, batch sizes would be - // 10, 20, 40, 80, ... Setting the value higher means that quota denials - // will result in more doomed API calls and associated event spam. Setting - // the value lower will result in more API call round trip periods for - // large batches. - // - // Given a number of pods to start "N": - // The number of doomed calls per sync once quota is exceeded is given by: - // min(N,SlowStartInitialBatchSize) - // The number of batches is given by: - // 1+floor(log_2(ceil(N/SlowStartInitialBatchSize))) - SlowStartInitialBatchSize = 1 ) -var UpdateTaintBackoff = wait.Backoff{ - Steps: 5, - Duration: 100 * time.Millisecond, - Jitter: 1.0, -} - -var UpdateLabelBackoff = wait.Backoff{ - Steps: 5, - Duration: 100 * time.Millisecond, - Jitter: 1.0, -} - var ( KeyFunc = cache.DeletionHandlingMetaNamespaceKeyFunc ) type ResyncPeriodFunc func() time.Duration -// Returns 0 for resyncPeriod in case resyncing is not needed. -func NoResyncPeriodFunc() time.Duration { - return 0 -} - -// StaticResyncPeriodFunc returns the resync period specified -func StaticResyncPeriodFunc(resyncPeriod time.Duration) ResyncPeriodFunc { - return func() time.Duration { - return resyncPeriod - } -} - // Expectations are a way for controllers to tell the controller manager what they expect. eg: // ControllerExpectations: { // controller1: expects 2 adds in 2 minutes @@ -367,12 +327,6 @@ func (u *UIDTrackingControllerExpectations) DeleteExpectations(rcKey string) { } } -// NewUIDTrackingControllerExpectations returns a wrapper around -// ControllerExpectations that is aware of deleteKeys. -func NewUIDTrackingControllerExpectations(ce ControllerExpectationsInterface) *UIDTrackingControllerExpectations { - return &UIDTrackingControllerExpectations{ControllerExpectationsInterface: ce, uidStore: cache.NewStore(UIDSetKeyFunc)} -} - // Reasons for pod events const ( // FailedCreatePodReason is added in an event and in a replica set condition diff --git a/pkg/controller/daemonpodupdater/kubernetes/controller_utils_test.go b/pkg/controller/daemonpodupdater/kubernetes/controller_utils_test.go new file mode 100644 index 00000000000..406141f8daf --- /dev/null +++ b/pkg/controller/daemonpodupdater/kubernetes/controller_utils_test.go @@ -0,0 +1,282 @@ +/* +Copyright 2015 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 kubernetes + +import ( + "context" + "encoding/json" + "fmt" + "math" + "net/http/httptest" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" + apiequality "k8s.io/apimachinery/pkg/api/equality" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/clock" + "k8s.io/apimachinery/pkg/util/uuid" + clientset "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/fake" + clientscheme "k8s.io/client-go/kubernetes/scheme" + restclient "k8s.io/client-go/rest" + "k8s.io/client-go/tools/cache" + "k8s.io/client-go/tools/record" + utiltesting "k8s.io/client-go/util/testing" +) + +// NewFakeControllerExpectationsLookup creates a fake store for PodExpectations. +func NewFakeControllerExpectationsLookup(ttl time.Duration) (*ControllerExpectations, *clock.FakeClock) { + fakeTime := time.Date(2009, time.November, 10, 23, 0, 0, 0, time.UTC) + fakeClock := clock.NewFakeClock(fakeTime) + ttlPolicy := &cache.TTLPolicy{TTL: ttl, Clock: fakeClock} + ttlStore := cache.NewFakeExpirationStore( + ExpKeyFunc, nil, ttlPolicy, fakeClock) + return &ControllerExpectations{ttlStore}, fakeClock +} + +func newReplicationController(replicas int) *v1.ReplicationController { + rc := &v1.ReplicationController{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + UID: uuid.NewUUID(), + Name: "foobar", + Namespace: metav1.NamespaceDefault, + ResourceVersion: "18", + }, + Spec: v1.ReplicationControllerSpec{ + Replicas: func() *int32 { i := int32(replicas); return &i }(), + Selector: map[string]string{"foo": "bar"}, + Template: &v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "name": "foo", + "type": "production", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Image: "foo/bar", + TerminationMessagePath: v1.TerminationMessagePathDefault, + ImagePullPolicy: v1.PullIfNotPresent, + }, + }, + RestartPolicy: v1.RestartPolicyAlways, + DNSPolicy: v1.DNSDefault, + NodeSelector: map[string]string{ + "baz": "blah", + }, + }, + }, + }, + } + return rc +} + +func TestControllerExpectations(t *testing.T) { + ttl := 30 * time.Second + e, fakeClock := NewFakeControllerExpectationsLookup(ttl) + // In practice we can't really have add and delete expectations since we only either create or + // delete replicas in one rc pass, and the rc goes to sleep soon after until the expectations are + // either fulfilled or timeout. + adds, dels := 10, 30 + rc := newReplicationController(1) + + // RC fires off adds and deletes at apiserver, then sets expectations + rcKey, err := KeyFunc(rc) + assert.NoError(t, err, "Couldn't get key for object %#v: %v", rc, err) + + e.SetExpectations(rcKey, adds, dels) + var wg sync.WaitGroup + for i := 0; i < adds+1; i++ { + wg.Add(1) + go func() { + // In prod this can happen either because of a failed create by the rc + // or after having observed a create via informer + e.CreationObserved(rcKey) + wg.Done() + }() + } + wg.Wait() + + // There are still delete expectations + assert.False(t, e.SatisfiedExpectations(rcKey), "Rc will sync before expectations are met") + + for i := 0; i < dels+1; i++ { + wg.Add(1) + go func() { + e.DeletionObserved(rcKey) + wg.Done() + }() + } + wg.Wait() + + // Expectations have been surpassed + podExp, exists, err := e.GetExpectations(rcKey) + assert.NoError(t, err, "Could not get expectations for rc, exists %v and err %v", exists, err) + assert.True(t, exists, "Could not get expectations for rc, exists %v and err %v", exists, err) + + add, del := podExp.GetExpectations() + assert.Equal(t, int64(-1), add, "Unexpected pod expectations %#v", podExp) + assert.Equal(t, int64(-1), del, "Unexpected pod expectations %#v", podExp) + assert.True(t, e.SatisfiedExpectations(rcKey), "Expectations are met but the rc will not sync") + + // Next round of rc sync, old expectations are cleared + e.SetExpectations(rcKey, 1, 2) + podExp, exists, err = e.GetExpectations(rcKey) + assert.NoError(t, err, "Could not get expectations for rc, exists %v and err %v", exists, err) + assert.True(t, exists, "Could not get expectations for rc, exists %v and err %v", exists, err) + add, del = podExp.GetExpectations() + + assert.Equal(t, int64(1), add, "Unexpected pod expectations %#v", podExp) + assert.Equal(t, int64(2), del, "Unexpected pod expectations %#v", podExp) + + // Expectations have expired because of ttl + fakeClock.Step(ttl + 1) + assert.True(t, e.SatisfiedExpectations(rcKey), + "Expectations should have expired but didn't") +} + +func TestCreatePods(t *testing.T) { + ns := metav1.NamespaceDefault + body := runtime.EncodeOrDie(clientscheme.Codecs.LegacyCodec(v1.SchemeGroupVersion), &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "empty_pod"}}) + fakeHandler := utiltesting.FakeHandler{ + StatusCode: 200, + ResponseBody: string(body), + } + testServer := httptest.NewServer(&fakeHandler) + defer testServer.Close() + clientset := clientset.NewForConfigOrDie(&restclient.Config{Host: testServer.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}}) + + podControl := RealPodControl{ + KubeClient: clientset, + Recorder: &record.FakeRecorder{}, + } + + controllerSpec := newReplicationController(1) + controllerRef := metav1.NewControllerRef(controllerSpec, v1.SchemeGroupVersion.WithKind("ReplicationController")) + + // Make sure createReplica sends a POST to the apiserver with a pod from the controllers pod template + err := podControl.CreatePods(context.TODO(), ns, controllerSpec.Spec.Template, controllerSpec, controllerRef) + assert.NoError(t, err, "unexpected error: %v", err) + + expectedPod := v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: controllerSpec.Spec.Template.Labels, + GenerateName: fmt.Sprintf("%s-", controllerSpec.Name), + }, + Spec: controllerSpec.Spec.Template.Spec, + } + fakeHandler.ValidateRequest(t, "/api/v1/namespaces/default/pods", "POST", nil) + var actualPod = &v1.Pod{} + err = json.Unmarshal([]byte(fakeHandler.RequestBody), actualPod) + assert.NoError(t, err, "unexpected error: %v", err) + assert.True(t, apiequality.Semantic.DeepDerivative(&expectedPod, actualPod), + "Body: %s", fakeHandler.RequestBody) +} + +func TestCreatePodsWithGenerateName(t *testing.T) { + ns := metav1.NamespaceDefault + body := runtime.EncodeOrDie(clientscheme.Codecs.LegacyCodec(v1.SchemeGroupVersion), &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "empty_pod"}}) + fakeHandler := utiltesting.FakeHandler{ + StatusCode: 200, + ResponseBody: string(body), + } + testServer := httptest.NewServer(&fakeHandler) + defer testServer.Close() + clientset := clientset.NewForConfigOrDie(&restclient.Config{Host: testServer.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}}) + + podControl := RealPodControl{ + KubeClient: clientset, + Recorder: &record.FakeRecorder{}, + } + + controllerSpec := newReplicationController(1) + controllerRef := metav1.NewControllerRef(controllerSpec, v1.SchemeGroupVersion.WithKind("ReplicationController")) + + // Make sure createReplica sends a POST to the apiserver with a pod from the controllers pod template + generateName := "hello-" + err := podControl.CreatePodsWithGenerateName(context.TODO(), ns, controllerSpec.Spec.Template, controllerSpec, controllerRef, generateName) + assert.NoError(t, err, "unexpected error: %v", err) + + expectedPod := v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: controllerSpec.Spec.Template.Labels, + GenerateName: generateName, + OwnerReferences: []metav1.OwnerReference{*controllerRef}, + }, + Spec: controllerSpec.Spec.Template.Spec, + } + + fakeHandler.ValidateRequest(t, "/api/v1/namespaces/default/pods", "POST", nil) + var actualPod = &v1.Pod{} + err = json.Unmarshal([]byte(fakeHandler.RequestBody), actualPod) + assert.NoError(t, err, "unexpected error: %v", err) + assert.True(t, apiequality.Semantic.DeepDerivative(&expectedPod, actualPod), + "Body: %s", fakeHandler.RequestBody) +} + +func TestDeletePodsAllowsMissing(t *testing.T) { + fakeClient := fake.NewSimpleClientset() + podControl := RealPodControl{ + KubeClient: fakeClient, + Recorder: &record.FakeRecorder{}, + } + + controllerSpec := newReplicationController(1) + + err := podControl.DeletePod(context.TODO(), "namespace-name", "podName", controllerSpec) + assert.True(t, apierrors.IsNotFound(err)) +} + +func TestComputeHash(t *testing.T) { + collisionCount := int32(1) + otherCollisionCount := int32(2) + maxCollisionCount := int32(math.MaxInt32) + tests := []struct { + name string + template *v1.PodTemplateSpec + collisionCount *int32 + otherCollisionCount *int32 + }{ + { + name: "simple", + template: &v1.PodTemplateSpec{}, + collisionCount: &collisionCount, + otherCollisionCount: &otherCollisionCount, + }, + { + name: "using math.MaxInt64", + template: &v1.PodTemplateSpec{}, + collisionCount: nil, + otherCollisionCount: &maxCollisionCount, + }, + } + + for _, test := range tests { + hash := ComputeHash(test.template, test.collisionCount) + otherHash := ComputeHash(test.template, test.otherCollisionCount) + + assert.NotEqual(t, hash, otherHash, "expected different hashes but got the same: %d", hash) + } +} diff --git a/pkg/controller/podupdater/kubernetes/pod_util.go b/pkg/controller/daemonpodupdater/kubernetes/pod_util.go similarity index 100% rename from pkg/controller/podupdater/kubernetes/pod_util.go rename to pkg/controller/daemonpodupdater/kubernetes/pod_util.go diff --git a/pkg/controller/daemonpodupdater/kubernetes/pod_util_test.go b/pkg/controller/daemonpodupdater/kubernetes/pod_util_test.go new file mode 100644 index 00000000000..e9d55223cb2 --- /dev/null +++ b/pkg/controller/daemonpodupdater/kubernetes/pod_util_test.go @@ -0,0 +1,80 @@ +/* +Copyright 2015 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 kubernetes + +import ( + "testing" + "time" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func newPod(now metav1.Time, ready bool, beforeSec int) *v1.Pod { + conditionStatus := v1.ConditionFalse + if ready { + conditionStatus = v1.ConditionTrue + } + return &v1.Pod{ + Status: v1.PodStatus{ + Conditions: []v1.PodCondition{ + { + Type: v1.PodReady, + LastTransitionTime: metav1.NewTime(now.Time.Add(-1 * time.Duration(beforeSec) * time.Second)), + Status: conditionStatus, + }, + }, + }, + } +} + +func TestIsPodAvailable(t *testing.T) { + now := metav1.Now() + tests := []struct { + pod *v1.Pod + minReadySeconds int32 + expected bool + }{ + { + pod: newPod(now, false, 0), + minReadySeconds: 0, + expected: false, + }, + { + pod: newPod(now, true, 0), + minReadySeconds: 1, + expected: false, + }, + { + pod: newPod(now, true, 0), + minReadySeconds: 0, + expected: true, + }, + { + pod: newPod(now, true, 51), + minReadySeconds: 50, + expected: true, + }, + } + + for i, test := range tests { + isAvailable := IsPodAvailable(test.pod, test.minReadySeconds, now) + if isAvailable != test.expected { + t.Errorf("[tc #%d] expected available pod: %t, got: %t", i, test.expected, isAvailable) + } + } +} diff --git a/pkg/controller/podupdater/util.go b/pkg/controller/daemonpodupdater/util.go similarity index 69% rename from pkg/controller/podupdater/util.go rename to pkg/controller/daemonpodupdater/util.go index c3952ddba9c..7ee929a8835 100644 --- a/pkg/controller/podupdater/util.go +++ b/pkg/controller/daemonpodupdater/util.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package podupdater +package daemonpodupdater import ( "context" @@ -30,7 +30,7 @@ import ( corelisters "k8s.io/client-go/listers/core/v1" "k8s.io/klog/v2" - k8sutil "github.com/openyurtio/openyurt/pkg/controller/podupdater/kubernetes" + k8sutil "github.com/openyurtio/openyurt/pkg/controller/daemonpodupdater/kubernetes" ) // GetDaemonsetPods get all pods belong to the given daemonset @@ -110,29 +110,24 @@ func NodeReady(nodeStatus *corev1.NodeStatus) bool { return false } -// SetPodUpgradeAnnotation calculate and set annotation "apps.openyurt.io/pod-upgradable" to pod -func SetPodUpgradeAnnotation(clientset client.Interface, ds *appsv1.DaemonSet, pod *corev1.Pod) error { - ok := IsDaemonsetPodLatest(ds, pod) +// SetPodUpdateAnnotation calculate and set annotation "apps.openyurt.io/pod-updatable" to pod +func SetPodUpdateAnnotation(clientset client.Interface, ds *appsv1.DaemonSet, pod *corev1.Pod) error { + isUpdatable := IsDaemonsetPodLatest(ds, pod) - var res bool - if !ok { - res = true - } - - metav1.SetMetaDataAnnotation(&pod.ObjectMeta, PodUpgradableAnnotation, strconv.FormatBool(res)) + metav1.SetMetaDataAnnotation(&pod.ObjectMeta, PodUpdatableAnnotation, strconv.FormatBool(!isUpdatable)) if _, err := clientset.CoreV1().Pods(pod.Namespace).Update(context.TODO(), pod, metav1.UpdateOptions{}); err != nil { return err } - klog.Infof("set pod %q annotation apps.openyurt.io/pod-upgradable to %v", pod.Name, res) + klog.Infof("set pod %q annotation apps.openyurt.io/pod-updatable to %v", pod.Name, !isUpdatable) return nil } // checkPrerequisites checks that daemonset meets two conditions -// 1. annotation "apps.openyurt.io/upgrade-strategy"="auto" or "ota" +// 1. annotation "apps.openyurt.io/update-strategy"="auto" or "ota" // 2. update strategy is "OnDelete" func checkPrerequisites(ds *appsv1.DaemonSet) bool { - v, ok := ds.Annotations[UpgradeAnnotation] + v, ok := ds.Annotations[UpdateAnnotation] if !ok || (v != "auto" && v != "ota") { return false } @@ -179,3 +174,42 @@ func findUpdatedPodsOnNode(ds *appsv1.DaemonSet, podsOnNode []*corev1.Pod) (newP } return newPod, oldPod, true } + +// GetTargetNodeName get the target node name of DaemonSet pods. If `.spec.NodeName` is not empty (nil), +// return `.spec.NodeName`; otherwise, retrieve node name of pending pods from NodeAffinity. Return error +// if failed to retrieve node name from `.spec.NodeName` and NodeAffinity. +func GetTargetNodeName(pod *corev1.Pod) (string, error) { + if len(pod.Spec.NodeName) != 0 { + return pod.Spec.NodeName, nil + } + + // Retrieve node name of unscheduled pods from NodeAffinity + if pod.Spec.Affinity == nil || + pod.Spec.Affinity.NodeAffinity == nil || + pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution == nil { + return "", fmt.Errorf("no spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution for pod %s/%s", + pod.Namespace, pod.Name) + } + + terms := pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms + if len(terms) < 1 { + return "", fmt.Errorf("no nodeSelectorTerms in requiredDuringSchedulingIgnoredDuringExecution of pod %s/%s", + pod.Namespace, pod.Name) + } + + for _, term := range terms { + for _, exp := range term.MatchFields { + if exp.Key == metav1.ObjectNameField && + exp.Operator == corev1.NodeSelectorOpIn { + if len(exp.Values) != 1 { + return "", fmt.Errorf("the matchFields value of '%s' is not unique for pod %s/%s", + metav1.ObjectNameField, pod.Namespace, pod.Name) + } + + return exp.Values[0], nil + } + } + } + + return "", fmt.Errorf("no node name found for pod %s/%s", pod.Namespace, pod.Name) +} diff --git a/pkg/controller/podupdater/util_test.go b/pkg/controller/daemonpodupdater/util_test.go similarity index 94% rename from pkg/controller/podupdater/util_test.go rename to pkg/controller/daemonpodupdater/util_test.go index a18b197ebb5..abd6b9c2098 100644 --- a/pkg/controller/podupdater/util_test.go +++ b/pkg/controller/daemonpodupdater/util_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package podupdater +package daemonpodupdater import ( "testing" @@ -89,7 +89,7 @@ func Test_checkPrerequisites(t *testing.T) { ds: &appsv1.DaemonSet{ ObjectMeta: v1.ObjectMeta{ Annotations: map[string]string{ - "apps.openyurt.io/upgrade-strategy": "ota", + "apps.openyurt.io/update-strategy": "ota", }, }, Spec: appsv1.DaemonSetSpec{ @@ -105,7 +105,7 @@ func Test_checkPrerequisites(t *testing.T) { ds: &appsv1.DaemonSet{ ObjectMeta: v1.ObjectMeta{ Annotations: map[string]string{ - "apps.openyurt.io/upgrade-strategy": "auto", + "apps.openyurt.io/update-strategy": "auto", }, }, Spec: appsv1.DaemonSetSpec{ @@ -121,7 +121,7 @@ func Test_checkPrerequisites(t *testing.T) { ds: &appsv1.DaemonSet{ ObjectMeta: v1.ObjectMeta{ Annotations: map[string]string{ - "apps.openyurt.io/upgrade-strategy": "other", + "apps.openyurt.io/update-strategy": "other", }, }, Spec: appsv1.DaemonSetSpec{ @@ -148,7 +148,7 @@ func Test_checkPrerequisites(t *testing.T) { ds: &appsv1.DaemonSet{ ObjectMeta: v1.ObjectMeta{ Annotations: map[string]string{ - "apps.openyurt.io/upgrade-strategy": "other", + "apps.openyurt.io/update-strategy": "other", }, }, }, diff --git a/pkg/controller/podupdater/kubernetes/daemonset_util.go b/pkg/controller/podupdater/kubernetes/daemonset_util.go deleted file mode 100644 index a7f4630da9f..00000000000 --- a/pkg/controller/podupdater/kubernetes/daemonset_util.go +++ /dev/null @@ -1,63 +0,0 @@ -/* -Copyright 2017 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 kubernetes - -import ( - "fmt" - - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// GetTargetNodeName get the target node name of DaemonSet pods. If `.spec.NodeName` is not empty (nil), -// return `.spec.NodeName`; otherwise, retrieve node name of pending pods from NodeAffinity. Return error -// if failed to retrieve node name from `.spec.NodeName` and NodeAffinity. -func GetTargetNodeName(pod *v1.Pod) (string, error) { - if len(pod.Spec.NodeName) != 0 { - return pod.Spec.NodeName, nil - } - - // Retrieve node name of unscheduled pods from NodeAffinity - if pod.Spec.Affinity == nil || - pod.Spec.Affinity.NodeAffinity == nil || - pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution == nil { - return "", fmt.Errorf("no spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution for pod %s/%s", - pod.Namespace, pod.Name) - } - - terms := pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms - if len(terms) < 1 { - return "", fmt.Errorf("no nodeSelectorTerms in requiredDuringSchedulingIgnoredDuringExecution of pod %s/%s", - pod.Namespace, pod.Name) - } - - for _, term := range terms { - for _, exp := range term.MatchFields { - if exp.Key == metav1.ObjectNameField && - exp.Operator == v1.NodeSelectorOpIn { - if len(exp.Values) != 1 { - return "", fmt.Errorf("the matchFields value of '%s' is not unique for pod %s/%s", - metav1.ObjectNameField, pod.Namespace, pod.Name) - } - - return exp.Values[0], nil - } - } - } - - return "", fmt.Errorf("no node name found for pod %s/%s", pod.Namespace, pod.Name) -} From 605544982a3866b727e1445f3720d9aa534e3c40 Mon Sep 17 00:00:00 2001 From: hxcGit Date: Mon, 12 Sep 2022 20:45:11 +0800 Subject: [PATCH 08/11] add node ready check for ota strategy Signed-off-by: hxcGit Signed-off-by: Xuecheng --- .../controller-manager.go | 3 + .../daemon_pod_updater_controller.go | 137 +++++++++--------- .../daemon_pod_updater_controller_test.go | 69 +++++++-- .../kubernetes/controller_utils.go | 89 ------------ pkg/controller/daemonpodupdater/util.go | 4 +- pkg/controller/daemonpodupdater/util_test.go | 70 ++++++++- .../kubernetes/controller/controller_utils.go | 58 -------- 7 files changed, 194 insertions(+), 236 deletions(-) delete mode 100644 pkg/util/kubernetes/controller/controller_utils.go diff --git a/cmd/yurt-controller-manager/controller-manager.go b/cmd/yurt-controller-manager/controller-manager.go index e3b77ddf474..d3f30d5392b 100644 --- a/cmd/yurt-controller-manager/controller-manager.go +++ b/cmd/yurt-controller-manager/controller-manager.go @@ -26,10 +26,13 @@ import ( "time" "k8s.io/component-base/logs" + // for JSON log format registration _ "k8s.io/component-base/logs/json/register" + // load all the prometheus client-go plugin _ "k8s.io/component-base/metrics/prometheus/clientgo" + // for version metric registration _ "k8s.io/component-base/metrics/prometheus/version" diff --git a/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go index 52a2095e242..357f070c488 100644 --- a/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go +++ b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go @@ -123,7 +123,7 @@ func NewController(kc client.Interface, daemonsetInformer appsinformers.DaemonSe return } - if newDS.ResourceVersion == oldDS.ResourceVersion || oldDS.Status.CurrentNumberScheduled != newDS.Status.DesiredNumberScheduled { + if newDS.ResourceVersion == oldDS.ResourceVersion { return } @@ -158,7 +158,6 @@ func (c *Controller) deletePod(obj interface{}) { // the deleted key/value. Note that this value might be stale. If the pod // changed labels the new daemonset will not be woken up till the periodic // resync. - if !ok { tombstone, ok := obj.(cache.DeletedFinalStateUnknown) if !ok { @@ -196,27 +195,6 @@ func (c *Controller) deletePod(obj interface{}) { c.expectations.DeletionObserved(dsKey) } -// resolveControllerRef returns the controller referenced by a ControllerRef, -// or nil if the ControllerRef could not be resolved to a matching controller -// of the correct Kind. -func (c *Controller) resolveControllerRef(namespace string, controllerRef *metav1.OwnerReference) *appsv1.DaemonSet { - // We can't look up by UID, so look up by Name and then verify UID. - // Don't even try to look up by Name if it's the wrong Kind. - if controllerRef.Kind != controllerKind.Kind { - return nil - } - ds, err := c.daemonsetLister.DaemonSets(namespace).Get(controllerRef.Name) - if err != nil { - return nil - } - if ds.UID != controllerRef.UID { - // The controller we found with this Name is not the same one that the - // ControllerRef points to. - return nil - } - return ds -} - func (c *Controller) Run(threadiness int, stopCh <-chan struct{}) { defer utilruntime.HandleCrash() @@ -224,47 +202,47 @@ func (c *Controller) Run(threadiness int, stopCh <-chan struct{}) { defer klog.Info("Shutting down daemonPodUpdater controller") defer c.daemonsetWorkqueue.ShutDown() - //synchronize the cache before starting to process events + // Synchronize the cache before starting to process events if !cache.WaitForCacheSync(stopCh, c.daemonsetSynced, c.nodeSynced, c.podSynced) { klog.Error("sync daemonPodUpdater controller timeout") } for i := 0; i < threadiness; i++ { - go wait.Until(c.runDaemonsetWorker, time.Second, stopCh) + go wait.Until(c.runWorker, time.Second, stopCh) } <-stopCh } -func (c *Controller) runDaemonsetWorker() { +func (c *Controller) runWorker() { for { obj, shutdown := c.daemonsetWorkqueue.Get() if shutdown { return } - if err := c.syncDaemonsetHandler(obj.(string)); err != nil { + // TODO(hxc): should requeue failed ds + if err := c.syncHandler(obj.(string)); err != nil { utilruntime.HandleError(err) } c.daemonsetWorkqueue.Done(obj) } } -func (c *Controller) syncDaemonsetHandler(key string) error { +func (c *Controller) syncHandler(key string) error { defer func() { klog.V(4).Infof("Finish syncing daemonPodUpdater request %q", key) }() - klog.V(4).Infof("Start handler daemonPodUpdater request %q", key) + klog.V(4).Infof("Start handling daemonPodUpdater request %q", key) namespace, name, err := cache.SplitMetaNamespaceKey(key) if err != nil { - utilruntime.HandleError(fmt.Errorf("invalid resource key: %s", key)) - return nil + return fmt.Errorf("invalid resource key: %s", key) } - // daemonset that need to be synced + // Daemonset that need to be synced ds, err := c.daemonsetLister.DaemonSets(namespace).Get(name) if err != nil { if apierrors.IsNotFound(err) { @@ -284,12 +262,12 @@ func (c *Controller) syncDaemonsetHandler(key string) error { return nil } - pods, err := GetDaemonsetPods(c.podLister, ds) + nodeToDaemonPods, err := c.getNodesToDaemonPods(ds) if err != nil { - return err + return fmt.Errorf("couldn't get node to daemon pod mapping for daemon set %q: %v", ds.Name, err) } - // recheck required annotation + // Recheck required annotation v, ok := ds.Annotations[UpdateAnnotation] if !ok { return fmt.Errorf("won't sync daemonset %q without annotation 'apps.openyurt.io/update-strategy'", ds.Name) @@ -297,12 +275,12 @@ func (c *Controller) syncDaemonsetHandler(key string) error { switch v { case OTAUpdate: - if err := c.checkOTAUpdate(ds, pods); err != nil { + if err := c.checkOTAUpdate(ds, nodeToDaemonPods); err != nil { return err } case AutoUpdate: - if err := c.autoUpdate(ds); err != nil { + if err := c.autoUpdate(ds, nodeToDaemonPods); err != nil { return err } default: @@ -313,25 +291,30 @@ func (c *Controller) syncDaemonsetHandler(key string) error { } // checkOTAUpdate compare every pod to its owner daemonset to check if pod is updatable -// If pod is in line with the latest daemonset version, set annotation "apps.openyurt.io/pod-updatable" to "true" +// If pod is in line with the latest daemonset spec, set annotation "apps.openyurt.io/pod-updatable" to "true" // while not, set annotation "apps.openyurt.io/pod-updatable" to "false" -func (c *Controller) checkOTAUpdate(ds *appsv1.DaemonSet, pods []*corev1.Pod) error { - for _, pod := range pods { - if err := SetPodUpdateAnnotation(c.kubeclientset, ds, pod); err != nil { - return err +func (c *Controller) checkOTAUpdate(ds *appsv1.DaemonSet, nodeToDaemonPods map[string][]*corev1.Pod) error { + for nodeName, pods := range nodeToDaemonPods { + // Check if node is ready, ignore not-ready node + ready, err := NodeReadyByName(c.nodeLister, nodeName) + if err != nil { + return fmt.Errorf("couldn't check node %q ready status, %v", nodeName, err) + } + if !ready { + continue + } + for _, pod := range pods { + if err := SetPodUpdateAnnotation(c.kubeclientset, ds, pod); err != nil { + return err + } } } return nil } // autoUpdate identifies the set of old pods to delete -func (c *Controller) autoUpdate(ds *appsv1.DaemonSet) error { - nodeToDaemonPods, err := c.getNodesToDaemonPods(ds) - if err != nil { - return fmt.Errorf("couldn't get node to daemon pod mapping for daemon set %q: %v", ds.Name, err) - } - - // TODO(hxc): calculate maxUnavailable specified by user, default is 1 +func (c *Controller) autoUpdate(ds *appsv1.DaemonSet, nodeToDaemonPods map[string][]*corev1.Pod) error { + // Calculate maxUnavailable specified by user, default is 1 maxUnavailable, err := c.maxUnavailableCounts(ds, nodeToDaemonPods) if err != nil { return fmt.Errorf("couldn't get maxUnavailable number for daemon set %q: %v", ds.Name, err) @@ -342,7 +325,8 @@ func (c *Controller) autoUpdate(ds *appsv1.DaemonSet) error { var candidatePodsToDelete []string for nodeName, pods := range nodeToDaemonPods { - // check if node is ready, ignore not-ready node + // Check if node is ready, ignore not-ready node + // this is a significant difference from the native daemonset controller ready, err := NodeReadyByName(c.nodeLister, nodeName) if err != nil { return fmt.Errorf("couldn't check node %q ready status, %v", nodeName, err) @@ -353,38 +337,38 @@ func (c *Controller) autoUpdate(ds *appsv1.DaemonSet) error { newPod, oldPod, ok := findUpdatedPodsOnNode(ds, pods) if !ok { - // let the manage loop clean up this node, and treat it as an unavailable node + // Let the manage loop clean up this node, and treat it as an unavailable node klog.V(3).Infof("DaemonSet %s/%s has excess pods on node %s, skipping to allow the core loop to process", ds.Namespace, ds.Name, nodeName) numUnavailable++ continue } switch { case oldPod == nil && newPod == nil, oldPod != nil && newPod != nil: - // the manage loop will handle creating or deleting the appropriate pod, consider this unavailable + // The manage loop will handle creating or deleting the appropriate pod, consider this unavailable numUnavailable++ case newPod != nil: - // this pod is up-to-date, check its availability + // This pod is up-to-date, check its availability if !k8sutil.IsPodAvailable(newPod, ds.Spec.MinReadySeconds, metav1.Time{Time: time.Now()}) { - // an unavailable new pod is counted against maxUnavailable + // An unavailable new pod is counted against maxUnavailable numUnavailable++ } default: - // this pod is old, it is an update candidate + // This pod is old, it is an update candidate switch { case !k8sutil.IsPodAvailable(oldPod, ds.Spec.MinReadySeconds, metav1.Time{Time: time.Now()}): - // the old pod isn't available, so it needs to be replaced + // The old pod isn't available, so it needs to be replaced klog.V(5).Infof("DaemonSet %s/%s pod %s on node %s is out of date and not available, allowing replacement", ds.Namespace, ds.Name, oldPod.Name, nodeName) - // record the replacement + // Record the replacement if allowedReplacementPods == nil { allowedReplacementPods = make([]string, 0, len(nodeToDaemonPods)) } allowedReplacementPods = append(allowedReplacementPods, oldPod.Name) case numUnavailable >= maxUnavailable: - // no point considering any other candidates + // No point considering any other candidates continue default: klog.V(5).Infof("DaemonSet %s/%s pod %s on node %s is out of date, this is a candidate to replace", ds.Namespace, ds.Name, oldPod.Name, nodeName) - // record the candidate + // Record the candidate if candidatePodsToDelete == nil { candidatePodsToDelete = make([]string, 0, maxUnavailable) } @@ -392,7 +376,7 @@ func (c *Controller) autoUpdate(ds *appsv1.DaemonSet) error { } } } - // use any of the candidates we can, including the allowedReplacemnntPods + // Use any of the candidates we can, including the allowedReplacemnntPods klog.V(5).Infof("DaemonSet %s/%s allowing %d replacements, up to %d unavailable, %d are unavailable, %d candidates", ds.Namespace, ds.Name, len(allowedReplacementPods), maxUnavailable, numUnavailable, len(candidatePodsToDelete)) remainingUnavailable := maxUnavailable - numUnavailable if remainingUnavailable < 0 { @@ -407,7 +391,7 @@ func (c *Controller) autoUpdate(ds *appsv1.DaemonSet) error { } func (c *Controller) getNodesToDaemonPods(ds *appsv1.DaemonSet) (map[string][]*corev1.Pod, error) { - // TODO(hxc): ignore adopt/orphan pod, just deal with pods in podLister + // Ignore adopt/orphan pod, just deal with pods in podLister pods, err := GetDaemonsetPods(c.podLister, ds) if err != nil { return nil, err @@ -446,10 +430,10 @@ func (c *Controller) syncPodsOnNodes(ds *appsv1.DaemonSet, podsToDelete []string c.expectations.SetExpectations(dsKey, 0, deleteDiff) - // error channel to communicate back failures, make the buffer big enough to avoid any blocking + // Error channel to communicate back failures, make the buffer big enough to avoid any blocking errCh := make(chan error, deleteDiff) - // delete pods process + // Delete pods process klog.V(4).Infof("Pods to delete for daemon set %s: %+v, deleting %d", ds.Name, podsToDelete, deleteDiff) deleteWait := sync.WaitGroup{} deleteWait.Add(deleteDiff) @@ -469,7 +453,7 @@ func (c *Controller) syncPodsOnNodes(ds *appsv1.DaemonSet, podsToDelete []string } deleteWait.Wait() - // collect errors if any for proper reporting/retry logic in the controller + // Collect errors if any for proper reporting/retry logic in the controller errors := []error{} close(errCh) for err := range errCh { @@ -480,7 +464,7 @@ func (c *Controller) syncPodsOnNodes(ds *appsv1.DaemonSet, podsToDelete []string // maxUnavailableCounts calculates the true number of allowed unavailable func (c *Controller) maxUnavailableCounts(ds *appsv1.DaemonSet, nodeToDaemonPods map[string][]*corev1.Pod) (int, error) { - // if annotation is not set, use default value one + // If annotation is not set, use default value one v, ok := ds.Annotations[MaxUnavailableAnnotation] if !ok { return 1, nil @@ -492,7 +476,7 @@ func (c *Controller) maxUnavailableCounts(ds *appsv1.DaemonSet, nodeToDaemonPods return -1, fmt.Errorf("invalid value for MaxUnavailable: %v", err) } - // if the daemonset returned with an impossible configuration, obey the default of unavailable=1 + // If the daemonset returned with an impossible configuration, obey the default of unavailable=1 if maxUnavailable == 0 { klog.Warningf("DaemonSet %s/%s is not configured for unavailability, defaulting to accepting unavailability", ds.Namespace, ds.Name) maxUnavailable = 1 @@ -500,3 +484,24 @@ func (c *Controller) maxUnavailableCounts(ds *appsv1.DaemonSet, nodeToDaemonPods klog.V(5).Infof("DaemonSet %s/%s, maxUnavailable: %d", ds.Namespace, ds.Name, maxUnavailable) return maxUnavailable, nil } + +// resolveControllerRef returns the controller referenced by a ControllerRef, +// or nil if the ControllerRef could not be resolved to a matching controller +// of the correct Kind. +func (c *Controller) resolveControllerRef(namespace string, controllerRef *metav1.OwnerReference) *appsv1.DaemonSet { + // We can't look up by UID, so look up by Name and then verify UID. + // Don't even try to look up by Name if it's the wrong Kind. + if controllerRef.Kind != controllerKind.Kind { + return nil + } + ds, err := c.daemonsetLister.DaemonSets(namespace).Get(controllerRef.Name) + if err != nil { + return nil + } + if ds.UID != controllerRef.UID { + // The controller we found with this Name is not the same one that the + // ControllerRef points to. + return nil + } + return ds +} diff --git a/pkg/controller/daemonpodupdater/daemon_pod_updater_controller_test.go b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller_test.go index fdb73feeafb..f7ff5d79e1c 100644 --- a/pkg/controller/daemonpodupdater/daemon_pod_updater_controller_test.go +++ b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller_test.go @@ -22,9 +22,9 @@ import ( "sync" "testing" + "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -169,12 +169,12 @@ type fakePodControl struct { sync.Mutex *k8sutil.FakePodControl podStore cache.Store - podIDMap map[string]*v1.Pod + podIDMap map[string]*corev1.Pod expectations k8sutil.ControllerExpectationsInterface } func newFakePodControl() *fakePodControl { - podIDMap := make(map[string]*v1.Pod) + podIDMap := make(map[string]*corev1.Pod) return &fakePodControl{ FakePodControl: &k8sutil.FakePodControl{}, podIDMap: podIDMap, @@ -250,7 +250,7 @@ func expectSyncDaemonSets(t *testing.T, tcase tCase, fakeCtrl *fakeController, d // Execute test case round := expectedDeletes / maxUnavailable for round >= 0 { - err = fakeCtrl.syncDaemonsetHandler(key) + err = fakeCtrl.syncHandler(key) if err != nil { t.Fatalf("Test %q does not passed, got syncDaemonsetHandler error %v", tcase.name, err) } @@ -268,18 +268,6 @@ func expectSyncDaemonSets(t *testing.T, tcase tCase, fakeCtrl *fakeController, d } } -// clearExpectations copies the FakePodControl to PodStore and clears the delete expectations. -// func clearExpectations(t *testing.T, fakeCtrl *fakeController, ds *appsv1.DaemonSet, fakePodControl *fakePodControl) { -// fakePodControl.Clear() - -// key, err := cache.MetaNamespaceKeyFunc(ds) -// if err != nil { -// t.Errorf("Could not get key for daemon.") -// return -// } -// fakeCtrl.expectations.DeleteExpectations(key) -// } - // ---------------------------------------------------------------------------------------------------------------- // -------------------------------------------------------util----------------------------------------------------- // ---------------------------------------------------------------------------------------------------------------- @@ -288,6 +276,10 @@ func setAutoUpdateAnnotation(ds *appsv1.DaemonSet) { metav1.SetMetaDataAnnotation(&ds.ObjectMeta, UpdateAnnotation, AutoUpdate) } +func setOTAUpdateAnnotation(ds *appsv1.DaemonSet) { + metav1.SetMetaDataAnnotation(&ds.ObjectMeta, UpdateAnnotation, OTAUpdate) +} + func setMaxUnavailableAnnotation(ds *appsv1.DaemonSet, v string) { metav1.SetMetaDataAnnotation(&ds.ObjectMeta, MaxUnavailableAnnotation, v) } @@ -479,3 +471,48 @@ func TestDaemonsetPodUpdater(t *testing.T) { } } } + +func TestOTAUpdate(t *testing.T) { + ds := newDaemonSet("ds", "foo/bar:v1") + setOTAUpdateAnnotation(ds) + + node := newNode("node", true) + oldPod := newPod("old-pod", node.Name, simpleDaemonSetLabel, ds) + ds.Spec.Template.Spec.Containers[0].Image = "foo/bar:v2" + newPod := newPod("new-pod", node.Name, simpleDaemonSetLabel, ds) + + fakeCtrl, _ := newTest(ds, oldPod, newPod, node) + + fakeCtrl.podStore.Add(oldPod) + fakeCtrl.podStore.Add(newPod) + fakeCtrl.dsStore.Add(ds) + fakeCtrl.nodeStore.Add(node) + + key, err := cache.MetaNamespaceKeyFunc(ds) + if err != nil { + t.Fatal(err) + } + if err = fakeCtrl.syncHandler(key); err != nil { + t.Fatalf("OTA test does not passed, got syncDaemonsetHandler error %v", err) + } + + // check whether ota upgradable annotation set properly + oldPodGot, err := fakeCtrl.kubeclientset.CoreV1().Pods(ds.Namespace).Get(context.TODO(), oldPod.Name, + metav1.GetOptions{}) + if err != nil { + t.Errorf("get oldPod failed, %+v", err) + } + + newPodGot, err := fakeCtrl.kubeclientset.CoreV1().Pods(ds.Namespace).Get(context.TODO(), newPod.Name, metav1.GetOptions{}) + if err != nil { + t.Errorf("get newPod failed, %+v", err) + } + + annOldPodGot, oldPodOK := oldPodGot.Annotations[PodUpdatableAnnotation] + assert.Equal(t, true, oldPodOK) + assert.Equal(t, "true", annOldPodGot) + + annNewPodGot, newPodOK := newPodGot.Annotations[PodUpdatableAnnotation] + assert.Equal(t, true, newPodOK) + assert.Equal(t, "false", annNewPodGot) +} diff --git a/pkg/controller/daemonpodupdater/kubernetes/controller_utils.go b/pkg/controller/daemonpodupdater/kubernetes/controller_utils.go index 42ea916f9c5..f6422fd1b32 100644 --- a/pkg/controller/daemonpodupdater/kubernetes/controller_utils.go +++ b/pkg/controller/daemonpodupdater/kubernetes/controller_utils.go @@ -36,7 +36,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/rand" - "k8s.io/apimachinery/pkg/util/sets" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" @@ -239,94 +238,6 @@ func NewControllerExpectations() *ControllerExpectations { return &ControllerExpectations{cache.NewStore(ExpKeyFunc)} } -// UIDSetKeyFunc to parse out the key from a UIDSet. -var UIDSetKeyFunc = func(obj interface{}) (string, error) { - if u, ok := obj.(*UIDSet); ok { - return u.key, nil - } - return "", fmt.Errorf("could not find key for obj %#v", obj) -} - -// UIDSet holds a key and a set of UIDs. Used by the -// UIDTrackingControllerExpectations to remember which UID it has seen/still -// waiting for. -type UIDSet struct { - sets.String - key string -} - -// UIDTrackingControllerExpectations tracks the UID of the pods it deletes. -// This cache is needed over plain old expectations to safely handle graceful -// deletion. The desired behavior is to treat an update that sets the -// DeletionTimestamp on an object as a delete. To do so consistently, one needs -// to remember the expected deletes so they aren't double counted. -// TODO: Track creates as well (#22599) -type UIDTrackingControllerExpectations struct { - ControllerExpectationsInterface - // TODO: There is a much nicer way to do this that involves a single store, - // a lock per entry, and a ControlleeExpectationsInterface type. - uidStoreLock sync.Mutex - // Store used for the UIDs associated with any expectation tracked via the - // ControllerExpectationsInterface. - uidStore cache.Store -} - -// GetUIDs is a convenience method to avoid exposing the set of expected uids. -// The returned set is not thread safe, all modifications must be made holding -// the uidStoreLock. -func (u *UIDTrackingControllerExpectations) GetUIDs(controllerKey string) sets.String { - if uid, exists, err := u.uidStore.GetByKey(controllerKey); err == nil && exists { - return uid.(*UIDSet).String - } - return nil -} - -// ExpectDeletions records expectations for the given deleteKeys, against the given controller. -func (u *UIDTrackingControllerExpectations) ExpectDeletions(rcKey string, deletedKeys []string) error { - expectedUIDs := sets.NewString() - for _, k := range deletedKeys { - expectedUIDs.Insert(k) - } - klog.V(4).Infof("Controller %v waiting on deletions for: %+v", rcKey, deletedKeys) - u.uidStoreLock.Lock() - defer u.uidStoreLock.Unlock() - - if existing := u.GetUIDs(rcKey); existing != nil && existing.Len() != 0 { - klog.Errorf("Clobbering existing delete keys: %+v", existing) - } - if err := u.uidStore.Add(&UIDSet{expectedUIDs, rcKey}); err != nil { - return err - } - return u.ControllerExpectationsInterface.ExpectDeletions(rcKey, expectedUIDs.Len()) -} - -// DeletionObserved records the given deleteKey as a deletion, for the given rc. -func (u *UIDTrackingControllerExpectations) DeletionObserved(rcKey, deleteKey string) { - u.uidStoreLock.Lock() - defer u.uidStoreLock.Unlock() - - uids := u.GetUIDs(rcKey) - if uids != nil && uids.Has(deleteKey) { - klog.V(4).Infof("Controller %v received delete for pod %v", rcKey, deleteKey) - u.ControllerExpectationsInterface.DeletionObserved(rcKey) - uids.Delete(deleteKey) - } -} - -// DeleteExpectations deletes the UID set and invokes DeleteExpectations on the -// underlying ControllerExpectationsInterface. -func (u *UIDTrackingControllerExpectations) DeleteExpectations(rcKey string) { - u.uidStoreLock.Lock() - defer u.uidStoreLock.Unlock() - - u.ControllerExpectationsInterface.DeleteExpectations(rcKey) - if uidExp, exists, err := u.uidStore.GetByKey(rcKey); err == nil && exists { - if err := u.uidStore.Delete(uidExp); err != nil { - klog.V(2).Infof("Error deleting uid expectations for controller %v: %v", rcKey, err) - } - } -} - // Reasons for pod events const ( // FailedCreatePodReason is added in an event and in a replica set condition diff --git a/pkg/controller/daemonpodupdater/util.go b/pkg/controller/daemonpodupdater/util.go index 7ee929a8835..cb60885505f 100644 --- a/pkg/controller/daemonpodupdater/util.go +++ b/pkg/controller/daemonpodupdater/util.go @@ -59,7 +59,7 @@ func GetDaemonsetPods(podLister corelisters.PodLister, ds *appsv1.DaemonSet) ([] return dsPods, nil } -// IsDaemonsetPodLatest check whether pod is latest by comparing its Spec with daemonset's +// IsDaemonsetPodLatest check whether pod is the latest by comparing its Spec with daemonset's // If pod is latest, return true, otherwise return false func IsDaemonsetPodLatest(ds *appsv1.DaemonSet, pod *corev1.Pod) bool { hash := k8sutil.ComputeHash(&ds.Spec.Template, ds.Status.CollisionCount) @@ -134,7 +134,7 @@ func checkPrerequisites(ds *appsv1.DaemonSet) bool { return ds.Spec.UpdateStrategy.Type == appsv1.OnDeleteDaemonSetStrategyType } -// Clones the given map and returns a new map with the given key and value added. +// CloneAndAddLabel clones the given map and returns a new map with the given key and value added. // Returns the given map, if labelKey is empty. func CloneAndAddLabel(labels map[string]string, labelKey, labelValue string) map[string]string { if labelKey == "" { diff --git a/pkg/controller/daemonpodupdater/util_test.go b/pkg/controller/daemonpodupdater/util_test.go index abd6b9c2098..524caa90378 100644 --- a/pkg/controller/daemonpodupdater/util_test.go +++ b/pkg/controller/daemonpodupdater/util_test.go @@ -22,7 +22,7 @@ import ( "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" ) @@ -87,7 +87,7 @@ func Test_checkPrerequisites(t *testing.T) { { name: "satisfied-ota", ds: &appsv1.DaemonSet{ - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ "apps.openyurt.io/update-strategy": "ota", }, @@ -103,7 +103,7 @@ func Test_checkPrerequisites(t *testing.T) { { name: "satisfied-auto", ds: &appsv1.DaemonSet{ - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ "apps.openyurt.io/update-strategy": "auto", }, @@ -119,7 +119,7 @@ func Test_checkPrerequisites(t *testing.T) { { name: "unsatisfied-other", ds: &appsv1.DaemonSet{ - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ "apps.openyurt.io/update-strategy": "other", }, @@ -146,7 +146,7 @@ func Test_checkPrerequisites(t *testing.T) { { name: "unsatisfied-without-updateStrategy", ds: &appsv1.DaemonSet{ - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ "apps.openyurt.io/update-strategy": "other", }, @@ -168,3 +168,63 @@ func Test_checkPrerequisites(t *testing.T) { }) } } + +func TestGetTargetNodeName(t *testing.T) { + pod := newPod("pod", "", nil, nil) + + podWithName := pod.DeepCopy() + podWithName.Spec.NodeName = "node" + + podwithAffinity := pod.DeepCopy() + podwithAffinity.Spec.Affinity = &corev1.Affinity{ + NodeAffinity: &corev1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{ + NodeSelectorTerms: []corev1.NodeSelectorTerm{ + { + MatchFields: []corev1.NodeSelectorRequirement{ + { + Key: metav1.ObjectNameField, + Operator: corev1.NodeSelectorOpIn, + Values: []string{"affinity"}, + }, + }, + }, + }, + }, + }, + } + + tests := []struct { + pod *corev1.Pod + want string + wantErr bool + }{ + { + pod: pod, + want: "", + wantErr: true, + }, + { + pod: podWithName, + want: "node", + wantErr: false, + }, + { + pod: podwithAffinity, + want: "affinity", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run("", func(t *testing.T) { + got, err := GetTargetNodeName(tt.pod) + if (err != nil) != tt.wantErr { + t.Errorf("GetTargetNodeName() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("GetTargetNodeName() got = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/util/kubernetes/controller/controller_utils.go b/pkg/util/kubernetes/controller/controller_utils.go deleted file mode 100644 index 18239357d35..00000000000 --- a/pkg/util/kubernetes/controller/controller_utils.go +++ /dev/null @@ -1,58 +0,0 @@ -/* -Copyright 2014 The Kubernetes Authors. -Copyright 2022 The OpenYurt 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 controller - -import ( - "encoding/binary" - "fmt" - "hash" - "hash/fnv" - - "github.com/davecgh/go-spew/spew" - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/util/rand" -) - -// ComputeHash returns a hash value calculated from pod template and -// a collisionCount to avoid hash collision. The hash will be safe encoded to -// avoid bad words. -func ComputeHash(template *v1.PodTemplateSpec, collisionCount *int32) string { - podTemplateSpecHasher := fnv.New32a() - DeepHashObject(podTemplateSpecHasher, *template) - - // Add collisionCount in the hash if it exists. - if collisionCount != nil { - collisionCountBytes := make([]byte, 8) - binary.LittleEndian.PutUint32(collisionCountBytes, uint32(*collisionCount)) - podTemplateSpecHasher.Write(collisionCountBytes) - } - - return rand.SafeEncodeString(fmt.Sprint(podTemplateSpecHasher.Sum32())) -} - -// DeepHashObject writes specified object to hash using the spew library -// which follows pointers and prints actual values of the nested objects -// ensuring the hash does not change when a pointer changes. -func DeepHashObject(hasher hash.Hash, objectToWrite interface{}) { - hasher.Reset() - printer := spew.ConfigState{ - Indent: " ", - SortKeys: true, - DisableMethods: true, - SpewKeys: true, - } - printer.Fprintf(hasher, "%#v", objectToWrite) -} From be730445450cebdad08422c4e1062a95baabbb61 Mon Sep 17 00:00:00 2001 From: Xuecheng Date: Thu, 15 Sep 2022 00:30:53 -0700 Subject: [PATCH 09/11] use pod condition to specify upgradability Signed-off-by: Xuecheng --- .../daemon_pod_updater_controller.go | 12 +++--- .../daemon_pod_updater_controller_test.go | 11 ++--- pkg/controller/daemonpodupdater/util.go | 41 +++++++++++++++++-- 3 files changed, 45 insertions(+), 19 deletions(-) diff --git a/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go index 357f070c488..5ca346e012c 100644 --- a/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go +++ b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go @@ -53,10 +53,8 @@ const ( OTAUpdate = "ota" AutoUpdate = "auto" - // PodUpdatableAnnotation is the annotation key added to pods to indicate - // whether a new version is available for update. - // This annotation will only be added if the update strategy is "apps.openyurt.io/update-strategy":"ota". - PodUpdatableAnnotation = "apps.openyurt.io/pod-updatable" + // PodNeedUpgrade indicates whether the pod is able to upgrade. + PodNeedUpgrade corev1.PodConditionType = "PodNeedUpgrade" // MaxUnavailableAnnotation is the annotation key added to daemonset to indicate // the max unavailable pods number. It's used with "apps.openyurt.io/update-strategy=auto". @@ -291,8 +289,8 @@ func (c *Controller) syncHandler(key string) error { } // checkOTAUpdate compare every pod to its owner daemonset to check if pod is updatable -// If pod is in line with the latest daemonset spec, set annotation "apps.openyurt.io/pod-updatable" to "true" -// while not, set annotation "apps.openyurt.io/pod-updatable" to "false" +// If pod is in line with the latest daemonset spec, set pod condition "PodNeedUpgrade" to "false" +// while not, set pod condition "PodNeedUpgrade" to "true" func (c *Controller) checkOTAUpdate(ds *appsv1.DaemonSet, nodeToDaemonPods map[string][]*corev1.Pod) error { for nodeName, pods := range nodeToDaemonPods { // Check if node is ready, ignore not-ready node @@ -304,7 +302,7 @@ func (c *Controller) checkOTAUpdate(ds *appsv1.DaemonSet, nodeToDaemonPods map[s continue } for _, pod := range pods { - if err := SetPodUpdateAnnotation(c.kubeclientset, ds, pod); err != nil { + if err := SetPodUpgradeCondition(c.kubeclientset, ds, pod); err != nil { return err } } diff --git a/pkg/controller/daemonpodupdater/daemon_pod_updater_controller_test.go b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller_test.go index f7ff5d79e1c..b45b67a7c6c 100644 --- a/pkg/controller/daemonpodupdater/daemon_pod_updater_controller_test.go +++ b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller_test.go @@ -496,7 +496,7 @@ func TestOTAUpdate(t *testing.T) { t.Fatalf("OTA test does not passed, got syncDaemonsetHandler error %v", err) } - // check whether ota upgradable annotation set properly + // check whether ota PodNeedUpgrade condition set properly oldPodGot, err := fakeCtrl.kubeclientset.CoreV1().Pods(ds.Namespace).Get(context.TODO(), oldPod.Name, metav1.GetOptions{}) if err != nil { @@ -508,11 +508,6 @@ func TestOTAUpdate(t *testing.T) { t.Errorf("get newPod failed, %+v", err) } - annOldPodGot, oldPodOK := oldPodGot.Annotations[PodUpdatableAnnotation] - assert.Equal(t, true, oldPodOK) - assert.Equal(t, "true", annOldPodGot) - - annNewPodGot, newPodOK := newPodGot.Annotations[PodUpdatableAnnotation] - assert.Equal(t, true, newPodOK) - assert.Equal(t, "false", annNewPodGot) + assert.Equal(t, true, IsPodUpdatable(oldPodGot)) + assert.Equal(t, false, IsPodUpdatable(newPodGot)) } diff --git a/pkg/controller/daemonpodupdater/util.go b/pkg/controller/daemonpodupdater/util.go index cb60885505f..26fcd8215b9 100644 --- a/pkg/controller/daemonpodupdater/util.go +++ b/pkg/controller/daemonpodupdater/util.go @@ -31,6 +31,7 @@ import ( "k8s.io/klog/v2" k8sutil "github.com/openyurtio/openyurt/pkg/controller/daemonpodupdater/kubernetes" + util "github.com/openyurtio/openyurt/pkg/controller/util/node" ) // GetDaemonsetPods get all pods belong to the given daemonset @@ -110,15 +111,29 @@ func NodeReady(nodeStatus *corev1.NodeStatus) bool { return false } -// SetPodUpdateAnnotation calculate and set annotation "apps.openyurt.io/pod-updatable" to pod -func SetPodUpdateAnnotation(clientset client.Interface, ds *appsv1.DaemonSet, pod *corev1.Pod) error { +// SetPodUpgradeCondition calculate and set pod condition "PodNeedUpgrade" +func SetPodUpgradeCondition(clientset client.Interface, ds *appsv1.DaemonSet, pod *corev1.Pod) error { isUpdatable := IsDaemonsetPodLatest(ds, pod) - metav1.SetMetaDataAnnotation(&pod.ObjectMeta, PodUpdatableAnnotation, strconv.FormatBool(!isUpdatable)) + // Comply with K8s, use constant ConditionTrue and ConditionFalse + var status corev1.ConditionStatus + switch isUpdatable { + case true: + status = corev1.ConditionFalse + case false: + status = corev1.ConditionTrue + } + + cond := &corev1.PodCondition{ + Type: PodNeedUpgrade, + Status: status, + } + util.UpdatePodCondition(&pod.Status, cond) + if _, err := clientset.CoreV1().Pods(pod.Namespace).Update(context.TODO(), pod, metav1.UpdateOptions{}); err != nil { return err } - klog.Infof("set pod %q annotation apps.openyurt.io/pod-updatable to %v", pod.Name, !isUpdatable) + klog.Infof("set pod %q condition PodNeedUpgrade to %v", pod.Name, !isUpdatable) return nil } @@ -213,3 +228,21 @@ func GetTargetNodeName(pod *corev1.Pod) (string, error) { return "", fmt.Errorf("no node name found for pod %s/%s", pod.Namespace, pod.Name) } + +// IsPodUpdatable returns true if a pod is updatable; false otherwise. +func IsPodUpdatable(pod *corev1.Pod) bool { + return IsPodUpgradeConditionTrue(pod.Status) +} + +// IsPodUpgradeConditionTrue returns true if a pod is updatable; false otherwise. +func IsPodUpgradeConditionTrue(status corev1.PodStatus) bool { + condition := GetPodUpgradeCondition(status) + return condition != nil && condition.Status == corev1.ConditionTrue +} + +// GetPodUpgradeCondition extracts the pod upgrade condition from the given status and returns that. +// Returns nil if the condition is not present. +func GetPodUpgradeCondition(status corev1.PodStatus) *corev1.PodCondition { + _, condition := k8sutil.GetPodCondition(&status, PodNeedUpgrade) + return condition +} From c2c44ee38dba7713a2a084a362572d7dd49e605e Mon Sep 17 00:00:00 2001 From: hxcGit Date: Tue, 20 Sep 2022 11:17:39 +0800 Subject: [PATCH 10/11] add code comments Signed-off-by: hxcGit --- cmd/yurt-controller-manager/app/core.go | 2 +- .../daemon_pod_updater_controller.go | 70 ++++++++++++------- pkg/controller/daemonpodupdater/util.go | 10 +-- 3 files changed, 52 insertions(+), 30 deletions(-) diff --git a/cmd/yurt-controller-manager/app/core.go b/cmd/yurt-controller-manager/app/core.go index 76f344b1d70..fce440de10d 100644 --- a/cmd/yurt-controller-manager/app/core.go +++ b/cmd/yurt-controller-manager/app/core.go @@ -69,7 +69,7 @@ func startYurtCSRApproverController(ctx ControllerContext) (http.Handler, bool, func startDaemonPodUpdaterController(ctx ControllerContext) (http.Handler, bool, error) { daemonPodUpdaterCtrl := daemonpodupdater.NewController( - ctx.ClientBuilder.ClientOrDie("podUpgrade-controller"), + ctx.ClientBuilder.ClientOrDie("daemonPodUpdater-controller"), ctx.InformerFactory.Apps().V1().DaemonSets(), ctx.InformerFactory.Core().V1().Nodes(), ctx.InformerFactory.Core().V1().Pods(), diff --git a/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go index 5ca346e012c..8be877751b0 100644 --- a/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go +++ b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go @@ -50,7 +50,12 @@ const ( // which update strategy is selected. Currently, "ota" and "auto" are supported. UpdateAnnotation = "apps.openyurt.io/update-strategy" - OTAUpdate = "ota" + // OTAUpdate set daemonset to over-the-air update mode. + // In daemonPodUpdater controller, we add PodNeedUpgrade condition to pods. + OTAUpdate = "ota" + // AutoUpdate set daemonset to auto update mode. + // In this mode, daemonset will keep updating even if there are not-ready nodes. + // For more details, see https://github.com/openyurtio/openyurt/pull/921. AutoUpdate = "auto" // PodNeedUpgrade indicates whether the pod is able to upgrade. @@ -58,11 +63,14 @@ const ( // MaxUnavailableAnnotation is the annotation key added to daemonset to indicate // the max unavailable pods number. It's used with "apps.openyurt.io/update-strategy=auto". + // If this annotation is not explicitly stated, it will be set to the default value 1. MaxUnavailableAnnotation = "apps.openyurt.io/max-unavailable" // BurstReplicas is a rate limiter for booting pods on a lot of pods. // The value of 250 is chosen b/c values that are too high can cause registry DoS issues. BurstReplicas = 250 + + maxRetries = 30 ) // controllerKind contains the schema.GroupVersionKind for this controller type. @@ -71,17 +79,15 @@ var controllerKind = appsv1.SchemeGroupVersion.WithKind("DaemonSet") type Controller struct { kubeclientset client.Interface podControl k8sutil.PodControlInterface - - daemonsetLister appslisters.DaemonSetLister - daemonsetSynced cache.InformerSynced - nodeLister corelisters.NodeLister - nodeSynced cache.InformerSynced - podLister corelisters.PodLister - podSynced cache.InformerSynced - - daemonsetWorkqueue workqueue.Interface - - expectations k8sutil.ControllerExpectationsInterface + // daemonPodUpdater watches daemonset, node and pod resource + daemonsetLister appslisters.DaemonSetLister + daemonsetSynced cache.InformerSynced + nodeLister corelisters.NodeLister + nodeSynced cache.InformerSynced + podLister corelisters.PodLister + podSynced cache.InformerSynced + daemonsetWorkqueue workqueue.RateLimitingInterface + expectations k8sutil.ControllerExpectationsInterface } func NewController(kc client.Interface, daemonsetInformer appsinformers.DaemonSetInformer, @@ -93,9 +99,10 @@ func NewController(kc client.Interface, daemonsetInformer appsinformers.DaemonSe ctrl := Controller{ kubeclientset: kc, + // Use PodControlInterface to delete pods, which is convenient for testing podControl: k8sutil.RealPodControl{ KubeClient: kc, - Recorder: eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: "podUpdater"}), + Recorder: eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: "daemonPodUpdater"}), }, daemonsetLister: daemonsetInformer.Lister(), @@ -107,16 +114,22 @@ func NewController(kc client.Interface, daemonsetInformer appsinformers.DaemonSe podLister: podInformer.Lister(), podSynced: podInformer.Informer().HasSynced, - daemonsetWorkqueue: workqueue.New(), + daemonsetWorkqueue: workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()), expectations: k8sutil.NewControllerExpectations(), } + // In this controller, we focus three cases + // 1. daemonset specification changes + // 2. node turns from not-ready to ready + // 3. pods were deleted successfully + // In case 2, daemonset.Status.DesiredNumberScheduled will change and, in case 3, daemonset.Status.NumberReady + // will change. Therefore, we focus only on the daemonset Update event, which can cover the above situations. daemonsetInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ UpdateFunc: func(old, new interface{}) { newDS := new.(*appsv1.DaemonSet) oldDS := old.(*appsv1.DaemonSet) - // Only control daemonset meets prerequisites + // Only handle daemonset meets prerequisites if !checkPrerequisites(newDS) { return } @@ -187,6 +200,7 @@ func (c *Controller) deletePod(obj interface{}) { } dsKey, err := cache.MetaNamespaceKeyFunc(ds) if err != nil { + utilruntime.HandleError(err) return } @@ -201,8 +215,7 @@ func (c *Controller) Run(threadiness int, stopCh <-chan struct{}) { defer c.daemonsetWorkqueue.ShutDown() // Synchronize the cache before starting to process events - if !cache.WaitForCacheSync(stopCh, c.daemonsetSynced, c.nodeSynced, - c.podSynced) { + if !cache.WaitForCacheSync(stopCh, c.daemonsetSynced, c.nodeSynced, c.podSynced) { klog.Error("sync daemonPodUpdater controller timeout") } @@ -220,10 +233,17 @@ func (c *Controller) runWorker() { return } - // TODO(hxc): should requeue failed ds if err := c.syncHandler(obj.(string)); err != nil { + if c.daemonsetWorkqueue.NumRequeues(obj) < maxRetries { + klog.Infof("error syncing event %v: %v", obj, err) + c.daemonsetWorkqueue.AddRateLimited(obj) + c.daemonsetWorkqueue.Done(obj) + continue + } utilruntime.HandleError(err) } + + c.daemonsetWorkqueue.Forget(obj) c.daemonsetWorkqueue.Done(obj) } } @@ -273,7 +293,7 @@ func (c *Controller) syncHandler(key string) error { switch v { case OTAUpdate: - if err := c.checkOTAUpdate(ds, nodeToDaemonPods); err != nil { + if err := c.otaUpdate(ds, nodeToDaemonPods); err != nil { return err } @@ -288,10 +308,10 @@ func (c *Controller) syncHandler(key string) error { return nil } -// checkOTAUpdate compare every pod to its owner daemonset to check if pod is updatable +// otaUpdate compare every pod to its owner daemonset to check if pod is updatable // If pod is in line with the latest daemonset spec, set pod condition "PodNeedUpgrade" to "false" // while not, set pod condition "PodNeedUpgrade" to "true" -func (c *Controller) checkOTAUpdate(ds *appsv1.DaemonSet, nodeToDaemonPods map[string][]*corev1.Pod) error { +func (c *Controller) otaUpdate(ds *appsv1.DaemonSet, nodeToDaemonPods map[string][]*corev1.Pod) error { for nodeName, pods := range nodeToDaemonPods { // Check if node is ready, ignore not-ready node ready, err := NodeReadyByName(c.nodeLister, nodeName) @@ -310,7 +330,8 @@ func (c *Controller) checkOTAUpdate(ds *appsv1.DaemonSet, nodeToDaemonPods map[s return nil } -// autoUpdate identifies the set of old pods to delete +// autoUpdate identifies the set of old pods to delete within the constraints imposed by the max-unavailable number. +// Just ignore and do not calculate not-ready nodes. func (c *Controller) autoUpdate(ds *appsv1.DaemonSet, nodeToDaemonPods map[string][]*corev1.Pod) error { // Calculate maxUnavailable specified by user, default is 1 maxUnavailable, err := c.maxUnavailableCounts(ds, nodeToDaemonPods) @@ -388,6 +409,7 @@ func (c *Controller) autoUpdate(ds *appsv1.DaemonSet, nodeToDaemonPods map[strin return c.syncPodsOnNodes(ds, oldPodsToDelete) } +// getNodesToDaemonPods returns a map from nodes to daemon pods (corresponding to ds) created for the nodes. func (c *Controller) getNodesToDaemonPods(ds *appsv1.DaemonSet) (map[string][]*corev1.Pod, error) { // Ignore adopt/orphan pod, just deal with pods in podLister pods, err := GetDaemonsetPods(c.podLister, ds) @@ -411,8 +433,8 @@ func (c *Controller) getNodesToDaemonPods(ds *appsv1.DaemonSet) (map[string][]*c return nodeToDaemonPods, nil } -// syncPodsOnNodes deletes pods on the given nodes -// returns slice with errors if any +// syncPodsOnNodes deletes pods on the given nodes. +// returns slice with errors if any. func (c *Controller) syncPodsOnNodes(ds *appsv1.DaemonSet, podsToDelete []string) error { // We need to set expectations before deleting pods to avoid race conditions. dsKey, err := cache.MetaNamespaceKeyFunc(ds) diff --git a/pkg/controller/daemonpodupdater/util.go b/pkg/controller/daemonpodupdater/util.go index 26fcd8215b9..28d114855c1 100644 --- a/pkg/controller/daemonpodupdater/util.go +++ b/pkg/controller/daemonpodupdater/util.go @@ -128,12 +128,12 @@ func SetPodUpgradeCondition(clientset client.Interface, ds *appsv1.DaemonSet, po Type: PodNeedUpgrade, Status: status, } - util.UpdatePodCondition(&pod.Status, cond) - - if _, err := clientset.CoreV1().Pods(pod.Namespace).Update(context.TODO(), pod, metav1.UpdateOptions{}); err != nil { - return err + if change := util.UpdatePodCondition(&pod.Status, cond); change { + if _, err := clientset.CoreV1().Pods(pod.Namespace).UpdateStatus(context.TODO(), pod, metav1.UpdateOptions{}); err != nil { + return err + } + klog.Infof("set pod %q condition PodNeedUpgrade to %v", pod.Name, !isUpdatable) } - klog.Infof("set pod %q condition PodNeedUpgrade to %v", pod.Name, !isUpdatable) return nil } From b1d4411a7043e1a1b98b273fc655f7e9c40f5474 Mon Sep 17 00:00:00 2001 From: hxcGit Date: Mon, 26 Sep 2022 16:20:36 +0800 Subject: [PATCH 11/11] remove node ready check for ota strategy Signed-off-by: hxcGit --- .../daemon_pod_updater_controller.go | 40 +++++++++---------- .../daemon_pod_updater_controller_test.go | 1 + pkg/controller/daemonpodupdater/util.go | 2 +- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go index 8be877751b0..194c37b7578 100644 --- a/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go +++ b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller.go @@ -1,5 +1,6 @@ /* Copyright 2022 The OpenYurt Authors. +Copyright 2017 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. @@ -280,11 +281,6 @@ func (c *Controller) syncHandler(key string) error { return nil } - nodeToDaemonPods, err := c.getNodesToDaemonPods(ds) - if err != nil { - return fmt.Errorf("couldn't get node to daemon pod mapping for daemon set %q: %v", ds.Name, err) - } - // Recheck required annotation v, ok := ds.Annotations[UpdateAnnotation] if !ok { @@ -293,12 +289,12 @@ func (c *Controller) syncHandler(key string) error { switch v { case OTAUpdate: - if err := c.otaUpdate(ds, nodeToDaemonPods); err != nil { + if err := c.otaUpdate(ds); err != nil { return err } case AutoUpdate: - if err := c.autoUpdate(ds, nodeToDaemonPods); err != nil { + if err := c.autoUpdate(ds); err != nil { return err } default: @@ -311,20 +307,15 @@ func (c *Controller) syncHandler(key string) error { // otaUpdate compare every pod to its owner daemonset to check if pod is updatable // If pod is in line with the latest daemonset spec, set pod condition "PodNeedUpgrade" to "false" // while not, set pod condition "PodNeedUpgrade" to "true" -func (c *Controller) otaUpdate(ds *appsv1.DaemonSet, nodeToDaemonPods map[string][]*corev1.Pod) error { - for nodeName, pods := range nodeToDaemonPods { - // Check if node is ready, ignore not-ready node - ready, err := NodeReadyByName(c.nodeLister, nodeName) - if err != nil { - return fmt.Errorf("couldn't check node %q ready status, %v", nodeName, err) - } - if !ready { - continue - } - for _, pod := range pods { - if err := SetPodUpgradeCondition(c.kubeclientset, ds, pod); err != nil { - return err - } +func (c *Controller) otaUpdate(ds *appsv1.DaemonSet) error { + pods, err := GetDaemonsetPods(c.podLister, ds) + if err != nil { + return err + } + + for _, pod := range pods { + if err := SetPodUpgradeCondition(c.kubeclientset, ds, pod); err != nil { + return err } } return nil @@ -332,7 +323,12 @@ func (c *Controller) otaUpdate(ds *appsv1.DaemonSet, nodeToDaemonPods map[string // autoUpdate identifies the set of old pods to delete within the constraints imposed by the max-unavailable number. // Just ignore and do not calculate not-ready nodes. -func (c *Controller) autoUpdate(ds *appsv1.DaemonSet, nodeToDaemonPods map[string][]*corev1.Pod) error { +func (c *Controller) autoUpdate(ds *appsv1.DaemonSet) error { + nodeToDaemonPods, err := c.getNodesToDaemonPods(ds) + if err != nil { + return fmt.Errorf("couldn't get node to daemon pod mapping for daemon set %q: %v", ds.Name, err) + } + // Calculate maxUnavailable specified by user, default is 1 maxUnavailable, err := c.maxUnavailableCounts(ds, nodeToDaemonPods) if err != nil { diff --git a/pkg/controller/daemonpodupdater/daemon_pod_updater_controller_test.go b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller_test.go index b45b67a7c6c..83441b949a0 100644 --- a/pkg/controller/daemonpodupdater/daemon_pod_updater_controller_test.go +++ b/pkg/controller/daemonpodupdater/daemon_pod_updater_controller_test.go @@ -1,5 +1,6 @@ /* Copyright 2022 The OpenYurt Authors. +Copyright 2017 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. diff --git a/pkg/controller/daemonpodupdater/util.go b/pkg/controller/daemonpodupdater/util.go index 28d114855c1..75b08db65a4 100644 --- a/pkg/controller/daemonpodupdater/util.go +++ b/pkg/controller/daemonpodupdater/util.go @@ -143,7 +143,7 @@ func SetPodUpgradeCondition(clientset client.Interface, ds *appsv1.DaemonSet, po // 2. update strategy is "OnDelete" func checkPrerequisites(ds *appsv1.DaemonSet) bool { v, ok := ds.Annotations[UpdateAnnotation] - if !ok || (v != "auto" && v != "ota") { + if !ok || (v != AutoUpdate && v != OTAUpdate) { return false } return ds.Spec.UpdateStrategy.Type == appsv1.OnDeleteDaemonSetStrategyType