Avoid comparing pod-template-hash when getting a deployment's new/old RSes

pull/6/head
Janet Kuo 2016-06-03 10:53:14 -07:00
parent 9f4e30ba88
commit c3d905776e
3 changed files with 225 additions and 3 deletions

View File

@ -140,11 +140,34 @@ func ListPods(deployment *extensions.Deployment, getPodList podListFunc) (*api.P
return getPodList(namespace, options)
}
// equalIgnoreHash returns true if two given podTemplateSpec are equal, ignoring the diff in value of Labels[pod-template-hash]
// We ignore pod-template-hash because the hash result would be different upon podTemplateSpec API changes
// (e.g. the addition of a new field will cause the hash code to change)
// Note that we assume input podTemplateSpecs contain non-empty labels
func equalIgnoreHash(template1, template2 api.PodTemplateSpec) (bool, error) {
// The podTemplateSpec must have a non-empty label so that label selectors can find them.
// This is checked by validation (of resources contain a podTemplateSpec).
if len(template1.Labels) == 0 || len(template2.Labels) == 0 {
return false, fmt.Errorf("Unexpected empty labels found in given template")
}
hash1 := template1.Labels[extensions.DefaultDeploymentUniqueLabelKey]
hash2 := template2.Labels[extensions.DefaultDeploymentUniqueLabelKey]
// compare equality ignoring pod-template-hash
template1.Labels[extensions.DefaultDeploymentUniqueLabelKey] = hash2
result := api.Semantic.DeepEqual(template1, template2)
template1.Labels[extensions.DefaultDeploymentUniqueLabelKey] = hash1
return result, nil
}
// FindNewReplicaSet returns the new RS this given deployment targets (the one with the same pod template).
func FindNewReplicaSet(deployment *extensions.Deployment, rsList []extensions.ReplicaSet) (*extensions.ReplicaSet, error) {
newRSTemplate := GetNewReplicaSetTemplate(deployment)
for i := range rsList {
if api.Semantic.DeepEqual(rsList[i].Spec.Template, newRSTemplate) {
equal, err := equalIgnoreHash(rsList[i].Spec.Template, newRSTemplate)
if err != nil {
return nil, err
}
if equal {
// This is the new ReplicaSet.
return &rsList[i], nil
}
@ -169,7 +192,11 @@ func FindOldReplicaSets(deployment *extensions.Deployment, rsList []extensions.R
return nil, nil, fmt.Errorf("invalid label selector: %v", err)
}
// Filter out replica set that has the same pod template spec as the deployment - that is the new replica set.
if api.Semantic.DeepEqual(rs.Spec.Template, newRSTemplate) {
equal, err := equalIgnoreHash(rs.Spec.Template, newRSTemplate)
if err != nil {
return nil, nil, err
}
if equal {
continue
}
allOldRSs[rs.ObjectMeta.Name] = rs

View File

@ -168,6 +168,9 @@ func generateRSWithLabel(labels map[string]string, image string) extensions.Repl
Replicas: 1,
Selector: &unversioned.LabelSelector{MatchLabels: labels},
Template: api.PodTemplateSpec{
ObjectMeta: api.ObjectMeta{
Labels: labels,
},
Spec: api.PodSpec{
Containers: []api.Container{
{
@ -388,6 +391,190 @@ func TestGetOldRCs(t *testing.T) {
}
}
func generatePodTemplateSpec(name, nodeName string, annotations, labels map[string]string) api.PodTemplateSpec {
return api.PodTemplateSpec{
ObjectMeta: api.ObjectMeta{
Name: name,
Annotations: annotations,
Labels: labels,
},
Spec: api.PodSpec{
NodeName: nodeName,
},
}
}
func TestEqualIgnoreHash(t *testing.T) {
tests := []struct {
test string
former, latter api.PodTemplateSpec
expected bool
}{
{
"Same spec, same labels",
generatePodTemplateSpec("foo", "foo-node", map[string]string{}, map[string]string{extensions.DefaultDeploymentUniqueLabelKey: "value-1", "something": "else"}),
generatePodTemplateSpec("foo", "foo-node", map[string]string{}, map[string]string{extensions.DefaultDeploymentUniqueLabelKey: "value-1", "something": "else"}),
true,
},
{
"Same spec, only pod-template-hash label value is different",
generatePodTemplateSpec("foo", "foo-node", map[string]string{}, map[string]string{extensions.DefaultDeploymentUniqueLabelKey: "value-1", "something": "else"}),
generatePodTemplateSpec("foo", "foo-node", map[string]string{}, map[string]string{extensions.DefaultDeploymentUniqueLabelKey: "value-2", "something": "else"}),
true,
},
{
"Same spec, the former doesn't have pod-template-hash label",
generatePodTemplateSpec("foo", "foo-node", map[string]string{}, map[string]string{"something": "else"}),
generatePodTemplateSpec("foo", "foo-node", map[string]string{}, map[string]string{extensions.DefaultDeploymentUniqueLabelKey: "value-2", "something": "else"}),
true,
},
{
"Same spec, the label is different, and the pod-template-hash label value is the same",
generatePodTemplateSpec("foo", "foo-node", map[string]string{}, map[string]string{extensions.DefaultDeploymentUniqueLabelKey: "value-1"}),
generatePodTemplateSpec("foo", "foo-node", map[string]string{}, map[string]string{extensions.DefaultDeploymentUniqueLabelKey: "value-1", "something": "else"}),
false,
},
{
"Different spec, same labels",
generatePodTemplateSpec("foo", "foo-node", map[string]string{"former": "value"}, map[string]string{extensions.DefaultDeploymentUniqueLabelKey: "value-1", "something": "else"}),
generatePodTemplateSpec("foo", "foo-node", map[string]string{"latter": "value"}, map[string]string{extensions.DefaultDeploymentUniqueLabelKey: "value-1", "something": "else"}),
false,
},
{
"Different spec, different pod-template-hash label value",
generatePodTemplateSpec("foo-1", "foo-node", map[string]string{}, map[string]string{extensions.DefaultDeploymentUniqueLabelKey: "value-1", "something": "else"}),
generatePodTemplateSpec("foo-2", "foo-node", map[string]string{}, map[string]string{extensions.DefaultDeploymentUniqueLabelKey: "value-2", "something": "else"}),
false,
},
{
"Different spec, the former doesn't have pod-template-hash label",
generatePodTemplateSpec("foo-1", "foo-node-1", map[string]string{}, map[string]string{"something": "else"}),
generatePodTemplateSpec("foo-2", "foo-node-2", map[string]string{}, map[string]string{extensions.DefaultDeploymentUniqueLabelKey: "value-2", "something": "else"}),
false,
},
{
"Different spec, different labels",
generatePodTemplateSpec("foo", "foo-node-1", map[string]string{}, map[string]string{"something": "else"}),
generatePodTemplateSpec("foo", "foo-node-2", map[string]string{}, map[string]string{"nothing": "else"}),
false,
},
}
for _, test := range tests {
equal, err := equalIgnoreHash(test.former, test.latter)
if err != nil {
t.Errorf("In test case %q, returned unexpected error %v", test.test, err)
}
if equal != test.expected {
t.Errorf("In test case %q, expected %v", test.test, test.expected)
}
equal, err = equalIgnoreHash(test.latter, test.former)
if err != nil {
t.Errorf("In test case %q (reverse order), returned unexpected error %v", test.test, err)
}
if equal != test.expected {
t.Errorf("In test case %q (reverse order), expected %v", test.test, test.expected)
}
}
}
func TestFindNewReplicaSet(t *testing.T) {
deployment := generateDeployment("nginx")
newRS := generateRS(deployment)
newRS.Labels[extensions.DefaultDeploymentUniqueLabelKey] = "different-hash"
oldDeployment := generateDeployment("nginx")
oldDeployment.Spec.Template.Spec.Containers[0].Name = "nginx-old-1"
oldRS := generateRS(oldDeployment)
oldRS.Status.FullyLabeledReplicas = oldRS.Spec.Replicas
tests := []struct {
test string
deployment extensions.Deployment
rsList []extensions.ReplicaSet
expected *extensions.ReplicaSet
}{
{
test: "Get new ReplicaSet with the same spec but different pod-template-hash value",
deployment: deployment,
rsList: []extensions.ReplicaSet{newRS, oldRS},
expected: &newRS,
},
{
test: "Get nil new ReplicaSet",
deployment: deployment,
rsList: []extensions.ReplicaSet{oldRS},
expected: nil,
},
}
for _, test := range tests {
if rs, err := FindNewReplicaSet(&test.deployment, test.rsList); !reflect.DeepEqual(rs, test.expected) || err != nil {
t.Errorf("In test case %q, expected %#v, got %#v: %v", test.test, test.expected, rs, err)
}
}
}
func TestFindOldReplicaSets(t *testing.T) {
deployment := generateDeployment("nginx")
newRS := generateRS(deployment)
newRS.Labels[extensions.DefaultDeploymentUniqueLabelKey] = "different-hash"
oldDeployment := generateDeployment("nginx")
oldDeployment.Spec.Template.Spec.Containers[0].Name = "nginx-old-1"
oldRS := generateRS(oldDeployment)
oldRS.Status.FullyLabeledReplicas = oldRS.Spec.Replicas
newPod := generatePodFromRS(newRS)
oldPod := generatePodFromRS(oldRS)
tests := []struct {
test string
deployment extensions.Deployment
rsList []extensions.ReplicaSet
podList *api.PodList
expected []*extensions.ReplicaSet
}{
{
test: "Get old ReplicaSets",
deployment: deployment,
rsList: []extensions.ReplicaSet{newRS, oldRS},
podList: &api.PodList{
Items: []api.Pod{
newPod,
oldPod,
},
},
expected: []*extensions.ReplicaSet{&oldRS},
},
{
test: "Get old ReplicaSets with no new ReplicaSet",
deployment: deployment,
rsList: []extensions.ReplicaSet{oldRS},
podList: &api.PodList{
Items: []api.Pod{
oldPod,
},
},
expected: []*extensions.ReplicaSet{&oldRS},
},
{
test: "Get empty old ReplicaSets",
deployment: deployment,
rsList: []extensions.ReplicaSet{newRS},
podList: &api.PodList{
Items: []api.Pod{
newPod,
},
},
expected: []*extensions.ReplicaSet{},
},
}
for _, test := range tests {
if old, _, err := FindOldReplicaSets(&test.deployment, test.rsList, test.podList); !reflect.DeepEqual(old, test.expected) || err != nil {
t.Errorf("In test case %q, expected %#v, got %#v: %v", test.test, test.expected, old, err)
}
}
}
// equal compares the equality of two ReplicaSet slices regardless of their ordering
func equal(rss1, rss2 []*extensions.ReplicaSet) bool {
if reflect.DeepEqual(rss1, rss2) {

View File

@ -62,6 +62,7 @@ import (
"k8s.io/kubernetes/pkg/types"
"k8s.io/kubernetes/pkg/util"
deploymentutil "k8s.io/kubernetes/pkg/util/deployment"
labelsutil "k8s.io/kubernetes/pkg/util/labels"
"k8s.io/kubernetes/pkg/util/sets"
"k8s.io/kubernetes/pkg/util/wait"
utilyaml "k8s.io/kubernetes/pkg/util/yaml"
@ -2873,6 +2874,12 @@ func WaitForDeploymentStatus(c clientset.Interface, ns, deploymentName string, d
return false, nil
}
allRSs = append(oldRSs, newRS)
// The old/new ReplicaSets need to contain the pod-template-hash label
for i := range allRSs {
if !labelsutil.SelectorHasLabel(allRSs[i].Spec.Selector, extensions.DefaultDeploymentUniqueLabelKey) {
return false, nil
}
}
totalCreated := deploymentutil.GetReplicaCountForReplicaSets(allRSs)
totalAvailable, err := deploymentutil.GetAvailablePodsForReplicaSets(c, allRSs, minReadySeconds)
if err != nil {
@ -2958,8 +2965,9 @@ func WaitForDeploymentRevisionAndImage(c clientset.Interface, ns, deploymentName
if err != nil {
return false, err
}
// The new ReplicaSet needs to be non-nil and contain the pod-template-hash label
newRS, err = deploymentutil.GetNewReplicaSet(deployment, c)
if err != nil || newRS == nil {
if err != nil || newRS == nil || !labelsutil.SelectorHasLabel(newRS.Spec.Selector, extensions.DefaultDeploymentUniqueLabelKey) {
return false, err
}
// Check revision of this deployment, and of the new replica set of this deployment