diff --git a/pkg/controller/deployment/deployment_controller_test.go b/pkg/controller/deployment/deployment_controller_test.go index c5392149523..1dc23456a3f 100644 --- a/pkg/controller/deployment/deployment_controller_test.go +++ b/pkg/controller/deployment/deployment_controller_test.go @@ -23,7 +23,7 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/testapi" "k8s.io/kubernetes/pkg/api/unversioned" - exp "k8s.io/kubernetes/pkg/apis/extensions" + "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" "k8s.io/kubernetes/pkg/client/record" "k8s.io/kubernetes/pkg/client/testing/core" @@ -38,14 +38,14 @@ var ( noTimestamp = unversioned.Time{} ) -func rs(name string, replicas int, selector map[string]string, timestamp unversioned.Time) *exp.ReplicaSet { - return &exp.ReplicaSet{ +func rs(name string, replicas int, selector map[string]string, timestamp unversioned.Time) *extensions.ReplicaSet { + return &extensions.ReplicaSet{ ObjectMeta: api.ObjectMeta{ Name: name, CreationTimestamp: timestamp, Namespace: api.NamespaceDefault, }, - Spec: exp.ReplicaSetSpec{ + Spec: extensions.ReplicaSetSpec{ Replicas: int32(replicas), Selector: &unversioned.LabelSelector{MatchLabels: selector}, Template: api.PodTemplateSpec{}, @@ -53,61 +53,32 @@ func rs(name string, replicas int, selector map[string]string, timestamp unversi } } -func newRSWithStatus(name string, specReplicas, statusReplicas int, selector map[string]string) *exp.ReplicaSet { +func newRSWithStatus(name string, specReplicas, statusReplicas int, selector map[string]string) *extensions.ReplicaSet { rs := rs(name, specReplicas, selector, noTimestamp) - rs.Status = exp.ReplicaSetStatus{ + rs.Status = extensions.ReplicaSetStatus{ Replicas: int32(statusReplicas), } return rs } -func deployment(name string, replicas int, maxSurge, maxUnavailable intstr.IntOrString, selector map[string]string) exp.Deployment { - return exp.Deployment{ +func newDeployment(name string, replicas int, revisionHistoryLimit *int32, maxSurge, maxUnavailable *intstr.IntOrString, selector map[string]string) *extensions.Deployment { + d := extensions.Deployment{ + TypeMeta: unversioned.TypeMeta{APIVersion: testapi.Default.GroupVersion().String()}, ObjectMeta: api.ObjectMeta{ + UID: uuid.NewUUID(), Name: name, Namespace: api.NamespaceDefault, }, - Spec: exp.DeploymentSpec{ + Spec: extensions.DeploymentSpec{ + Strategy: extensions.DeploymentStrategy{ + Type: extensions.RollingUpdateDeploymentStrategyType, + RollingUpdate: &extensions.RollingUpdateDeployment{}, + }, Replicas: int32(replicas), Selector: &unversioned.LabelSelector{MatchLabels: selector}, - Strategy: exp.DeploymentStrategy{ - Type: exp.RollingUpdateDeploymentStrategyType, - RollingUpdate: &exp.RollingUpdateDeployment{ - MaxSurge: maxSurge, - MaxUnavailable: maxUnavailable, - }, - }, - }, - } -} - -func newDeployment(replicas int, revisionHistoryLimit *int) *exp.Deployment { - var v *int32 - if revisionHistoryLimit != nil { - v = new(int32) - *v = int32(*revisionHistoryLimit) - } - d := exp.Deployment{ - TypeMeta: unversioned.TypeMeta{APIVersion: testapi.Default.GroupVersion().String()}, - ObjectMeta: api.ObjectMeta{ - UID: uuid.NewUUID(), - Name: "foobar", - Namespace: api.NamespaceDefault, - ResourceVersion: "18", - }, - Spec: exp.DeploymentSpec{ - Strategy: exp.DeploymentStrategy{ - Type: exp.RollingUpdateDeploymentStrategyType, - RollingUpdate: &exp.RollingUpdateDeployment{}, - }, - Replicas: int32(replicas), - Selector: &unversioned.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}, Template: api.PodTemplateSpec{ ObjectMeta: api.ObjectMeta{ - Labels: map[string]string{ - "name": "foo", - "type": "production", - }, + Labels: selector, }, Spec: api.PodSpec{ Containers: []api.Container{ @@ -117,33 +88,32 @@ func newDeployment(replicas int, revisionHistoryLimit *int) *exp.Deployment { }, }, }, - RevisionHistoryLimit: v, + RevisionHistoryLimit: revisionHistoryLimit, }, } + if maxSurge != nil { + d.Spec.Strategy.RollingUpdate.MaxSurge = *maxSurge + } + if maxUnavailable != nil { + d.Spec.Strategy.RollingUpdate.MaxUnavailable = *maxUnavailable + } return &d } -// TODO: Consolidate all deployment helpers into one. -func newDeploymentEnhanced(replicas int, maxSurge intstr.IntOrString) *exp.Deployment { - d := newDeployment(replicas, nil) - d.Spec.Strategy.RollingUpdate.MaxSurge = maxSurge - return d -} - -func newReplicaSet(d *exp.Deployment, name string, replicas int) *exp.ReplicaSet { - return &exp.ReplicaSet{ +func newReplicaSet(d *extensions.Deployment, name string, replicas int) *extensions.ReplicaSet { + return &extensions.ReplicaSet{ ObjectMeta: api.ObjectMeta{ Name: name, Namespace: api.NamespaceDefault, }, - Spec: exp.ReplicaSetSpec{ + Spec: extensions.ReplicaSetSpec{ Replicas: int32(replicas), Template: d.Spec.Template, }, } } -func getKey(d *exp.Deployment, t *testing.T) string { +func getKey(d *extensions.Deployment, t *testing.T) string { if key, err := controller.KeyFunc(d); err != nil { t.Errorf("Unexpected error getting key for deployment %v: %v", d.Name, err) return "" @@ -157,8 +127,8 @@ type fixture struct { client *fake.Clientset // Objects to put in the store. - dLister []*exp.Deployment - rsLister []*exp.ReplicaSet + dLister []*extensions.Deployment + rsLister []*extensions.ReplicaSet podLister []*api.Pod // Actions expected to happen on the client. Objects from here are also @@ -167,21 +137,21 @@ type fixture struct { objects []runtime.Object } -func (f *fixture) expectUpdateDeploymentAction(d *exp.Deployment) { +func (f *fixture) expectUpdateDeploymentAction(d *extensions.Deployment) { f.actions = append(f.actions, core.NewUpdateAction(unversioned.GroupVersionResource{Resource: "deployments"}, d.Namespace, d)) } -func (f *fixture) expectUpdateDeploymentStatusAction(d *exp.Deployment) { +func (f *fixture) expectUpdateDeploymentStatusAction(d *extensions.Deployment) { action := core.NewUpdateAction(unversioned.GroupVersionResource{Resource: "deployments"}, d.Namespace, d) action.Subresource = "status" f.actions = append(f.actions, action) } -func (f *fixture) expectCreateRSAction(rs *exp.ReplicaSet) { +func (f *fixture) expectCreateRSAction(rs *extensions.ReplicaSet) { f.actions = append(f.actions, core.NewCreateAction(unversioned.GroupVersionResource{Resource: "replicasets"}, rs.Namespace, rs)) } -func (f *fixture) expectUpdateRSAction(rs *exp.ReplicaSet) { +func (f *fixture) expectUpdateRSAction(rs *extensions.ReplicaSet) { f.actions = append(f.actions, core.NewUpdateAction(unversioned.GroupVersionResource{Resource: "replicasets"}, rs.Namespace, rs)) } @@ -239,7 +209,7 @@ func (f *fixture) run(deploymentName string) { func TestSyncDeploymentCreatesReplicaSet(t *testing.T) { f := newFixture(t) - d := newDeployment(1, nil) + d := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"}) f.dLister = append(f.dLister, d) f.objects = append(f.objects, d) @@ -255,7 +225,7 @@ func TestSyncDeploymentCreatesReplicaSet(t *testing.T) { func TestSyncDeploymentDontDoAnythingDuringDeletion(t *testing.T) { f := newFixture(t) - d := newDeployment(1, nil) + d := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"}) now := unversioned.Now() d.DeletionTimestamp = &now f.dLister = append(f.dLister, d) @@ -272,7 +242,7 @@ func TestDeploymentController_dontSyncDeploymentsWithEmptyPodSelector(t *testing controller.rsListerSynced = alwaysReady controller.podListerSynced = alwaysReady - d := newDeployment(1, nil) + d := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"}) empty := unversioned.LabelSelector{} d.Spec.Selector = &empty controller.dLister.Indexer.Add(d) diff --git a/pkg/controller/deployment/rolling_test.go b/pkg/controller/deployment/rolling_test.go index da929b95de7..3dd5db6b82f 100644 --- a/pkg/controller/deployment/rolling_test.go +++ b/pkg/controller/deployment/rolling_test.go @@ -80,18 +80,20 @@ func TestDeploymentController_reconcileNewReplicaSet(t *testing.T) { }, } - for i, test := range tests { + for i := range tests { + test := tests[i] t.Logf("executing scenario %d", i) newRS := rs("foo-v2", test.newReplicas, nil, noTimestamp) oldRS := rs("foo-v2", test.oldReplicas, nil, noTimestamp) allRSs := []*exp.ReplicaSet{newRS, oldRS} - deployment := deployment("foo", test.deploymentReplicas, test.maxSurge, intstr.FromInt(0), nil) + maxUnavailable := intstr.FromInt(0) + deployment := newDeployment("foo", test.deploymentReplicas, nil, &test.maxSurge, &maxUnavailable, map[string]string{"foo": "bar"}) fake := fake.Clientset{} controller := &DeploymentController{ client: &fake, eventRecorder: &record.FakeRecorder{}, } - scaled, err := controller.reconcileNewReplicaSet(allRSs, newRS, &deployment) + scaled, err := controller.reconcileNewReplicaSet(allRSs, newRS, deployment) if err != nil { t.Errorf("unexpected error: %v", err) continue @@ -178,7 +180,8 @@ func TestDeploymentController_reconcileOldReplicaSets(t *testing.T) { scaleExpected: false, }, } - for i, test := range tests { + for i := range tests { + test := tests[i] t.Logf("executing scenario %d", i) newSelector := map[string]string{"foo": "new"} @@ -187,8 +190,8 @@ func TestDeploymentController_reconcileOldReplicaSets(t *testing.T) { oldRS := rs("foo-old", test.oldReplicas, oldSelector, noTimestamp) oldRSs := []*exp.ReplicaSet{oldRS} allRSs := []*exp.ReplicaSet{oldRS, newRS} - - deployment := deployment("foo", test.deploymentReplicas, intstr.FromInt(0), test.maxUnavailable, newSelector) + maxSurge := intstr.FromInt(0) + deployment := newDeployment("foo", test.deploymentReplicas, nil, &maxSurge, &test.maxUnavailable, newSelector) fakeClientset := fake.Clientset{} fakeClientset.AddReactor("list", "pods", func(action core.Action) (handled bool, ret runtime.Object, err error) { switch action.(type) { @@ -267,7 +270,7 @@ func TestDeploymentController_reconcileOldReplicaSets(t *testing.T) { eventRecorder: &record.FakeRecorder{}, } - scaled, err := controller.reconcileOldReplicaSets(allRSs, oldRSs, newRS, &deployment) + scaled, err := controller.reconcileOldReplicaSets(allRSs, oldRSs, newRS, deployment) if err != nil { t.Errorf("unexpected error: %v", err) continue @@ -325,7 +328,9 @@ func TestDeploymentController_cleanupUnhealthyReplicas(t *testing.T) { t.Logf("executing scenario %d", i) oldRS := rs("foo-v2", test.oldReplicas, nil, noTimestamp) oldRSs := []*exp.ReplicaSet{oldRS} - deployment := deployment("foo", 10, intstr.FromInt(2), intstr.FromInt(2), nil) + maxSurge := intstr.FromInt(2) + maxUnavailable := intstr.FromInt(2) + deployment := newDeployment("foo", 10, nil, &maxSurge, &maxUnavailable, nil) fakeClientset := fake.Clientset{} fakeClientset.AddReactor("list", "pods", func(action core.Action) (handled bool, ret runtime.Object, err error) { switch action.(type) { @@ -370,7 +375,7 @@ func TestDeploymentController_cleanupUnhealthyReplicas(t *testing.T) { client: &fakeClientset, eventRecorder: &record.FakeRecorder{}, } - _, cleanupCount, err := controller.cleanupUnhealthyReplicas(oldRSs, &deployment, 0, int32(test.maxCleanupCount)) + _, cleanupCount, err := controller.cleanupUnhealthyReplicas(oldRSs, deployment, 0, int32(test.maxCleanupCount)) if err != nil { t.Errorf("unexpected error: %v", err) continue @@ -436,12 +441,14 @@ func TestDeploymentController_scaleDownOldReplicaSetsForRollingUpdate(t *testing }, } - for i, test := range tests { + for i := range tests { + test := tests[i] t.Logf("executing scenario %d", i) oldRS := rs("foo-v2", test.oldReplicas, nil, noTimestamp) allRSs := []*exp.ReplicaSet{oldRS} oldRSs := []*exp.ReplicaSet{oldRS} - deployment := deployment("foo", test.deploymentReplicas, intstr.FromInt(0), test.maxUnavailable, map[string]string{"foo": "bar"}) + maxSurge := intstr.FromInt(0) + deployment := newDeployment("foo", test.deploymentReplicas, nil, &maxSurge, &test.maxUnavailable, map[string]string{"foo": "bar"}) fakeClientset := fake.Clientset{} fakeClientset.AddReactor("list", "pods", func(action core.Action) (handled bool, ret runtime.Object, err error) { switch action.(type) { @@ -471,7 +478,7 @@ func TestDeploymentController_scaleDownOldReplicaSetsForRollingUpdate(t *testing client: &fakeClientset, eventRecorder: &record.FakeRecorder{}, } - scaled, err := controller.scaleDownOldReplicaSetsForRollingUpdate(allRSs, oldRSs, &deployment) + scaled, err := controller.scaleDownOldReplicaSetsForRollingUpdate(allRSs, oldRSs, deployment) if !test.errorExpected && err != nil { t.Errorf("unexpected error: %v", err) continue diff --git a/pkg/controller/deployment/sync.go b/pkg/controller/deployment/sync.go index f739462f52b..0b4b6dc410b 100644 --- a/pkg/controller/deployment/sync.go +++ b/pkg/controller/deployment/sync.go @@ -534,22 +534,6 @@ func (dc *DeploymentController) isScalingEvent(d *extensions.Deployment) (bool, if err != nil { return false, err } - // If there is no new replica set matching this deployment and the deployment isn't paused - // then there is a new rollout that waits to happen - if newRS == nil && !d.Spec.Paused { - // Update all active replicas sets to the new deployment size. SetReplicasAnnotations makes - // sure that we will update only replica sets that don't have the current size of the deployment. - maxSurge := deploymentutil.MaxSurge(*d) - for _, rs := range controller.FilterActiveReplicaSets(oldRSs) { - if updated := deploymentutil.SetReplicasAnnotations(rs, d.Spec.Replicas, d.Spec.Replicas+maxSurge); updated { - if _, err := dc.client.Extensions().ReplicaSets(rs.Namespace).Update(rs); err != nil { - glog.Infof("Cannot update annotations for replica set %q: %v", rs.Name, err) - return false, err - } - } - } - return false, nil - } allRSs := append(oldRSs, newRS) for _, rs := range controller.FilterActiveReplicaSets(allRSs) { desired, ok := deploymentutil.GetDesiredReplicasAnnotation(rs) diff --git a/pkg/controller/deployment/sync_test.go b/pkg/controller/deployment/sync_test.go index 1b05bc804b0..a2c7bf11c7b 100644 --- a/pkg/controller/deployment/sync_test.go +++ b/pkg/controller/deployment/sync_test.go @@ -21,7 +21,7 @@ import ( "time" "k8s.io/kubernetes/pkg/api/unversioned" - exp "k8s.io/kubernetes/pkg/apis/extensions" + "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" "k8s.io/kubernetes/pkg/client/record" "k8s.io/kubernetes/pkg/controller" @@ -29,198 +29,220 @@ import ( "k8s.io/kubernetes/pkg/util/intstr" ) +func maxSurge(val int) *intstr.IntOrString { + surge := intstr.FromInt(val) + return &surge +} + func TestScale(t *testing.T) { newTimestamp := unversioned.Date(2016, 5, 20, 2, 0, 0, 0, time.UTC) oldTimestamp := unversioned.Date(2016, 5, 20, 1, 0, 0, 0, time.UTC) olderTimestamp := unversioned.Date(2016, 5, 20, 0, 0, 0, 0, time.UTC) + var updatedTemplate = func(replicas int) *extensions.Deployment { + d := newDeployment("foo", replicas, nil, nil, nil, map[string]string{"foo": "bar"}) + d.Spec.Template.Labels["another"] = "label" + return d + } + tests := []struct { name string - deployment *exp.Deployment - oldDeployment *exp.Deployment + deployment *extensions.Deployment + oldDeployment *extensions.Deployment - newRS *exp.ReplicaSet - oldRSs []*exp.ReplicaSet + newRS *extensions.ReplicaSet + oldRSs []*extensions.ReplicaSet - expectedNew *exp.ReplicaSet - expectedOld []*exp.ReplicaSet + expectedNew *extensions.ReplicaSet + expectedOld []*extensions.ReplicaSet desiredReplicasAnnotations map[string]int32 }{ { name: "normal scaling event: 10 -> 12", - deployment: newDeployment(12, nil), - oldDeployment: newDeployment(10, nil), + deployment: newDeployment("foo", 12, nil, nil, nil, nil), + oldDeployment: newDeployment("foo", 10, nil, nil, nil, nil), newRS: rs("foo-v1", 10, nil, newTimestamp), - oldRSs: []*exp.ReplicaSet{}, + oldRSs: []*extensions.ReplicaSet{}, expectedNew: rs("foo-v1", 12, nil, newTimestamp), - expectedOld: []*exp.ReplicaSet{}, + expectedOld: []*extensions.ReplicaSet{}, }, { name: "normal scaling event: 10 -> 5", - deployment: newDeployment(5, nil), - oldDeployment: newDeployment(10, nil), + deployment: newDeployment("foo", 5, nil, nil, nil, nil), + oldDeployment: newDeployment("foo", 10, nil, nil, nil, nil), newRS: rs("foo-v1", 10, nil, newTimestamp), - oldRSs: []*exp.ReplicaSet{}, + oldRSs: []*extensions.ReplicaSet{}, expectedNew: rs("foo-v1", 5, nil, newTimestamp), - expectedOld: []*exp.ReplicaSet{}, + expectedOld: []*extensions.ReplicaSet{}, }, { name: "proportional scaling: 5 -> 10", - deployment: newDeployment(10, nil), - oldDeployment: newDeployment(5, nil), + deployment: newDeployment("foo", 10, nil, nil, nil, nil), + oldDeployment: newDeployment("foo", 5, nil, nil, nil, nil), newRS: rs("foo-v2", 2, nil, newTimestamp), - oldRSs: []*exp.ReplicaSet{rs("foo-v1", 3, nil, oldTimestamp)}, + oldRSs: []*extensions.ReplicaSet{rs("foo-v1", 3, nil, oldTimestamp)}, expectedNew: rs("foo-v2", 4, nil, newTimestamp), - expectedOld: []*exp.ReplicaSet{rs("foo-v1", 6, nil, oldTimestamp)}, + expectedOld: []*extensions.ReplicaSet{rs("foo-v1", 6, nil, oldTimestamp)}, }, { name: "proportional scaling: 5 -> 3", - deployment: newDeployment(3, nil), - oldDeployment: newDeployment(5, nil), + deployment: newDeployment("foo", 3, nil, nil, nil, nil), + oldDeployment: newDeployment("foo", 5, nil, nil, nil, nil), newRS: rs("foo-v2", 2, nil, newTimestamp), - oldRSs: []*exp.ReplicaSet{rs("foo-v1", 3, nil, oldTimestamp)}, + oldRSs: []*extensions.ReplicaSet{rs("foo-v1", 3, nil, oldTimestamp)}, expectedNew: rs("foo-v2", 1, nil, newTimestamp), - expectedOld: []*exp.ReplicaSet{rs("foo-v1", 2, nil, oldTimestamp)}, + expectedOld: []*extensions.ReplicaSet{rs("foo-v1", 2, nil, oldTimestamp)}, }, { name: "proportional scaling: 9 -> 4", - deployment: newDeployment(4, nil), - oldDeployment: newDeployment(9, nil), + deployment: newDeployment("foo", 4, nil, nil, nil, nil), + oldDeployment: newDeployment("foo", 9, nil, nil, nil, nil), newRS: rs("foo-v2", 8, nil, newTimestamp), - oldRSs: []*exp.ReplicaSet{rs("foo-v1", 1, nil, oldTimestamp)}, + oldRSs: []*extensions.ReplicaSet{rs("foo-v1", 1, nil, oldTimestamp)}, expectedNew: rs("foo-v2", 4, nil, newTimestamp), - expectedOld: []*exp.ReplicaSet{rs("foo-v1", 0, nil, oldTimestamp)}, + expectedOld: []*extensions.ReplicaSet{rs("foo-v1", 0, nil, oldTimestamp)}, }, { name: "proportional scaling: 7 -> 10", - deployment: newDeployment(10, nil), - oldDeployment: newDeployment(7, nil), + deployment: newDeployment("foo", 10, nil, nil, nil, nil), + oldDeployment: newDeployment("foo", 7, nil, nil, nil, nil), newRS: rs("foo-v3", 2, nil, newTimestamp), - oldRSs: []*exp.ReplicaSet{rs("foo-v2", 3, nil, oldTimestamp), rs("foo-v1", 2, nil, olderTimestamp)}, + oldRSs: []*extensions.ReplicaSet{rs("foo-v2", 3, nil, oldTimestamp), rs("foo-v1", 2, nil, olderTimestamp)}, expectedNew: rs("foo-v3", 3, nil, newTimestamp), - expectedOld: []*exp.ReplicaSet{rs("foo-v2", 4, nil, oldTimestamp), rs("foo-v1", 3, nil, olderTimestamp)}, + expectedOld: []*extensions.ReplicaSet{rs("foo-v2", 4, nil, oldTimestamp), rs("foo-v1", 3, nil, olderTimestamp)}, }, { name: "proportional scaling: 13 -> 8", - deployment: newDeployment(8, nil), - oldDeployment: newDeployment(13, nil), + deployment: newDeployment("foo", 8, nil, nil, nil, nil), + oldDeployment: newDeployment("foo", 13, nil, nil, nil, nil), newRS: rs("foo-v3", 2, nil, newTimestamp), - oldRSs: []*exp.ReplicaSet{rs("foo-v2", 8, nil, oldTimestamp), rs("foo-v1", 3, nil, olderTimestamp)}, + oldRSs: []*extensions.ReplicaSet{rs("foo-v2", 8, nil, oldTimestamp), rs("foo-v1", 3, nil, olderTimestamp)}, expectedNew: rs("foo-v3", 1, nil, newTimestamp), - expectedOld: []*exp.ReplicaSet{rs("foo-v2", 5, nil, oldTimestamp), rs("foo-v1", 2, nil, olderTimestamp)}, + expectedOld: []*extensions.ReplicaSet{rs("foo-v2", 5, nil, oldTimestamp), rs("foo-v1", 2, nil, olderTimestamp)}, }, // Scales up the new replica set. { name: "leftover distribution: 3 -> 4", - deployment: newDeployment(4, nil), - oldDeployment: newDeployment(3, nil), + deployment: newDeployment("foo", 4, nil, nil, nil, nil), + oldDeployment: newDeployment("foo", 3, nil, nil, nil, nil), newRS: rs("foo-v3", 1, nil, newTimestamp), - oldRSs: []*exp.ReplicaSet{rs("foo-v2", 1, nil, oldTimestamp), rs("foo-v1", 1, nil, olderTimestamp)}, + oldRSs: []*extensions.ReplicaSet{rs("foo-v2", 1, nil, oldTimestamp), rs("foo-v1", 1, nil, olderTimestamp)}, expectedNew: rs("foo-v3", 2, nil, newTimestamp), - expectedOld: []*exp.ReplicaSet{rs("foo-v2", 1, nil, oldTimestamp), rs("foo-v1", 1, nil, olderTimestamp)}, + expectedOld: []*extensions.ReplicaSet{rs("foo-v2", 1, nil, oldTimestamp), rs("foo-v1", 1, nil, olderTimestamp)}, }, // Scales down the older replica set. { name: "leftover distribution: 3 -> 2", - deployment: newDeployment(2, nil), - oldDeployment: newDeployment(3, nil), + deployment: newDeployment("foo", 2, nil, nil, nil, nil), + oldDeployment: newDeployment("foo", 3, nil, nil, nil, nil), newRS: rs("foo-v3", 1, nil, newTimestamp), - oldRSs: []*exp.ReplicaSet{rs("foo-v2", 1, nil, oldTimestamp), rs("foo-v1", 1, nil, olderTimestamp)}, + oldRSs: []*extensions.ReplicaSet{rs("foo-v2", 1, nil, oldTimestamp), rs("foo-v1", 1, nil, olderTimestamp)}, expectedNew: rs("foo-v3", 1, nil, newTimestamp), - expectedOld: []*exp.ReplicaSet{rs("foo-v2", 1, nil, oldTimestamp), rs("foo-v1", 0, nil, olderTimestamp)}, + expectedOld: []*extensions.ReplicaSet{rs("foo-v2", 1, nil, oldTimestamp), rs("foo-v1", 0, nil, olderTimestamp)}, }, // Scales up the latest replica set first. { name: "proportional scaling (no new rs): 4 -> 5", - deployment: newDeployment(5, nil), - oldDeployment: newDeployment(4, nil), + deployment: newDeployment("foo", 5, nil, nil, nil, nil), + oldDeployment: newDeployment("foo", 4, nil, nil, nil, nil), newRS: nil, - oldRSs: []*exp.ReplicaSet{rs("foo-v2", 2, nil, oldTimestamp), rs("foo-v1", 2, nil, olderTimestamp)}, + oldRSs: []*extensions.ReplicaSet{rs("foo-v2", 2, nil, oldTimestamp), rs("foo-v1", 2, nil, olderTimestamp)}, expectedNew: nil, - expectedOld: []*exp.ReplicaSet{rs("foo-v2", 3, nil, oldTimestamp), rs("foo-v1", 2, nil, olderTimestamp)}, + expectedOld: []*extensions.ReplicaSet{rs("foo-v2", 3, nil, oldTimestamp), rs("foo-v1", 2, nil, olderTimestamp)}, }, // Scales down to zero { name: "proportional scaling: 6 -> 0", - deployment: newDeployment(0, nil), - oldDeployment: newDeployment(6, nil), + deployment: newDeployment("foo", 0, nil, nil, nil, nil), + oldDeployment: newDeployment("foo", 6, nil, nil, nil, nil), newRS: rs("foo-v3", 3, nil, newTimestamp), - oldRSs: []*exp.ReplicaSet{rs("foo-v2", 2, nil, oldTimestamp), rs("foo-v1", 1, nil, olderTimestamp)}, + oldRSs: []*extensions.ReplicaSet{rs("foo-v2", 2, nil, oldTimestamp), rs("foo-v1", 1, nil, olderTimestamp)}, expectedNew: rs("foo-v3", 0, nil, newTimestamp), - expectedOld: []*exp.ReplicaSet{rs("foo-v2", 0, nil, oldTimestamp), rs("foo-v1", 0, nil, olderTimestamp)}, + expectedOld: []*extensions.ReplicaSet{rs("foo-v2", 0, nil, oldTimestamp), rs("foo-v1", 0, nil, olderTimestamp)}, }, // Scales up from zero { name: "proportional scaling: 0 -> 6", - deployment: newDeployment(6, nil), - oldDeployment: newDeployment(0, nil), + deployment: newDeployment("foo", 6, nil, nil, nil, nil), + oldDeployment: newDeployment("foo", 6, nil, nil, nil, nil), newRS: rs("foo-v3", 0, nil, newTimestamp), - oldRSs: []*exp.ReplicaSet{rs("foo-v2", 0, nil, oldTimestamp), rs("foo-v1", 0, nil, olderTimestamp)}, + oldRSs: []*extensions.ReplicaSet{rs("foo-v2", 0, nil, oldTimestamp), rs("foo-v1", 0, nil, olderTimestamp)}, expectedNew: rs("foo-v3", 6, nil, newTimestamp), - expectedOld: []*exp.ReplicaSet{rs("foo-v2", 0, nil, oldTimestamp), rs("foo-v1", 0, nil, olderTimestamp)}, + expectedOld: []*extensions.ReplicaSet{rs("foo-v2", 0, nil, oldTimestamp), rs("foo-v1", 0, nil, olderTimestamp)}, }, // Scenario: deployment.spec.replicas == 3 ( foo-v1.spec.replicas == foo-v2.spec.replicas == foo-v3.spec.replicas == 1 ) // Deployment is scaled to 5. foo-v3.spec.replicas and foo-v2.spec.replicas should increment by 1 but foo-v2 fails to // update. { name: "failed rs update", - deployment: newDeployment(5, nil), - oldDeployment: newDeployment(5, nil), + deployment: newDeployment("foo", 5, nil, nil, nil, nil), + oldDeployment: newDeployment("foo", 5, nil, nil, nil, nil), newRS: rs("foo-v3", 2, nil, newTimestamp), - oldRSs: []*exp.ReplicaSet{rs("foo-v2", 1, nil, oldTimestamp), rs("foo-v1", 1, nil, olderTimestamp)}, + oldRSs: []*extensions.ReplicaSet{rs("foo-v2", 1, nil, oldTimestamp), rs("foo-v1", 1, nil, olderTimestamp)}, expectedNew: rs("foo-v3", 2, nil, newTimestamp), - expectedOld: []*exp.ReplicaSet{rs("foo-v2", 2, nil, oldTimestamp), rs("foo-v1", 1, nil, olderTimestamp)}, + expectedOld: []*extensions.ReplicaSet{rs("foo-v2", 2, nil, oldTimestamp), rs("foo-v1", 1, nil, olderTimestamp)}, desiredReplicasAnnotations: map[string]int32{"foo-v2": int32(3)}, }, { name: "deployment with surge pods", - deployment: newDeploymentEnhanced(20, intstr.FromInt(2)), - oldDeployment: newDeploymentEnhanced(10, intstr.FromInt(2)), + deployment: newDeployment("foo", 20, nil, maxSurge(2), nil, nil), + oldDeployment: newDeployment("foo", 10, nil, maxSurge(2), nil, nil), newRS: rs("foo-v2", 6, nil, newTimestamp), - oldRSs: []*exp.ReplicaSet{rs("foo-v1", 6, nil, oldTimestamp)}, + oldRSs: []*extensions.ReplicaSet{rs("foo-v1", 6, nil, oldTimestamp)}, expectedNew: rs("foo-v2", 11, nil, newTimestamp), - expectedOld: []*exp.ReplicaSet{rs("foo-v1", 11, nil, oldTimestamp)}, + expectedOld: []*extensions.ReplicaSet{rs("foo-v1", 11, nil, oldTimestamp)}, }, { name: "change both surge and size", - deployment: newDeploymentEnhanced(50, intstr.FromInt(6)), - oldDeployment: newDeploymentEnhanced(10, intstr.FromInt(3)), + deployment: newDeployment("foo", 50, nil, maxSurge(6), nil, nil), + oldDeployment: newDeployment("foo", 10, nil, maxSurge(3), nil, nil), newRS: rs("foo-v2", 5, nil, newTimestamp), - oldRSs: []*exp.ReplicaSet{rs("foo-v1", 8, nil, oldTimestamp)}, + oldRSs: []*extensions.ReplicaSet{rs("foo-v1", 8, nil, oldTimestamp)}, expectedNew: rs("foo-v2", 22, nil, newTimestamp), - expectedOld: []*exp.ReplicaSet{rs("foo-v1", 34, nil, oldTimestamp)}, + expectedOld: []*extensions.ReplicaSet{rs("foo-v1", 34, nil, oldTimestamp)}, + }, + { + name: "change both size and template", + deployment: updatedTemplate(14), + oldDeployment: newDeployment("foo", 10, nil, nil, nil, map[string]string{"foo": "bar"}), + + newRS: nil, + oldRSs: []*extensions.ReplicaSet{rs("foo-v2", 7, nil, newTimestamp), rs("foo-v1", 3, nil, oldTimestamp)}, + + expectedNew: nil, + expectedOld: []*extensions.ReplicaSet{rs("foo-v2", 10, nil, newTimestamp), rs("foo-v1", 4, nil, oldTimestamp)}, }, } @@ -266,9 +288,9 @@ func TestScale(t *testing.T) { } for n := range test.oldRSs { rs := test.oldRSs[n] - exp := test.expectedOld[n] - if exp.Spec.Replicas != rs.Spec.Replicas { - t.Errorf("%s: expected old (%s) replicas: %d, got: %d", test.name, rs.Name, exp.Spec.Replicas, rs.Spec.Replicas) + expected := test.expectedOld[n] + if expected.Spec.Replicas != rs.Spec.Replicas { + t.Errorf("%s: expected old (%s) replicas: %d, got: %d", test.name, rs.Name, expected.Spec.Replicas, rs.Spec.Replicas) } } } @@ -278,12 +300,12 @@ func TestDeploymentController_cleanupDeployment(t *testing.T) { selector := map[string]string{"foo": "bar"} tests := []struct { - oldRSs []*exp.ReplicaSet - revisionHistoryLimit int + oldRSs []*extensions.ReplicaSet + revisionHistoryLimit int32 expectedDeletions int }{ { - oldRSs: []*exp.ReplicaSet{ + oldRSs: []*extensions.ReplicaSet{ newRSWithStatus("foo-1", 0, 0, selector), newRSWithStatus("foo-2", 0, 0, selector), newRSWithStatus("foo-3", 0, 0, selector), @@ -293,7 +315,7 @@ func TestDeploymentController_cleanupDeployment(t *testing.T) { }, { // Only delete the replica set with Spec.Replicas = Status.Replicas = 0. - oldRSs: []*exp.ReplicaSet{ + oldRSs: []*extensions.ReplicaSet{ newRSWithStatus("foo-1", 0, 0, selector), newRSWithStatus("foo-2", 0, 1, selector), newRSWithStatus("foo-3", 1, 0, selector), @@ -304,7 +326,7 @@ func TestDeploymentController_cleanupDeployment(t *testing.T) { }, { - oldRSs: []*exp.ReplicaSet{ + oldRSs: []*extensions.ReplicaSet{ newRSWithStatus("foo-1", 0, 0, selector), newRSWithStatus("foo-2", 0, 0, selector), }, @@ -312,7 +334,7 @@ func TestDeploymentController_cleanupDeployment(t *testing.T) { expectedDeletions: 2, }, { - oldRSs: []*exp.ReplicaSet{ + oldRSs: []*extensions.ReplicaSet{ newRSWithStatus("foo-1", 1, 1, selector), newRSWithStatus("foo-2", 1, 1, selector), }, @@ -321,7 +343,8 @@ func TestDeploymentController_cleanupDeployment(t *testing.T) { }, } - for i, test := range tests { + for i := range tests { + test := tests[i] fake := &fake.Clientset{} controller := NewDeploymentController(fake, controller.NoResyncPeriodFunc) @@ -332,7 +355,7 @@ func TestDeploymentController_cleanupDeployment(t *testing.T) { controller.rsLister.Indexer.Add(rs) } - d := newDeployment(1, &tests[i].revisionHistoryLimit) + d := newDeployment("foo", 1, &test.revisionHistoryLimit, nil, nil, map[string]string{"foo": "bar"}) controller.cleanupDeployment(test.oldRSs, d) gotDeletions := 0