From 87a240f828eeb7bbb04cffe9330c04b106571deb Mon Sep 17 00:00:00 2001
From: Janet Kuo <chiachenk@google.com>
Date: Tue, 1 Mar 2016 14:55:23 -0800
Subject: [PATCH] Improve deployment error logs

---
 test/e2e/deployment.go | 10 ++++------
 test/e2e/util.go       | 27 ++++++++++++++++++++++++++-
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/test/e2e/deployment.go b/test/e2e/deployment.go
index ffe52085b4..f843662e46 100644
--- a/test/e2e/deployment.go
+++ b/test/e2e/deployment.go
@@ -866,17 +866,15 @@ func testDeploymentLabelAdopted(f *Framework) {
 	// New RS should contain pod-template-hash in its selector, label, and template label
 	newRS, err := deploymentutil.GetNewReplicaSet(deployment, c)
 	Expect(err).NotTo(HaveOccurred())
-	Expect(len(newRS.Labels[extensions.DefaultDeploymentUniqueLabelKey])).Should(BeNumerically(">", 0))
-	Expect(len(newRS.Spec.Selector.MatchLabels[extensions.DefaultDeploymentUniqueLabelKey])).Should(BeNumerically(">", 0))
-	Expect(len(newRS.Spec.Template.Labels[extensions.DefaultDeploymentUniqueLabelKey])).Should(BeNumerically(">", 0))
+	err = checkRSHashLabel(newRS)
+	Expect(err).NotTo(HaveOccurred())
 	// All pods targeted by the deployment should contain pod-template-hash in their labels, and there should be only 3 pods
 	selector, err := unversioned.LabelSelectorAsSelector(deployment.Spec.Selector)
 	Expect(err).NotTo(HaveOccurred())
 	options := api.ListOptions{LabelSelector: selector}
 	pods, err := c.Core().Pods(ns).List(options)
 	Expect(err).NotTo(HaveOccurred())
-	for _, pod := range pods.Items {
-		Expect(len(pod.Labels[extensions.DefaultDeploymentUniqueLabelKey])).Should(BeNumerically(">", 0))
-	}
+	err = checkPodHashLabel(pods)
+	Expect(err).NotTo(HaveOccurred())
 	Expect(len(pods.Items)).Should(Equal(replicas))
 }
diff --git a/test/e2e/util.go b/test/e2e/util.go
index f74a61c900..7403393a8d 100644
--- a/test/e2e/util.go
+++ b/test/e2e/util.go
@@ -2410,7 +2410,7 @@ func waitForDeploymentRevisionAndImage(c clientset.Interface, ns, deploymentName
 		logReplicaSetsOfDeployment(deployment, nil, newRS)
 	}
 	if err != nil {
-		return fmt.Errorf("error waiting for deployment %s revision and image to match expectation: %v", deploymentName, err)
+		return fmt.Errorf("error waiting for deployment %s (got %s / %s) and new RS %s (got %s / %s) revision and image to match expectation (expected %s / %s): %v", deploymentName, deployment.Annotations[deploymentutil.RevisionAnnotation], deployment.Spec.Template.Spec.Containers[0].Image, newRS.Name, newRS.Annotations[deploymentutil.RevisionAnnotation], newRS.Spec.Template.Spec.Containers[0].Image, revision, image, err)
 	}
 	return nil
 }
@@ -3353,3 +3353,28 @@ func isElementOf(podUID types.UID, pods *api.PodList) bool {
 	}
 	return false
 }
+
+func checkRSHashLabel(rs *extensions.ReplicaSet) error {
+	if len(rs.Labels[extensions.DefaultDeploymentUniqueLabelKey]) == 0 ||
+		len(rs.Spec.Selector.MatchLabels[extensions.DefaultDeploymentUniqueLabelKey]) == 0 ||
+		len(rs.Spec.Template.Labels[extensions.DefaultDeploymentUniqueLabelKey]) == 0 {
+		return fmt.Errorf("unexpected RS missing required pod-hash-template: %+v, selector = %+v, template = %+v", rs, rs.Spec.Selector, rs.Spec.Template)
+	}
+	return nil
+}
+
+func checkPodHashLabel(pods *api.PodList) error {
+	invalidPod := ""
+	for _, pod := range pods.Items {
+		if len(pod.Labels[extensions.DefaultDeploymentUniqueLabelKey]) == 0 {
+			if len(invalidPod) == 0 {
+				invalidPod = "unexpected pods missing required pod-hash-template:"
+			}
+			invalidPod = fmt.Sprintf("%s %+v;", invalidPod, pod)
+		}
+	}
+	if len(invalidPod) > 0 {
+		return fmt.Errorf("%s", invalidPod)
+	}
+	return nil
+}