From 1d1c6f19b4167e0a52a04c8d4fad21f74f717120 Mon Sep 17 00:00:00 2001 From: Janet Kuo Date: Tue, 23 Feb 2016 14:14:22 -0800 Subject: [PATCH 1/5] Fix the nil pointer dereference in addHashKeyToRSAndPods --- pkg/util/deployment/deployment.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/util/deployment/deployment.go b/pkg/util/deployment/deployment.go index cfa84f79c6b..5396325d769 100644 --- a/pkg/util/deployment/deployment.go +++ b/pkg/util/deployment/deployment.go @@ -172,6 +172,7 @@ func rsAndPodsWithHashKeySynced(deployment extensions.Deployment, c clientset.In // 2. Add hash label to all pods this rs owns // 3. Add hash label to the rs's label and selector func addHashKeyToRSAndPods(deployment extensions.Deployment, c clientset.Interface, rs extensions.ReplicaSet, getPodList podListFunc) (updatedRS *extensions.ReplicaSet, err error) { + updatedRS = &rs // If the rs already has the new hash label in its selector, it's done syncing namespace := deployment.Namespace hash := fmt.Sprintf("%d", podutil.GetPodTemplateSpecHash(api.PodTemplateSpec{ @@ -179,7 +180,7 @@ func addHashKeyToRSAndPods(deployment extensions.Deployment, c clientset.Interfa Spec: rs.Spec.Template.Spec, })) if labelsutil.SelectorHasLabel(rs.Spec.Selector, extensions.DefaultDeploymentUniqueLabelKey) { - return &rs, nil + return } // 1. Add hash template label to the rs. This ensures that any newly created pods will have the new label. if len(rs.Spec.Template.Labels[extensions.DefaultDeploymentUniqueLabelKey]) == 0 { From 020ab8813d77f235245f95efc16efc622ec38cec Mon Sep 17 00:00:00 2001 From: Janet Kuo Date: Tue, 23 Feb 2016 18:07:04 -0800 Subject: [PATCH 2/5] Check observed generation only after rs template is labeled in addHashKeyToRSAndPods --- pkg/util/deployment/deployment.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/util/deployment/deployment.go b/pkg/util/deployment/deployment.go index 5396325d769..4694cfd2e4d 100644 --- a/pkg/util/deployment/deployment.go +++ b/pkg/util/deployment/deployment.go @@ -190,11 +190,11 @@ func addHashKeyToRSAndPods(deployment extensions.Deployment, c clientset.Interfa if err != nil { return nil, fmt.Errorf("error updating rs %s pod template label with template hash: %v", updatedRS.Name, err) } - } - // Make sure rs pod template is updated so that it won't create pods without the new label (orphaned pods). - if updatedRS.Generation > updatedRS.Status.ObservedGeneration { - if err = waitForReplicaSetUpdated(c, updatedRS.Generation, namespace, rs.Name); err != nil { - return nil, fmt.Errorf("error waiting for rs %s generation %d observed by controller: %v", updatedRS.Name, updatedRS.Generation, err) + // Make sure rs pod template is updated so that it won't create pods without the new label (orphaned pods). + if updatedRS.Generation > updatedRS.Status.ObservedGeneration { + if err = waitForReplicaSetUpdated(c, updatedRS.Generation, namespace, rs.Name); err != nil { + return nil, fmt.Errorf("error waiting for rs %s generation %d observed by controller: %v", updatedRS.Name, updatedRS.Generation, err) + } } } From 082702390a07dc5ced075f3b685dd8e1586375f3 Mon Sep 17 00:00:00 2001 From: Janet Kuo Date: Wed, 24 Feb 2016 10:19:27 -0800 Subject: [PATCH 3/5] Fix test flake --- pkg/util/deployment/deployment.go | 10 +++++----- test/e2e/util.go | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/util/deployment/deployment.go b/pkg/util/deployment/deployment.go index 4694cfd2e4d..bf8ee414b1b 100644 --- a/pkg/util/deployment/deployment.go +++ b/pkg/util/deployment/deployment.go @@ -183,8 +183,8 @@ func addHashKeyToRSAndPods(deployment extensions.Deployment, c clientset.Interfa return } // 1. Add hash template label to the rs. This ensures that any newly created pods will have the new label. - if len(rs.Spec.Template.Labels[extensions.DefaultDeploymentUniqueLabelKey]) == 0 { - updatedRS, err = updateRSWithRetries(c.Extensions().ReplicaSets(namespace), &rs, func(updated *extensions.ReplicaSet) { + if len(updatedRS.Spec.Template.Labels[extensions.DefaultDeploymentUniqueLabelKey]) == 0 { + updatedRS, err = updateRSWithRetries(c.Extensions().ReplicaSets(namespace), updatedRS, func(updated *extensions.ReplicaSet) { updated.Spec.Template.Labels = labelsutil.AddLabel(updated.Spec.Template.Labels, extensions.DefaultDeploymentUniqueLabelKey, hash) }) if err != nil { @@ -192,14 +192,14 @@ func addHashKeyToRSAndPods(deployment extensions.Deployment, c clientset.Interfa } // Make sure rs pod template is updated so that it won't create pods without the new label (orphaned pods). if updatedRS.Generation > updatedRS.Status.ObservedGeneration { - if err = waitForReplicaSetUpdated(c, updatedRS.Generation, namespace, rs.Name); err != nil { + if err = waitForReplicaSetUpdated(c, updatedRS.Generation, namespace, updatedRS.Name); err != nil { return nil, fmt.Errorf("error waiting for rs %s generation %d observed by controller: %v", updatedRS.Name, updatedRS.Generation, err) } } } // 2. Update all pods managed by the rs to have the new hash label, so they will be correctly adopted. - selector, err := unversioned.LabelSelectorAsSelector(rs.Spec.Selector) + selector, err := unversioned.LabelSelectorAsSelector(updatedRS.Spec.Selector) if err != nil { return nil, err } @@ -214,7 +214,7 @@ func addHashKeyToRSAndPods(deployment extensions.Deployment, c clientset.Interfa // 3. Update rs label and selector to include the new hash label // Copy the old selector, so that we can scrub out any orphaned pods - if updatedRS, err = updateRSWithRetries(c.Extensions().ReplicaSets(namespace), &rs, func(updated *extensions.ReplicaSet) { + if updatedRS, err = updateRSWithRetries(c.Extensions().ReplicaSets(namespace), updatedRS, func(updated *extensions.ReplicaSet) { updated.Labels = labelsutil.AddLabel(updated.Labels, extensions.DefaultDeploymentUniqueLabelKey, hash) updated.Spec.Selector = labelsutil.AddLabelToSelector(updated.Spec.Selector, extensions.DefaultDeploymentUniqueLabelKey, hash) }); err != nil { diff --git a/test/e2e/util.go b/test/e2e/util.go index a6bba156346..d271a04a38f 100644 --- a/test/e2e/util.go +++ b/test/e2e/util.go @@ -2227,11 +2227,11 @@ func waitForDeploymentOldRSsNum(c *clientset.Clientset, ns, deploymentName strin } func logReplicaSetsOfDeployment(deployment *extensions.Deployment, allOldRSs []*extensions.ReplicaSet, newRS *extensions.ReplicaSet) { - Logf("Deployment = %+v", deployment) + Logf("Deployment: %+v. Selector = %+v", deployment, deployment.Spec.Selector) for i := range allOldRSs { - Logf("All old ReplicaSets (%d/%d) of deployment %s: %+v", i+1, len(allOldRSs), deployment.Name, allOldRSs[i]) + Logf("All old ReplicaSets (%d/%d) of deployment %s: %+v. Selector = %+v", i+1, len(allOldRSs), deployment.Name, allOldRSs[i], allOldRSs[i].Spec.Selector) } - Logf("New ReplicaSet of deployment %s: %+v", deployment.Name, newRS) + Logf("New ReplicaSet of deployment %s: %+v. Selector = %+v", deployment.Name, newRS, newRS.Spec.Selector) } func waitForObservedDeployment(c *clientset.Clientset, ns, deploymentName string, desiredGeneration int64) error { From 96908d6de5f3069157942043c4dbf973bb80f95c Mon Sep 17 00:00:00 2001 From: Janet Kuo Date: Wed, 24 Feb 2016 16:41:26 -0800 Subject: [PATCH 4/5] Add more logs --- pkg/util/deployment/deployment.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/util/deployment/deployment.go b/pkg/util/deployment/deployment.go index bf8ee414b1b..498d52b7923 100644 --- a/pkg/util/deployment/deployment.go +++ b/pkg/util/deployment/deployment.go @@ -21,6 +21,8 @@ import ( "strconv" "time" + "github.com/golang/glog" + "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/unversioned" "k8s.io/kubernetes/pkg/apis/extensions" @@ -196,6 +198,7 @@ func addHashKeyToRSAndPods(deployment extensions.Deployment, c clientset.Interfa return nil, fmt.Errorf("error waiting for rs %s generation %d observed by controller: %v", updatedRS.Name, updatedRS.Generation, err) } } + glog.V(4).Infof("Observed the update of rs %s's pod template with hash %s.", rs.Name, hash) } // 2. Update all pods managed by the rs to have the new hash label, so they will be correctly adopted. @@ -211,6 +214,7 @@ func addHashKeyToRSAndPods(deployment extensions.Deployment, c clientset.Interfa if err = labelPodsWithHash(podList, c, namespace, hash); err != nil { return nil, err } + glog.V(4).Infof("Labeled rs %s's pods with hash %s.", rs.Name, hash) // 3. Update rs label and selector to include the new hash label // Copy the old selector, so that we can scrub out any orphaned pods @@ -220,6 +224,7 @@ func addHashKeyToRSAndPods(deployment extensions.Deployment, c clientset.Interfa }); err != nil { return nil, fmt.Errorf("error updating rs %s label and selector with template hash: %v", updatedRS.Name, err) } + glog.V(4).Infof("Updated rs %s's selector and label with hash %s.", rs.Name, hash) // TODO: look for orphaned pods and label them in the background somewhere else periodically @@ -246,6 +251,7 @@ func labelPodsWithHash(podList *api.PodList, c clientset.Interface, namespace, h }); err != nil { return err } + glog.V(4).Infof("Labeled pod %s with hash %s.", pod.Name, hash) } } return nil From 3ed8426a984b71323a2fbea147530e0b7311c6a9 Mon Sep 17 00:00:00 2001 From: Janet Kuo Date: Wed, 24 Feb 2016 18:09:59 -0800 Subject: [PATCH 5/5] When calculating pod template hash, don't include the hash label --- pkg/util/deployment/deployment.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/util/deployment/deployment.go b/pkg/util/deployment/deployment.go index 498d52b7923..e4ee7b766d4 100644 --- a/pkg/util/deployment/deployment.go +++ b/pkg/util/deployment/deployment.go @@ -176,14 +176,16 @@ func rsAndPodsWithHashKeySynced(deployment extensions.Deployment, c clientset.In func addHashKeyToRSAndPods(deployment extensions.Deployment, c clientset.Interface, rs extensions.ReplicaSet, getPodList podListFunc) (updatedRS *extensions.ReplicaSet, err error) { updatedRS = &rs // If the rs already has the new hash label in its selector, it's done syncing - namespace := deployment.Namespace - hash := fmt.Sprintf("%d", podutil.GetPodTemplateSpecHash(api.PodTemplateSpec{ - ObjectMeta: rs.Spec.Template.ObjectMeta, - Spec: rs.Spec.Template.Spec, - })) if labelsutil.SelectorHasLabel(rs.Spec.Selector, extensions.DefaultDeploymentUniqueLabelKey) { return } + namespace := deployment.Namespace + meta := rs.Spec.Template.ObjectMeta + meta.Labels = labelsutil.CloneAndRemoveLabel(meta.Labels, extensions.DefaultDeploymentUniqueLabelKey) + hash := fmt.Sprintf("%d", podutil.GetPodTemplateSpecHash(api.PodTemplateSpec{ + ObjectMeta: meta, + Spec: rs.Spec.Template.Spec, + })) // 1. Add hash template label to the rs. This ensures that any newly created pods will have the new label. if len(updatedRS.Spec.Template.Labels[extensions.DefaultDeploymentUniqueLabelKey]) == 0 { updatedRS, err = updateRSWithRetries(c.Extensions().ReplicaSets(namespace), updatedRS, func(updated *extensions.ReplicaSet) {