Merge pull request #32995 from kargakis/scale-before-rolling-out

Automatic merge from submit-queue

controller: scale proportionally before rolling out new templates

Once we have progressDeadlineSeconds (https://github.com/kubernetes/kubernetes/pull/19343) and https://github.com/kubernetes/kubernetes/issues/32863 we should be able to get meaningful errors for the concerns I raised in https://github.com/kubernetes/kubernetes/issues/29357.

cc: @kubernetes/deployment 

Fixes https://github.com/kubernetes/kubernetes/issues/29357
This commit is contained in:
Kubernetes Submit Queue 2016-10-06 10:03:52 -07:00 committed by GitHub
commit 8dd9462dd1
4 changed files with 156 additions and 172 deletions

View File

@ -23,7 +23,7 @@ import (
"k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/testapi" "k8s.io/kubernetes/pkg/api/testapi"
"k8s.io/kubernetes/pkg/api/unversioned" "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/clientset_generated/internalclientset/fake"
"k8s.io/kubernetes/pkg/client/record" "k8s.io/kubernetes/pkg/client/record"
"k8s.io/kubernetes/pkg/client/testing/core" "k8s.io/kubernetes/pkg/client/testing/core"
@ -38,14 +38,14 @@ var (
noTimestamp = unversioned.Time{} noTimestamp = unversioned.Time{}
) )
func rs(name string, replicas int, selector map[string]string, timestamp unversioned.Time) *exp.ReplicaSet { func rs(name string, replicas int, selector map[string]string, timestamp unversioned.Time) *extensions.ReplicaSet {
return &exp.ReplicaSet{ return &extensions.ReplicaSet{
ObjectMeta: api.ObjectMeta{ ObjectMeta: api.ObjectMeta{
Name: name, Name: name,
CreationTimestamp: timestamp, CreationTimestamp: timestamp,
Namespace: api.NamespaceDefault, Namespace: api.NamespaceDefault,
}, },
Spec: exp.ReplicaSetSpec{ Spec: extensions.ReplicaSetSpec{
Replicas: int32(replicas), Replicas: int32(replicas),
Selector: &unversioned.LabelSelector{MatchLabels: selector}, Selector: &unversioned.LabelSelector{MatchLabels: selector},
Template: api.PodTemplateSpec{}, 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 := rs(name, specReplicas, selector, noTimestamp)
rs.Status = exp.ReplicaSetStatus{ rs.Status = extensions.ReplicaSetStatus{
Replicas: int32(statusReplicas), Replicas: int32(statusReplicas),
} }
return rs return rs
} }
func deployment(name string, replicas int, maxSurge, maxUnavailable intstr.IntOrString, selector map[string]string) exp.Deployment { func newDeployment(name string, replicas int, revisionHistoryLimit *int32, maxSurge, maxUnavailable *intstr.IntOrString, selector map[string]string) *extensions.Deployment {
return exp.Deployment{ d := extensions.Deployment{
ObjectMeta: api.ObjectMeta{
Name: name,
Namespace: api.NamespaceDefault,
},
Spec: exp.DeploymentSpec{
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()}, TypeMeta: unversioned.TypeMeta{APIVersion: testapi.Default.GroupVersion().String()},
ObjectMeta: api.ObjectMeta{ ObjectMeta: api.ObjectMeta{
UID: uuid.NewUUID(), UID: uuid.NewUUID(),
Name: "foobar", Name: name,
Namespace: api.NamespaceDefault, Namespace: api.NamespaceDefault,
ResourceVersion: "18",
}, },
Spec: exp.DeploymentSpec{ Spec: extensions.DeploymentSpec{
Strategy: exp.DeploymentStrategy{ Strategy: extensions.DeploymentStrategy{
Type: exp.RollingUpdateDeploymentStrategyType, Type: extensions.RollingUpdateDeploymentStrategyType,
RollingUpdate: &exp.RollingUpdateDeployment{}, RollingUpdate: &extensions.RollingUpdateDeployment{},
}, },
Replicas: int32(replicas), Replicas: int32(replicas),
Selector: &unversioned.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}, Selector: &unversioned.LabelSelector{MatchLabels: selector},
Template: api.PodTemplateSpec{ Template: api.PodTemplateSpec{
ObjectMeta: api.ObjectMeta{ ObjectMeta: api.ObjectMeta{
Labels: map[string]string{ Labels: selector,
"name": "foo",
"type": "production",
},
}, },
Spec: api.PodSpec{ Spec: api.PodSpec{
Containers: []api.Container{ 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 return &d
} }
// TODO: Consolidate all deployment helpers into one. func newReplicaSet(d *extensions.Deployment, name string, replicas int) *extensions.ReplicaSet {
func newDeploymentEnhanced(replicas int, maxSurge intstr.IntOrString) *exp.Deployment { return &extensions.ReplicaSet{
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{
ObjectMeta: api.ObjectMeta{ ObjectMeta: api.ObjectMeta{
Name: name, Name: name,
Namespace: api.NamespaceDefault, Namespace: api.NamespaceDefault,
}, },
Spec: exp.ReplicaSetSpec{ Spec: extensions.ReplicaSetSpec{
Replicas: int32(replicas), Replicas: int32(replicas),
Template: d.Spec.Template, 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 { if key, err := controller.KeyFunc(d); err != nil {
t.Errorf("Unexpected error getting key for deployment %v: %v", d.Name, err) t.Errorf("Unexpected error getting key for deployment %v: %v", d.Name, err)
return "" return ""
@ -157,8 +127,8 @@ type fixture struct {
client *fake.Clientset client *fake.Clientset
// Objects to put in the store. // Objects to put in the store.
dLister []*exp.Deployment dLister []*extensions.Deployment
rsLister []*exp.ReplicaSet rsLister []*extensions.ReplicaSet
podLister []*api.Pod podLister []*api.Pod
// Actions expected to happen on the client. Objects from here are also // Actions expected to happen on the client. Objects from here are also
@ -167,21 +137,21 @@ type fixture struct {
objects []runtime.Object 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)) 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 := core.NewUpdateAction(unversioned.GroupVersionResource{Resource: "deployments"}, d.Namespace, d)
action.Subresource = "status" action.Subresource = "status"
f.actions = append(f.actions, action) 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)) 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)) 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) { func TestSyncDeploymentCreatesReplicaSet(t *testing.T) {
f := newFixture(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.dLister = append(f.dLister, d)
f.objects = append(f.objects, d) f.objects = append(f.objects, d)
@ -255,7 +225,7 @@ func TestSyncDeploymentCreatesReplicaSet(t *testing.T) {
func TestSyncDeploymentDontDoAnythingDuringDeletion(t *testing.T) { func TestSyncDeploymentDontDoAnythingDuringDeletion(t *testing.T) {
f := newFixture(t) f := newFixture(t)
d := newDeployment(1, nil) d := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"})
now := unversioned.Now() now := unversioned.Now()
d.DeletionTimestamp = &now d.DeletionTimestamp = &now
f.dLister = append(f.dLister, d) f.dLister = append(f.dLister, d)
@ -272,7 +242,7 @@ func TestDeploymentController_dontSyncDeploymentsWithEmptyPodSelector(t *testing
controller.rsListerSynced = alwaysReady controller.rsListerSynced = alwaysReady
controller.podListerSynced = alwaysReady controller.podListerSynced = alwaysReady
d := newDeployment(1, nil) d := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"})
empty := unversioned.LabelSelector{} empty := unversioned.LabelSelector{}
d.Spec.Selector = &empty d.Spec.Selector = &empty
controller.dLister.Indexer.Add(d) controller.dLister.Indexer.Add(d)

View File

@ -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) t.Logf("executing scenario %d", i)
newRS := rs("foo-v2", test.newReplicas, nil, noTimestamp) newRS := rs("foo-v2", test.newReplicas, nil, noTimestamp)
oldRS := rs("foo-v2", test.oldReplicas, nil, noTimestamp) oldRS := rs("foo-v2", test.oldReplicas, nil, noTimestamp)
allRSs := []*exp.ReplicaSet{newRS, oldRS} 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{} fake := fake.Clientset{}
controller := &DeploymentController{ controller := &DeploymentController{
client: &fake, client: &fake,
eventRecorder: &record.FakeRecorder{}, eventRecorder: &record.FakeRecorder{},
} }
scaled, err := controller.reconcileNewReplicaSet(allRSs, newRS, &deployment) scaled, err := controller.reconcileNewReplicaSet(allRSs, newRS, deployment)
if err != nil { if err != nil {
t.Errorf("unexpected error: %v", err) t.Errorf("unexpected error: %v", err)
continue continue
@ -178,7 +180,8 @@ func TestDeploymentController_reconcileOldReplicaSets(t *testing.T) {
scaleExpected: false, scaleExpected: false,
}, },
} }
for i, test := range tests { for i := range tests {
test := tests[i]
t.Logf("executing scenario %d", i) t.Logf("executing scenario %d", i)
newSelector := map[string]string{"foo": "new"} newSelector := map[string]string{"foo": "new"}
@ -187,8 +190,8 @@ func TestDeploymentController_reconcileOldReplicaSets(t *testing.T) {
oldRS := rs("foo-old", test.oldReplicas, oldSelector, noTimestamp) oldRS := rs("foo-old", test.oldReplicas, oldSelector, noTimestamp)
oldRSs := []*exp.ReplicaSet{oldRS} oldRSs := []*exp.ReplicaSet{oldRS}
allRSs := []*exp.ReplicaSet{oldRS, newRS} allRSs := []*exp.ReplicaSet{oldRS, newRS}
maxSurge := intstr.FromInt(0)
deployment := deployment("foo", test.deploymentReplicas, intstr.FromInt(0), test.maxUnavailable, newSelector) deployment := newDeployment("foo", test.deploymentReplicas, nil, &maxSurge, &test.maxUnavailable, newSelector)
fakeClientset := fake.Clientset{} fakeClientset := fake.Clientset{}
fakeClientset.AddReactor("list", "pods", func(action core.Action) (handled bool, ret runtime.Object, err error) { fakeClientset.AddReactor("list", "pods", func(action core.Action) (handled bool, ret runtime.Object, err error) {
switch action.(type) { switch action.(type) {
@ -267,7 +270,7 @@ func TestDeploymentController_reconcileOldReplicaSets(t *testing.T) {
eventRecorder: &record.FakeRecorder{}, eventRecorder: &record.FakeRecorder{},
} }
scaled, err := controller.reconcileOldReplicaSets(allRSs, oldRSs, newRS, &deployment) scaled, err := controller.reconcileOldReplicaSets(allRSs, oldRSs, newRS, deployment)
if err != nil { if err != nil {
t.Errorf("unexpected error: %v", err) t.Errorf("unexpected error: %v", err)
continue continue
@ -325,7 +328,9 @@ func TestDeploymentController_cleanupUnhealthyReplicas(t *testing.T) {
t.Logf("executing scenario %d", i) t.Logf("executing scenario %d", i)
oldRS := rs("foo-v2", test.oldReplicas, nil, noTimestamp) oldRS := rs("foo-v2", test.oldReplicas, nil, noTimestamp)
oldRSs := []*exp.ReplicaSet{oldRS} 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 := fake.Clientset{}
fakeClientset.AddReactor("list", "pods", func(action core.Action) (handled bool, ret runtime.Object, err error) { fakeClientset.AddReactor("list", "pods", func(action core.Action) (handled bool, ret runtime.Object, err error) {
switch action.(type) { switch action.(type) {
@ -370,7 +375,7 @@ func TestDeploymentController_cleanupUnhealthyReplicas(t *testing.T) {
client: &fakeClientset, client: &fakeClientset,
eventRecorder: &record.FakeRecorder{}, 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 { if err != nil {
t.Errorf("unexpected error: %v", err) t.Errorf("unexpected error: %v", err)
continue 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) t.Logf("executing scenario %d", i)
oldRS := rs("foo-v2", test.oldReplicas, nil, noTimestamp) oldRS := rs("foo-v2", test.oldReplicas, nil, noTimestamp)
allRSs := []*exp.ReplicaSet{oldRS} allRSs := []*exp.ReplicaSet{oldRS}
oldRSs := []*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 := fake.Clientset{}
fakeClientset.AddReactor("list", "pods", func(action core.Action) (handled bool, ret runtime.Object, err error) { fakeClientset.AddReactor("list", "pods", func(action core.Action) (handled bool, ret runtime.Object, err error) {
switch action.(type) { switch action.(type) {
@ -471,7 +478,7 @@ func TestDeploymentController_scaleDownOldReplicaSetsForRollingUpdate(t *testing
client: &fakeClientset, client: &fakeClientset,
eventRecorder: &record.FakeRecorder{}, eventRecorder: &record.FakeRecorder{},
} }
scaled, err := controller.scaleDownOldReplicaSetsForRollingUpdate(allRSs, oldRSs, &deployment) scaled, err := controller.scaleDownOldReplicaSetsForRollingUpdate(allRSs, oldRSs, deployment)
if !test.errorExpected && err != nil { if !test.errorExpected && err != nil {
t.Errorf("unexpected error: %v", err) t.Errorf("unexpected error: %v", err)
continue continue

View File

@ -534,22 +534,6 @@ func (dc *DeploymentController) isScalingEvent(d *extensions.Deployment) (bool,
if err != nil { if err != nil {
return false, err 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) allRSs := append(oldRSs, newRS)
for _, rs := range controller.FilterActiveReplicaSets(allRSs) { for _, rs := range controller.FilterActiveReplicaSets(allRSs) {
desired, ok := deploymentutil.GetDesiredReplicasAnnotation(rs) desired, ok := deploymentutil.GetDesiredReplicasAnnotation(rs)

View File

@ -21,7 +21,7 @@ import (
"time" "time"
"k8s.io/kubernetes/pkg/api/unversioned" "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/clientset_generated/internalclientset/fake"
"k8s.io/kubernetes/pkg/client/record" "k8s.io/kubernetes/pkg/client/record"
"k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/controller"
@ -29,198 +29,220 @@ import (
"k8s.io/kubernetes/pkg/util/intstr" "k8s.io/kubernetes/pkg/util/intstr"
) )
func maxSurge(val int) *intstr.IntOrString {
surge := intstr.FromInt(val)
return &surge
}
func TestScale(t *testing.T) { func TestScale(t *testing.T) {
newTimestamp := unversioned.Date(2016, 5, 20, 2, 0, 0, 0, time.UTC) newTimestamp := unversioned.Date(2016, 5, 20, 2, 0, 0, 0, time.UTC)
oldTimestamp := unversioned.Date(2016, 5, 20, 1, 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) 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 { tests := []struct {
name string name string
deployment *exp.Deployment deployment *extensions.Deployment
oldDeployment *exp.Deployment oldDeployment *extensions.Deployment
newRS *exp.ReplicaSet newRS *extensions.ReplicaSet
oldRSs []*exp.ReplicaSet oldRSs []*extensions.ReplicaSet
expectedNew *exp.ReplicaSet expectedNew *extensions.ReplicaSet
expectedOld []*exp.ReplicaSet expectedOld []*extensions.ReplicaSet
desiredReplicasAnnotations map[string]int32 desiredReplicasAnnotations map[string]int32
}{ }{
{ {
name: "normal scaling event: 10 -> 12", name: "normal scaling event: 10 -> 12",
deployment: newDeployment(12, nil), deployment: newDeployment("foo", 12, nil, nil, nil, nil),
oldDeployment: newDeployment(10, nil), oldDeployment: newDeployment("foo", 10, nil, nil, nil, nil),
newRS: rs("foo-v1", 10, nil, newTimestamp), newRS: rs("foo-v1", 10, nil, newTimestamp),
oldRSs: []*exp.ReplicaSet{}, oldRSs: []*extensions.ReplicaSet{},
expectedNew: rs("foo-v1", 12, nil, newTimestamp), expectedNew: rs("foo-v1", 12, nil, newTimestamp),
expectedOld: []*exp.ReplicaSet{}, expectedOld: []*extensions.ReplicaSet{},
}, },
{ {
name: "normal scaling event: 10 -> 5", name: "normal scaling event: 10 -> 5",
deployment: newDeployment(5, nil), deployment: newDeployment("foo", 5, nil, nil, nil, nil),
oldDeployment: newDeployment(10, nil), oldDeployment: newDeployment("foo", 10, nil, nil, nil, nil),
newRS: rs("foo-v1", 10, nil, newTimestamp), newRS: rs("foo-v1", 10, nil, newTimestamp),
oldRSs: []*exp.ReplicaSet{}, oldRSs: []*extensions.ReplicaSet{},
expectedNew: rs("foo-v1", 5, nil, newTimestamp), expectedNew: rs("foo-v1", 5, nil, newTimestamp),
expectedOld: []*exp.ReplicaSet{}, expectedOld: []*extensions.ReplicaSet{},
}, },
{ {
name: "proportional scaling: 5 -> 10", name: "proportional scaling: 5 -> 10",
deployment: newDeployment(10, nil), deployment: newDeployment("foo", 10, nil, nil, nil, nil),
oldDeployment: newDeployment(5, nil), oldDeployment: newDeployment("foo", 5, nil, nil, nil, nil),
newRS: rs("foo-v2", 2, nil, newTimestamp), 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), 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", name: "proportional scaling: 5 -> 3",
deployment: newDeployment(3, nil), deployment: newDeployment("foo", 3, nil, nil, nil, nil),
oldDeployment: newDeployment(5, nil), oldDeployment: newDeployment("foo", 5, nil, nil, nil, nil),
newRS: rs("foo-v2", 2, nil, newTimestamp), 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), 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", name: "proportional scaling: 9 -> 4",
deployment: newDeployment(4, nil), deployment: newDeployment("foo", 4, nil, nil, nil, nil),
oldDeployment: newDeployment(9, nil), oldDeployment: newDeployment("foo", 9, nil, nil, nil, nil),
newRS: rs("foo-v2", 8, nil, newTimestamp), 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), 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", name: "proportional scaling: 7 -> 10",
deployment: newDeployment(10, nil), deployment: newDeployment("foo", 10, nil, nil, nil, nil),
oldDeployment: newDeployment(7, nil), oldDeployment: newDeployment("foo", 7, nil, nil, nil, nil),
newRS: rs("foo-v3", 2, nil, newTimestamp), 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), 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", name: "proportional scaling: 13 -> 8",
deployment: newDeployment(8, nil), deployment: newDeployment("foo", 8, nil, nil, nil, nil),
oldDeployment: newDeployment(13, nil), oldDeployment: newDeployment("foo", 13, nil, nil, nil, nil),
newRS: rs("foo-v3", 2, nil, newTimestamp), 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), 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. // Scales up the new replica set.
{ {
name: "leftover distribution: 3 -> 4", name: "leftover distribution: 3 -> 4",
deployment: newDeployment(4, nil), deployment: newDeployment("foo", 4, nil, nil, nil, nil),
oldDeployment: newDeployment(3, nil), oldDeployment: newDeployment("foo", 3, nil, nil, nil, nil),
newRS: rs("foo-v3", 1, nil, newTimestamp), 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), 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. // Scales down the older replica set.
{ {
name: "leftover distribution: 3 -> 2", name: "leftover distribution: 3 -> 2",
deployment: newDeployment(2, nil), deployment: newDeployment("foo", 2, nil, nil, nil, nil),
oldDeployment: newDeployment(3, nil), oldDeployment: newDeployment("foo", 3, nil, nil, nil, nil),
newRS: rs("foo-v3", 1, nil, newTimestamp), 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), 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. // Scales up the latest replica set first.
{ {
name: "proportional scaling (no new rs): 4 -> 5", name: "proportional scaling (no new rs): 4 -> 5",
deployment: newDeployment(5, nil), deployment: newDeployment("foo", 5, nil, nil, nil, nil),
oldDeployment: newDeployment(4, nil), oldDeployment: newDeployment("foo", 4, nil, nil, nil, nil),
newRS: 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, 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 // Scales down to zero
{ {
name: "proportional scaling: 6 -> 0", name: "proportional scaling: 6 -> 0",
deployment: newDeployment(0, nil), deployment: newDeployment("foo", 0, nil, nil, nil, nil),
oldDeployment: newDeployment(6, nil), oldDeployment: newDeployment("foo", 6, nil, nil, nil, nil),
newRS: rs("foo-v3", 3, nil, newTimestamp), 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), 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 // Scales up from zero
{ {
name: "proportional scaling: 0 -> 6", name: "proportional scaling: 0 -> 6",
deployment: newDeployment(6, nil), deployment: newDeployment("foo", 6, nil, nil, nil, nil),
oldDeployment: newDeployment(0, nil), oldDeployment: newDeployment("foo", 6, nil, nil, nil, nil),
newRS: rs("foo-v3", 0, nil, newTimestamp), 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), 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 ) // 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 // Deployment is scaled to 5. foo-v3.spec.replicas and foo-v2.spec.replicas should increment by 1 but foo-v2 fails to
// update. // update.
{ {
name: "failed rs update", name: "failed rs update",
deployment: newDeployment(5, nil), deployment: newDeployment("foo", 5, nil, nil, nil, nil),
oldDeployment: newDeployment(5, nil), oldDeployment: newDeployment("foo", 5, nil, nil, nil, nil),
newRS: rs("foo-v3", 2, nil, newTimestamp), 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), 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)}, desiredReplicasAnnotations: map[string]int32{"foo-v2": int32(3)},
}, },
{ {
name: "deployment with surge pods", name: "deployment with surge pods",
deployment: newDeploymentEnhanced(20, intstr.FromInt(2)), deployment: newDeployment("foo", 20, nil, maxSurge(2), nil, nil),
oldDeployment: newDeploymentEnhanced(10, intstr.FromInt(2)), oldDeployment: newDeployment("foo", 10, nil, maxSurge(2), nil, nil),
newRS: rs("foo-v2", 6, nil, newTimestamp), 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), 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", name: "change both surge and size",
deployment: newDeploymentEnhanced(50, intstr.FromInt(6)), deployment: newDeployment("foo", 50, nil, maxSurge(6), nil, nil),
oldDeployment: newDeploymentEnhanced(10, intstr.FromInt(3)), oldDeployment: newDeployment("foo", 10, nil, maxSurge(3), nil, nil),
newRS: rs("foo-v2", 5, nil, newTimestamp), 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), 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 { for n := range test.oldRSs {
rs := test.oldRSs[n] rs := test.oldRSs[n]
exp := test.expectedOld[n] expected := test.expectedOld[n]
if exp.Spec.Replicas != rs.Spec.Replicas { if expected.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) 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"} selector := map[string]string{"foo": "bar"}
tests := []struct { tests := []struct {
oldRSs []*exp.ReplicaSet oldRSs []*extensions.ReplicaSet
revisionHistoryLimit int revisionHistoryLimit int32
expectedDeletions int expectedDeletions int
}{ }{
{ {
oldRSs: []*exp.ReplicaSet{ oldRSs: []*extensions.ReplicaSet{
newRSWithStatus("foo-1", 0, 0, selector), newRSWithStatus("foo-1", 0, 0, selector),
newRSWithStatus("foo-2", 0, 0, selector), newRSWithStatus("foo-2", 0, 0, selector),
newRSWithStatus("foo-3", 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. // 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-1", 0, 0, selector),
newRSWithStatus("foo-2", 0, 1, selector), newRSWithStatus("foo-2", 0, 1, selector),
newRSWithStatus("foo-3", 1, 0, 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-1", 0, 0, selector),
newRSWithStatus("foo-2", 0, 0, selector), newRSWithStatus("foo-2", 0, 0, selector),
}, },
@ -312,7 +334,7 @@ func TestDeploymentController_cleanupDeployment(t *testing.T) {
expectedDeletions: 2, expectedDeletions: 2,
}, },
{ {
oldRSs: []*exp.ReplicaSet{ oldRSs: []*extensions.ReplicaSet{
newRSWithStatus("foo-1", 1, 1, selector), newRSWithStatus("foo-1", 1, 1, selector),
newRSWithStatus("foo-2", 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{} fake := &fake.Clientset{}
controller := NewDeploymentController(fake, controller.NoResyncPeriodFunc) controller := NewDeploymentController(fake, controller.NoResyncPeriodFunc)
@ -332,7 +355,7 @@ func TestDeploymentController_cleanupDeployment(t *testing.T) {
controller.rsLister.Indexer.Add(rs) 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) controller.cleanupDeployment(test.oldRSs, d)
gotDeletions := 0 gotDeletions := 0