Fix the map concurrent read/write issue in deployment controller

This commit is contained in:
Janet Kuo 2016-08-03 11:19:12 -07:00
parent 2301ab7e0e
commit 2d1490a024
2 changed files with 47 additions and 19 deletions

View File

@ -402,17 +402,26 @@ func ListPods(deployment *extensions.Deployment, getPodList podListFunc) (*api.P
// (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) {
// First, compare template.Labels (ignoring hash)
labels1, labels2 := template1.Labels, template2.Labels
// 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 {
if len(labels1) == 0 || len(labels2) == 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
if len(labels1) > len(labels2) {
labels1, labels2 = labels2, labels1
}
// We make sure len(labels2) >= len(labels1)
for k, v := range labels2 {
if labels1[k] != v && k != extensions.DefaultDeploymentUniqueLabelKey {
return false, nil
}
}
// Then, compare the templates without comparing their labels
template1.Labels, template2.Labels = nil, nil
result := api.Semantic.DeepEqual(template1, template2)
template1.Labels[extensions.DefaultDeploymentUniqueLabelKey] = hash1
return result, nil
}

View File

@ -464,20 +464,39 @@ func TestEqualIgnoreHash(t *testing.T) {
}
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)
runTest := func(t1, t2 api.PodTemplateSpec, reversed bool) {
// Set up
t1Copy, err := api.Scheme.DeepCopy(t1)
if err != nil {
t.Errorf("Failed setting up the test: %v", err)
}
t2Copy, err := api.Scheme.DeepCopy(t2)
if err != nil {
t.Errorf("Failed setting up the test: %v", err)
}
reverseString := ""
if reversed {
reverseString = " (reverse order)"
}
// Run
equal, err := equalIgnoreHash(t1, t2)
// Check
if err != nil {
t.Errorf("In test case %q%s, expected no error, returned %v", test.test, reverseString, err)
}
if equal != test.expected {
t.Errorf("In test case %q%s, expected %v", test.test, reverseString, test.expected)
}
if t1.Labels == nil || t2.Labels == nil {
t.Errorf("In test case %q%s, unexpected labels becomes nil", test.test, reverseString)
}
if !reflect.DeepEqual(t1, t1Copy) || !reflect.DeepEqual(t2, t2Copy) {
t.Errorf("In test case %q%s, unexpected input template modified", test.test, reverseString)
}
}
runTest(test.former, test.latter, false)
// Test the same case in reverse order
runTest(test.latter, test.former, true)
}
}