diff --git a/pkg/controller/daemon/update.go b/pkg/controller/daemon/update.go index e1a13f9c4d6..cf66cdaebc5 100644 --- a/pkg/controller/daemon/update.go +++ b/pkg/controller/daemon/update.go @@ -31,6 +31,7 @@ import ( 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/json" podutil "k8s.io/kubernetes/pkg/api/v1/pod" "k8s.io/kubernetes/pkg/controller" @@ -367,12 +368,18 @@ func (dsc *DaemonSetsController) dedupCurHistories(ctx context.Context, ds *apps } for _, pod := range pods { if pod.Labels[apps.DefaultDaemonSetUniqueLabelKey] != keepCur.Labels[apps.DefaultDaemonSetUniqueLabelKey] { - toUpdate := pod.DeepCopy() - if toUpdate.Labels == nil { - toUpdate.Labels = make(map[string]string) + patchRaw := map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + apps.DefaultDaemonSetUniqueLabelKey: keepCur.Labels[apps.DefaultDaemonSetUniqueLabelKey], + }, + }, } - toUpdate.Labels[apps.DefaultDaemonSetUniqueLabelKey] = keepCur.Labels[apps.DefaultDaemonSetUniqueLabelKey] - _, err = dsc.kubeClient.CoreV1().Pods(ds.Namespace).Update(ctx, toUpdate, metav1.UpdateOptions{}) + patchJson, err := json.Marshal(patchRaw) + if err != nil { + return nil, err + } + _, err = dsc.kubeClient.CoreV1().Pods(ds.Namespace).Patch(ctx, pod.Name, types.MergePatchType, patchJson, metav1.PatchOptions{}) if err != nil { return nil, err } diff --git a/test/integration/daemonset/daemonset_test.go b/test/integration/daemonset/daemonset_test.go index cbc116766c6..2d5e9fa4fb6 100644 --- a/test/integration/daemonset/daemonset_test.go +++ b/test/integration/daemonset/daemonset_test.go @@ -782,6 +782,136 @@ func TestLaunchWithHashCollision(t *testing.T) { validateDaemonSetCollisionCount(dsClient, ds.Name, orgCollisionCount+1, t) } +// Test DaemonSet Controller updates label of the pod after "DedupCurHistories". The scenario is +// 1. Create an another controllerrevision owned by the daemonset but with higher revision and different hash +// 2. Add a node to ensure the controller sync +// 3. The dsc is expected to "PATCH" the existing pod label with new hash and deletes the old controllerrevision once finishes the update +func TestDSCUpdatesPodLabelAfterDedupCurHistories(t *testing.T) { + server, closeFn, dc, informers, clientset := setup(t) + defer closeFn() + ns := framework.CreateTestingNamespace("one-node-daemonset-test", server, t) + defer framework.DeleteTestingNamespace(ns, server, t) + + dsClient := clientset.AppsV1().DaemonSets(ns.Name) + podInformer := informers.Core().V1().Pods().Informer() + nodeClient := clientset.CoreV1().Nodes() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + informers.Start(ctx.Done()) + go dc.Run(ctx, 5) + + // Start Scheduler + setupScheduler(ctx, t, clientset, informers) + + // Create single node + _, err := nodeClient.Create(context.TODO(), newNode("single-node", nil), metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Failed to create node: %v", err) + } + + // Create new DaemonSet with RollingUpdate strategy + orgDs := newDaemonSet("foo", ns.Name) + oneIntString := intstr.FromInt(1) + orgDs.Spec.UpdateStrategy = apps.DaemonSetUpdateStrategy{ + Type: apps.RollingUpdateDaemonSetStrategyType, + RollingUpdate: &apps.RollingUpdateDaemonSet{ + MaxUnavailable: &oneIntString, + }, + } + ds, err := dsClient.Create(context.TODO(), orgDs, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Failed to create DaemonSet: %v", err) + } + t.Logf("ds created") + // Wait for the DaemonSet to be created before proceeding + err = waitForDaemonSetAndControllerRevisionCreated(clientset, ds.Name, ds.Namespace) + if err != nil { + t.Fatalf("Failed to create DaemonSet: %v", err) + } + + ds, err = dsClient.Get(context.TODO(), ds.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get DaemonSet: %v", err) + } + + // Look up the ControllerRevision for the DaemonSet + _, name := hashAndNameForDaemonSet(ds) + revision, err := clientset.AppsV1().ControllerRevisions(ds.Namespace).Get(context.TODO(), name, metav1.GetOptions{}) + if err != nil || revision == nil { + t.Fatalf("Failed to look up ControllerRevision: %v", err) + } + t.Logf("revision: %v", revision.Name) + + // Create a "fake" ControllerRevision which is owned by the same daemonset + one := int64(1) + ds.Spec.Template.Spec.TerminationGracePeriodSeconds = &one + + newHash, newName := hashAndNameForDaemonSet(ds) + newRevision := &apps.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: newName, + Namespace: ds.Namespace, + Labels: labelsutil.CloneAndAddLabel(ds.Spec.Template.Labels, apps.DefaultDaemonSetUniqueLabelKey, newHash), + Annotations: ds.Annotations, + OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(ds, apps.SchemeGroupVersion.WithKind("DaemonSet"))}, + }, + Data: revision.Data, + Revision: revision.Revision + 1, + } + _, err = clientset.AppsV1().ControllerRevisions(ds.Namespace).Create(context.TODO(), newRevision, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Failed to create ControllerRevision: %v", err) + } + t.Logf("revision: %v", newName) + + // ensure the daemonset to be synced + _, err = nodeClient.Create(context.TODO(), newNode("second-node", nil), metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Failed to create node: %v", err) + } + + // check whether the pod label is updated after controllerrevision is created + err = wait.PollImmediate(100*time.Millisecond, 10*time.Second, func() (bool, error) { + objects := podInformer.GetIndexer().List() + for _, object := range objects { + pod := object.(*v1.Pod) + t.Logf("newHash: %v, label: %v", newHash, pod.ObjectMeta.Labels[apps.DefaultDaemonSetUniqueLabelKey]) + for _, oref := range pod.OwnerReferences { + if oref.Name == ds.Name && oref.UID == ds.UID && oref.Kind == "DaemonSet" { + if pod.ObjectMeta.Labels[apps.DefaultDaemonSetUniqueLabelKey] != newHash { + return false, nil + } + } + } + } + return true, nil + }) + if err != nil { + t.Fatalf("Failed to update the pod label after new controllerrevision is created: %v", err) + } + + revs, err := clientset.AppsV1().ControllerRevisions(ds.Namespace).List(context.TODO(), metav1.ListOptions{}) + if err != nil { + t.Fatalf("Failed to list controllerrevision: %v", err) + } + if revs.Size() == 0 { + t.Fatalf("No avaialable controllerrevision") + } + + for _, rev := range revs.Items { + t.Logf("revision: %v;hash: %v", rev.Name, rev.ObjectMeta.Labels[apps.DefaultDaemonSetUniqueLabelKey]) + for _, oref := range rev.OwnerReferences { + if oref.Kind == "DaemonSet" && oref.UID == ds.UID { + if rev.Name != newName { + t.Fatalf("duplicate controllerrevision is not deleted") + } + } + } + } +} + // TestTaintedNode tests tainted node isn't expected to have pod scheduled func TestTaintedNode(t *testing.T) { forEachStrategy(t, func(t *testing.T, strategy *apps.DaemonSetUpdateStrategy) {