diff --git a/pkg/controller/history/controller_history.go b/pkg/controller/history/controller_history.go index e4201c65bc9..16934d6cd4e 100644 --- a/pkg/controller/history/controller_history.go +++ b/pkg/controller/history/controller_history.go @@ -54,15 +54,17 @@ func ControllerRevisionName(prefix string, hash uint32) string { return fmt.Sprintf("%s-%d", prefix, hash) } -// NewControllerRevision returns the a ControllerRevision with a ControllerRef pointing parent and indicating that +// NewControllerRevision returns a ControllerRevision with a ControllerRef pointing to parent and indicating that // parent is of parentKind. The ControllerRevision has labels matching selector, contains Data equal to data, and -// has a Revision equal to revision. If the returned error is nil, the returned ControllerRevision is valid. If the +// has a Revision equal to revision. The collisionCount is used when creating the name of the ControllerRevision +// so the name is likely unique. If the returned error is nil, the returned ControllerRevision is valid. If the // returned error is not nil, the returned ControllerRevision is invalid for use. func NewControllerRevision(parent metav1.Object, parentKind schema.GroupVersionKind, selector labels.Selector, data runtime.RawExtension, - revision int64) (*apps.ControllerRevision, error) { + revision int64, + collisionCount *int32) (*apps.ControllerRevision, error) { labelMap, err := labels.ConvertSelectorToLabelsMap(selector.String()) if err != nil { return nil, err @@ -86,7 +88,7 @@ func NewControllerRevision(parent metav1.Object, Data: data, Revision: revision, } - hash := HashControllerRevision(cr, nil) + hash := HashControllerRevision(cr, collisionCount) cr.Name = ControllerRevisionName(parent.GetName(), hash) cr.Labels[ControllerRevisionHashLabel] = strconv.FormatInt(int64(hash), 10) return cr, nil @@ -94,7 +96,7 @@ func NewControllerRevision(parent metav1.Object, // HashControllerRevision hashes the contents of revision's Data using FNV hashing. If probe is not nil, the byte value // of probe is added written to the hash as well. -func HashControllerRevision(revision *apps.ControllerRevision, probe *uint32) uint32 { +func HashControllerRevision(revision *apps.ControllerRevision, probe *int32) uint32 { hf := fnv.New32() if len(revision.Data.Raw) > 0 { hf.Write(revision.Data.Raw) @@ -177,11 +179,13 @@ type Interface interface { // returned error is not nil, the returned slice is not valid. ListControllerRevisions(parent metav1.Object, selector labels.Selector) ([]*apps.ControllerRevision, error) // CreateControllerRevision attempts to create the revision as owned by parent via a ControllerRef. If name - // collision occurs, a unique identifier is added to the hash of the revision and it is renamed using - // ControllerRevisionName. Implementations may cease to attempt to retry creation after some number of attempts - // and return an error. If the returned error is not nil, creation failed. If the returned error is nil, the - // returned ControllerRevision has been created. - CreateControllerRevision(parent metav1.Object, revision *apps.ControllerRevision) (*apps.ControllerRevision, error) + // collision occurs, collisionCount (incremented each time collision occurs except for the first time) is + // added to the hash of the revision and it is renamed using ControllerRevisionName. Implementations may + // cease to attempt to retry creation after some number of attempts and return an error. If the returned + // error is not nil, creation failed. If the returned error is nil, the returned ControllerRevision has been + // created. + // Callers must make sure that collisionCount is not nil. An error is returned if it is. + CreateControllerRevision(parent metav1.Object, revision *apps.ControllerRevision, collisionCount *int32) (*apps.ControllerRevision, error) // DeleteControllerRevision attempts to delete revision. If the returned error is not nil, deletion has failed. DeleteControllerRevision(revision *apps.ControllerRevision) error // UpdateControllerRevision updates revision such that its Revision is equal to newRevision. Implementations @@ -233,9 +237,10 @@ func (rh *realHistory) ListControllerRevisions(parent metav1.Object, selector la return owned, err } -func (rh *realHistory) CreateControllerRevision(parent metav1.Object, revision *apps.ControllerRevision) (*apps.ControllerRevision, error) { - // Initialize the probe to 0 - probe := uint32(0) +func (rh *realHistory) CreateControllerRevision(parent metav1.Object, revision *apps.ControllerRevision, collisionCount *int32) (*apps.ControllerRevision, error) { + if collisionCount == nil { + return nil, fmt.Errorf("collisionCount should not be nil") + } // Clone the input any, err := scheme.Scheme.DeepCopy(revision) @@ -246,18 +251,12 @@ func (rh *realHistory) CreateControllerRevision(parent metav1.Object, revision * // Continue to attempt to create the revision updating the name with a new hash on each iteration for { - var hash uint32 - // The first attempt uses no probe to resolve collisions - if probe > 0 { - hash = HashControllerRevision(revision, &probe) - } else { - hash = HashControllerRevision(revision, nil) - } + hash := HashControllerRevision(revision, collisionCount) // Update the revisions name and labels clone.Name = ControllerRevisionName(parent.GetName(), hash) created, err := rh.client.AppsV1beta1().ControllerRevisions(parent.GetNamespace()).Create(clone) if errors.IsAlreadyExists(err) { - probe++ + *collisionCount++ continue } return created, err @@ -370,9 +369,10 @@ func (fh *fakeHistory) addRevision(revision *apps.ControllerRevision) (*apps.Con return revision, fh.indexer.Update(revision) } -func (fh *fakeHistory) CreateControllerRevision(parent metav1.Object, revision *apps.ControllerRevision) (*apps.ControllerRevision, error) { - // Initialize the probe to 0 - probe := uint32(0) +func (fh *fakeHistory) CreateControllerRevision(parent metav1.Object, revision *apps.ControllerRevision, collisionCount *int32) (*apps.ControllerRevision, error) { + if collisionCount == nil { + return nil, fmt.Errorf("collisionCount should not be nil") + } // Clone the input any, err := scheme.Scheme.DeepCopy(revision) @@ -384,18 +384,12 @@ func (fh *fakeHistory) CreateControllerRevision(parent metav1.Object, revision * // Continue to attempt to create the revision updating the name with a new hash on each iteration for { - var hash uint32 - // The first attempt uses no probe to resolve collisions - if probe > 0 { - hash = HashControllerRevision(revision, &probe) - } else { - hash = HashControllerRevision(revision, nil) - } + hash := HashControllerRevision(revision, collisionCount) // Update the revisions name and labels clone.Name = ControllerRevisionName(parent.GetName(), hash) created, err := fh.addRevision(clone) if errors.IsAlreadyExists(err) { - probe++ + *collisionCount++ continue } return created, err diff --git a/pkg/controller/history/controller_history_test.go b/pkg/controller/history/controller_history_test.go index ab5845eb342..5e9c1eadf15 100644 --- a/pkg/controller/history/controller_history_test.go +++ b/pkg/controller/history/controller_history_test.go @@ -85,22 +85,22 @@ func TestRealHistory_ListControllerRevisions(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, nil) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } ss2Rev1.Namespace = ss2.Namespace - ss1Orphan, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 3) + ss1Orphan, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 3, nil) if err != nil { t.Fatal(err) } @@ -186,22 +186,22 @@ func TestFakeHistory_ListControllerRevisions(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, nil) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } ss2Rev1.Namespace = ss2.Namespace - ss1Orphan, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 3) + ss1Orphan, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 3, nil) if err != nil { t.Fatal(err) } @@ -257,33 +257,53 @@ func TestRealHistory_CreateControllerRevision(t *testing.T) { testFn := func(test *testcase, t *testing.T) { client := fake.NewSimpleClientset() informerFactory := informers.NewSharedInformerFactory(client, controller.NoResyncPeriodFunc()) - stop := make(chan struct{}) defer close(stop) informerFactory.Start(stop) informer := informerFactory.Apps().V1beta1().ControllerRevisions() informerFactory.WaitForCacheSync(stop) history := NewHistory(client, informer.Lister()) + + var collisionCount int32 for i := range test.existing { - _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision) + _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision, &collisionCount) if err != nil { t.Fatal(err) } } - created, err := history.CreateControllerRevision(test.parent, test.revision) + // Clear collisionCount before creating the test revision + collisionCount = 0 + created, err := history.CreateControllerRevision(test.parent, test.revision, &collisionCount) if err != nil { t.Errorf("%s: %s", test.name, err) } - if test.rename && created.Name == test.revision.Name { - t.Errorf("%s: wanted rename got %s %s", test.name, created.Name, test.revision.Name) + if test.rename { + if created.Name == test.revision.Name { + t.Errorf("%s: wanted rename got %s %s", test.name, created.Name, test.revision.Name) + } + expectedName := ControllerRevisionName(test.parent.GetName(), HashControllerRevision(test.revision, &collisionCount)) + if created.Name != expectedName { + t.Errorf("%s: on name collision wanted new name %s got %s", test.name, expectedName, created.Name) + } + + // Second name collision should have incremented collisionCount to 2 + _, err = history.CreateControllerRevision(test.parent, test.revision, &collisionCount) + if err != nil { + t.Errorf("%s: %s", test.name, err) + } + if collisionCount != 2 { + t.Errorf("%s: on second name collision wanted collisionCount 1 got %d", test.name, collisionCount) + } } if !test.rename && created.Name != test.revision.Name { t.Errorf("%s: wanted %s got %s", test.name, test.revision.Name, created.Name) } } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) + ss1.Status.CollisionCount = new(int32) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) + ss2.Status.CollisionCount = new(int32) sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) if err != nil { t.Fatal(err) @@ -292,17 +312,17 @@ func TestRealHistory_CreateControllerRevision(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -374,26 +394,47 @@ func TestFakeHistory_CreateControllerRevision(t *testing.T) { informer := informerFactory.Apps().V1beta1().ControllerRevisions() informerFactory.WaitForCacheSync(stop) history := NewFakeHistory(informer) + + var collisionCount int32 for i := range test.existing { - _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision) + _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision, &collisionCount) if err != nil { t.Fatal(err) } } - created, err := history.CreateControllerRevision(test.parent, test.revision) + // Clear collisionCount before creating the test revision + collisionCount = 0 + created, err := history.CreateControllerRevision(test.parent, test.revision, &collisionCount) if err != nil { t.Errorf("%s: %s", test.name, err) } - if test.rename && created.Name == test.revision.Name { - t.Errorf("%s: wanted rename got %s %s", test.name, created.Name, test.revision.Name) + if test.rename { + if created.Name == test.revision.Name { + t.Errorf("%s: wanted rename got %s %s", test.name, created.Name, test.revision.Name) + } + expectedName := ControllerRevisionName(test.parent.GetName(), HashControllerRevision(test.revision, &collisionCount)) + if created.Name != expectedName { + t.Errorf("%s: on name collision wanted new name %s got %s", test.name, expectedName, created.Name) + } + + // Second name collision should have incremented collisionCount to 2 + _, err = history.CreateControllerRevision(test.parent, test.revision, &collisionCount) + if err != nil { + t.Errorf("%s: %s", test.name, err) + } + if collisionCount != 2 { + t.Errorf("%s: on second name collision wanted collisionCount 1 got %d", test.name, collisionCount) + } } if !test.rename && created.Name != test.revision.Name { t.Errorf("%s: wanted %s got %s", test.name, test.revision.Name, created.Name) } } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) + ss1.Status.CollisionCount = new(int32) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) + ss2.Status.CollisionCount = new(int32) sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) if err != nil { t.Fatal(err) @@ -402,17 +443,17 @@ func TestFakeHistory_CreateControllerRevision(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -511,8 +552,9 @@ func TestRealHistory_UpdateControllerRevision(t *testing.T) { informer := informerFactory.Apps().V1beta1().ControllerRevisions() informerFactory.WaitForCacheSync(stop) history := NewHistory(client, informer.Lister()) + var collisionCount int32 for i := range test.existing { - _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision) + _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision, &collisionCount) if err != nil { t.Fatal(err) } @@ -532,17 +574,18 @@ func TestRealHistory_UpdateControllerRevision(t *testing.T) { } } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) + ss1.Status.CollisionCount = new(int32) sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -641,8 +684,9 @@ func TestFakeHistory_UpdateControllerRevision(t *testing.T) { informer := informerFactory.Apps().V1beta1().ControllerRevisions() informerFactory.WaitForCacheSync(stop) history := NewFakeHistory(informer) + var collisionCount int32 for i := range test.existing { - _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision) + _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision, &collisionCount) if err != nil { t.Fatal(err) } @@ -659,17 +703,18 @@ func TestFakeHistory_UpdateControllerRevision(t *testing.T) { } } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) + ss1.Status.CollisionCount = new(int32) sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -731,8 +776,9 @@ func TestRealHistory_DeleteControllerRevision(t *testing.T) { informer := informerFactory.Apps().V1beta1().ControllerRevisions() informerFactory.WaitForCacheSync(stop) history := NewHistory(client, informer.Lister()) + var collisionCount int32 for i := range test.existing { - _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision) + _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision, &collisionCount) if err != nil { t.Fatal(err) } @@ -746,7 +792,9 @@ func TestRealHistory_DeleteControllerRevision(t *testing.T) { } } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) + ss1.Status.CollisionCount = new(int32) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) + ss2.Status.CollisionCount = new(int32) sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) if err != nil { t.Fatal(err) @@ -755,22 +803,22 @@ func TestRealHistory_DeleteControllerRevision(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } ss2Rev1.Namespace = ss2.Namespace - ss2Rev2, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 2) + ss2Rev2, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 2, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -839,8 +887,9 @@ func TestFakeHistory_DeleteControllerRevision(t *testing.T) { informer := informerFactory.Apps().V1beta1().ControllerRevisions() informerFactory.WaitForCacheSync(stop) history := NewFakeHistory(informer) + var collisionCount int32 for i := range test.existing { - _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision) + _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision, &collisionCount) if err != nil { t.Fatal(err) } @@ -854,7 +903,9 @@ func TestFakeHistory_DeleteControllerRevision(t *testing.T) { } } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) + ss1.Status.CollisionCount = new(int32) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) + ss2.Status.CollisionCount = new(int32) sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) if err != nil { t.Fatal(err) @@ -863,22 +914,22 @@ func TestFakeHistory_DeleteControllerRevision(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } ss2Rev1.Namespace = ss2.Namespace - ss2Rev2, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 2) + ss2Rev2, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 2, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -982,8 +1033,9 @@ func TestRealHistory_AdoptControllerRevision(t *testing.T) { informerFactory.WaitForCacheSync(stop) history := NewHistory(client, informer.Lister()) + var collisionCount int32 for i := range test.existing { - _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision) + _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision, &collisionCount) if err != nil { t.Fatal(err) } @@ -1001,7 +1053,9 @@ func TestRealHistory_AdoptControllerRevision(t *testing.T) { } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) + ss1.Status.CollisionCount = new(int32) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) + ss2.Status.CollisionCount = new(int32) sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) if err != nil { t.Fatal(err) @@ -1010,18 +1064,18 @@ func TestRealHistory_AdoptControllerRevision(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace ss1Rev2.OwnerReferences = []metav1.OwnerReference{} - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -1093,8 +1147,9 @@ func TestFakeHistory_AdoptControllerRevision(t *testing.T) { informerFactory.WaitForCacheSync(stop) history := NewFakeHistory(informer) + var collisionCount int32 for i := range test.existing { - _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision) + _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision, &collisionCount) if err != nil { t.Fatal(err) } @@ -1112,7 +1167,9 @@ func TestFakeHistory_AdoptControllerRevision(t *testing.T) { } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) + ss1.Status.CollisionCount = new(int32) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) + ss2.Status.CollisionCount = new(int32) sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) if err != nil { t.Fatal(err) @@ -1121,18 +1178,18 @@ func TestFakeHistory_AdoptControllerRevision(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace ss1Rev2.OwnerReferences = []metav1.OwnerReference{} - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -1243,8 +1300,9 @@ func TestRealHistory_ReleaseControllerRevision(t *testing.T) { informerFactory.WaitForCacheSync(stop) history := NewHistory(client, informer.Lister()) + var collisionCount int32 for i := range test.existing { - _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision) + _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision, &collisionCount) if err != nil { t.Fatal(err) } @@ -1276,18 +1334,18 @@ func TestRealHistory_ReleaseControllerRevision(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, nil) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace ss1Rev2.OwnerReferences = []metav1.OwnerReference{} - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, nil) if err != nil { t.Fatal(err) } @@ -1371,8 +1429,9 @@ func TestFakeHistory_ReleaseControllerRevision(t *testing.T) { informer := informerFactory.Apps().V1beta1().ControllerRevisions() informerFactory.WaitForCacheSync(stop) history := NewFakeHistory(informer) + var collisionCount int32 for i := range test.existing { - _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision) + _, err := history.CreateControllerRevision(test.existing[i].parent, test.existing[i].revision, &collisionCount) if err != nil { t.Fatal(err) } @@ -1395,7 +1454,9 @@ func TestFakeHistory_ReleaseControllerRevision(t *testing.T) { } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) + ss1.Status.CollisionCount = new(int32) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) + ss2.Status.CollisionCount = new(int32) sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) if err != nil { t.Fatal(err) @@ -1404,18 +1465,18 @@ func TestFakeHistory_ReleaseControllerRevision(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace ss1Rev2.OwnerReferences = []metav1.OwnerReference{} - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -1499,7 +1560,9 @@ func TestFindEqualRevisions(t *testing.T) { } } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) + ss1.Status.CollisionCount = new(int32) ss2 := newStatefulSet(3, "ss2", types.UID("ss2"), map[string]string{"goo": "car"}) + ss2.Status.CollisionCount = new(int32) sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) if err != nil { t.Fatal(err) @@ -1508,23 +1571,23 @@ func TestFindEqualRevisions(t *testing.T) { if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace ss1Rev2.OwnerReferences = []metav1.OwnerReference{} - ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1) + ss2Rev1, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 1, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } ss2Rev1.Namespace = ss2.Namespace - ss2Rev2, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 2) + ss2Rev2, err := NewControllerRevision(ss2, parentKind, sel2, rawTemplate(&ss2.Spec.Template), 2, ss2.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -1569,22 +1632,23 @@ func TestSortControllerRevisions(t *testing.T) { } } ss1 := newStatefulSet(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) + ss1.Status.CollisionCount = new(int32) sel1, err := metav1.LabelSelectorAsSelector(ss1.Spec.Selector) if err != nil { t.Fatal(err) } - ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1) + ss1Rev1, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev1.Namespace = ss1.Namespace - ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2) + ss1Rev2, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } ss1Rev2.Namespace = ss1.Namespace - ss1Rev3, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2) + ss1Rev3, err := NewControllerRevision(ss1, parentKind, sel1, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) if err != nil { t.Fatal(err) } diff --git a/pkg/controller/statefulset/stateful_set_control.go b/pkg/controller/statefulset/stateful_set_control.go index 637236b8768..0fe3ff9708c 100644 --- a/pkg/controller/statefulset/stateful_set_control.go +++ b/pkg/controller/statefulset/stateful_set_control.go @@ -80,13 +80,13 @@ func (ssc *defaultStatefulSetControl) UpdateStatefulSet(set *apps.StatefulSet, p history.SortControllerRevisions(revisions) // get the current, and update revisions - currentRevision, updateRevision, err := ssc.getStatefulSetRevisions(set, revisions) + currentRevision, updateRevision, collisionCount, err := ssc.getStatefulSetRevisions(set, revisions) if err != nil { return err } // perform the main update function and get the status - status, err := ssc.updateStatefulSet(set, currentRevision, updateRevision, pods) + status, err := ssc.updateStatefulSet(set, currentRevision, updateRevision, collisionCount, pods) if err != nil { return err } @@ -174,21 +174,31 @@ func (ssc *defaultStatefulSetControl) truncateHistory( return nil } -// getStatefulSetRevisions returns the current and update ControllerRevisions for set. This method may create a new revision, -// or modify the Revision of an existing revision if an update to set is detected. This method expects that revisions -// is sorted when supplied. +// getStatefulSetRevisions returns the current and update ControllerRevisions for set. It also +// returns a collision count that records the number of name collisions set saw when creating +// new ControllerRevisions. This count is incremented on every name collision and is used in +// building the ControllerRevision names for name collision avoidance. This method may create +// a new revision, or modify the Revision of an existing revision if an update to set is detected. +// This method expects that revisions is sorted when supplied. func (ssc *defaultStatefulSetControl) getStatefulSetRevisions( set *apps.StatefulSet, - revisions []*apps.ControllerRevision) (*apps.ControllerRevision, *apps.ControllerRevision, error) { + revisions []*apps.ControllerRevision) (*apps.ControllerRevision, *apps.ControllerRevision, int32, error) { var currentRevision, updateRevision *apps.ControllerRevision revisionCount := len(revisions) history.SortControllerRevisions(revisions) + // Use a local copy of set.Status.CollisionCount to avoid modifying set.Status directly. + // This copy is returned so the value gets carried over to set.Status in updateStatefulSet. + var collisionCount int32 + if set.Status.CollisionCount != nil { + collisionCount = *set.Status.CollisionCount + } + // create a new revision from the current set - updateRevision, err := newRevision(set, nextRevision(revisions)) + updateRevision, err := newRevision(set, nextRevision(revisions), &collisionCount) if err != nil { - return nil, nil, err + return nil, nil, collisionCount, err } // find any equivalent revisions @@ -205,13 +215,13 @@ func (ssc *defaultStatefulSetControl) getStatefulSetRevisions( equalRevisions[equalCount-1], updateRevision.Revision) if err != nil { - return nil, nil, err + return nil, nil, collisionCount, err } } else { //if there is no equivalent revision we create a new one - updateRevision, err = ssc.controllerHistory.CreateControllerRevision(set, updateRevision) + updateRevision, err = ssc.controllerHistory.CreateControllerRevision(set, updateRevision, &collisionCount) if err != nil { - return nil, nil, err + return nil, nil, collisionCount, err } } @@ -227,7 +237,7 @@ func (ssc *defaultStatefulSetControl) getStatefulSetRevisions( currentRevision = updateRevision } - return currentRevision, updateRevision, nil + return currentRevision, updateRevision, collisionCount, nil } // updateStatefulSet performs the update function for a StatefulSet. This method creates, updates, and deletes Pods in @@ -243,6 +253,7 @@ func (ssc *defaultStatefulSetControl) updateStatefulSet( set *apps.StatefulSet, currentRevision *apps.ControllerRevision, updateRevision *apps.ControllerRevision, + collisionCount int32, pods []*v1.Pod) (*apps.StatefulSetStatus, error) { // get the current and update revisions of the set. currentSet, err := applyRevision(set, currentRevision) @@ -260,6 +271,8 @@ func (ssc *defaultStatefulSetControl) updateStatefulSet( *status.ObservedGeneration = set.Generation status.CurrentRevision = currentRevision.Name status.UpdateRevision = updateRevision.Name + status.CollisionCount = new(int32) + *status.CollisionCount = collisionCount replicaCount := int(*set.Spec.Replicas) // slice that will contain all Pods such that 0 <= getOrdinal(pod) < set.Spec.Replicas diff --git a/pkg/controller/statefulset/stateful_set_control_test.go b/pkg/controller/statefulset/stateful_set_control_test.go index 3d0184968bd..50d7b0822b6 100644 --- a/pkg/controller/statefulset/stateful_set_control_test.go +++ b/pkg/controller/statefulset/stateful_set_control_test.go @@ -465,14 +465,15 @@ func TestStatefulSetControl_getSetRevisions(t *testing.T) { informerFactory.Core().V1().Pods().Informer().HasSynced, informerFactory.Apps().V1beta1().ControllerRevisions().Informer().HasSynced, ) + test.set.Status.CollisionCount = new(int32) for i := range test.existing { - ssc.controllerHistory.CreateControllerRevision(test.set, test.existing[i]) + ssc.controllerHistory.CreateControllerRevision(test.set, test.existing[i], test.set.Status.CollisionCount) } revisions, err := ssc.ListRevisions(test.set) if err != nil { t.Fatal(err) } - current, update, err := ssc.getStatefulSetRevisions(test.set, revisions) + current, update, _, err := ssc.getStatefulSetRevisions(test.set, revisions) revisions, err = ssc.ListRevisions(test.set) if err != nil { t.Fatal(err) @@ -480,20 +481,20 @@ func TestStatefulSetControl_getSetRevisions(t *testing.T) { if len(revisions) != test.expectedCount { t.Errorf("%s: want %d revisions got %d", test.name, test.expectedCount, len(revisions)) } - if test.err && err != nil { + if test.err && err == nil { t.Errorf("%s: expected error", test.name) } if !test.err && !history.EqualRevision(current, test.expectedCurrent) { t.Errorf("%s: for current want %v got %v", test.name, test.expectedCurrent, current) } if !test.err && !history.EqualRevision(update, test.expectedUpdate) { - t.Errorf("%s: for current want %v got %v", test.name, test.expectedUpdate, update) + t.Errorf("%s: for update want %v got %v", test.name, test.expectedUpdate, update) } if !test.err && test.expectedCurrent != nil && current != nil && test.expectedCurrent.Revision != current.Revision { t.Errorf("%s: for current revision want %d got %d", test.name, test.expectedCurrent.Revision, current.Revision) } if !test.err && test.expectedUpdate != nil && update != nil && test.expectedUpdate.Revision != update.Revision { - t.Errorf("%s: for current revision want %d got %d", test.name, test.expectedUpdate.Revision, update.Revision) + t.Errorf("%s: for update revision want %d got %d", test.name, test.expectedUpdate.Revision, update.Revision) } } @@ -508,14 +509,17 @@ func TestStatefulSetControl_getSetRevisions(t *testing.T) { } set := newStatefulSet(3) + set.Status.CollisionCount = new(int32) rev0 := newRevisionOrDie(set, 1) set1 := copySet(set) set1.Spec.Template.Spec.Containers[0].Image = "foo" set1.Status.CurrentRevision = rev0.Name + set1.Status.CollisionCount = new(int32) rev1 := newRevisionOrDie(set1, 2) set2 := copySet(set1) set2.Spec.Template.Labels["new"] = "label" set2.Status.CurrentRevision = rev0.Name + set2.Status.CollisionCount = new(int32) rev2 := newRevisionOrDie(set2, 3) tests := []testcase{ { @@ -2097,7 +2101,7 @@ func updateStatefulSetControl(set *apps.StatefulSet, } func newRevisionOrDie(set *apps.StatefulSet, revision int64) *apps.ControllerRevision { - rev, err := newRevision(set, revision) + rev, err := newRevision(set, revision, set.Status.CollisionCount) if err != nil { panic(err) } diff --git a/pkg/controller/statefulset/stateful_set_utils.go b/pkg/controller/statefulset/stateful_set_utils.go index e210cc915d5..ed3570ab4d9 100644 --- a/pkg/controller/statefulset/stateful_set_utils.go +++ b/pkg/controller/statefulset/stateful_set_utils.go @@ -288,7 +288,7 @@ func getPatch(set *apps.StatefulSet) ([]byte, error) { // The Revision of the returned ControllerRevision is set to revision. If the returned error is nil, the returned // ControllerRevision is valid. StatefulSet revisions are stored as patches that re-apply the current state of set // to a new StatefulSet using a strategic merge patch to replace the saved state of the new StatefulSet. -func newRevision(set *apps.StatefulSet, revision int64) (*apps.ControllerRevision, error) { +func newRevision(set *apps.StatefulSet, revision int64, collisionCount *int32) (*apps.ControllerRevision, error) { patch, err := getPatch(set) if err != nil { return nil, err @@ -301,7 +301,8 @@ func newRevision(set *apps.StatefulSet, revision int64) (*apps.ControllerRevisio controllerKind, selector, runtime.RawExtension{Raw: patch}, - revision) + revision, + collisionCount) if err != nil { return nil, err } diff --git a/pkg/controller/statefulset/stateful_set_utils_test.go b/pkg/controller/statefulset/stateful_set_utils_test.go index 6efe8c466f7..f8e6469accb 100644 --- a/pkg/controller/statefulset/stateful_set_utils_test.go +++ b/pkg/controller/statefulset/stateful_set_utils_test.go @@ -274,7 +274,8 @@ func TestNewPodControllerRef(t *testing.T) { func TestCreateApplyRevision(t *testing.T) { set := newStatefulSet(1) - revision, err := newRevision(set, 1) + set.Status.CollisionCount = new(int32) + revision, err := newRevision(set, 1, set.Status.CollisionCount) if err != nil { t.Fatal(err) } @@ -289,7 +290,7 @@ func TestCreateApplyRevision(t *testing.T) { if err != nil { t.Fatal(err) } - restoredRevision, err := newRevision(restoredSet, 2) + restoredRevision, err := newRevision(restoredSet, 2, restoredSet.Status.CollisionCount) if err != nil { t.Fatal(err) }