Merge pull request #44439 from kargakis/fix-saturation-check

Automatic merge from submit-queue (batch tested with PRs 44741, 44853, 44572, 44797, 44439)

controller: fix saturation check in Deployments

Fixes https://github.com/kubernetes/kubernetes/issues/44436

@kubernetes/sig-apps-bugs 

I'll cherry-pick this back to 1.6 and 1.5
This commit is contained in:
Kubernetes Submit Queue 2017-04-25 01:56:42 -07:00 committed by GitHub
commit d86a01570b
2 changed files with 27 additions and 9 deletions

View File

@ -31,9 +31,9 @@ import (
deploymentutil "k8s.io/kubernetes/pkg/controller/deployment/util"
)
func maxSurge(val int) *intstr.IntOrString {
surge := intstr.FromInt(val)
return &surge
func intOrStrP(val int) *intstr.IntOrString {
intOrStr := intstr.FromInt(val)
return &intOrStr
}
func TestScale(t *testing.T) {
@ -218,8 +218,8 @@ func TestScale(t *testing.T) {
},
{
name: "deployment with surge pods",
deployment: newDeployment("foo", 20, nil, maxSurge(2), nil, nil),
oldDeployment: newDeployment("foo", 10, nil, maxSurge(2), nil, nil),
deployment: newDeployment("foo", 20, nil, intOrStrP(2), nil, nil),
oldDeployment: newDeployment("foo", 10, nil, intOrStrP(2), nil, nil),
newRS: rs("foo-v2", 6, nil, newTimestamp),
oldRSs: []*extensions.ReplicaSet{rs("foo-v1", 6, nil, oldTimestamp)},
@ -229,8 +229,8 @@ func TestScale(t *testing.T) {
},
{
name: "change both surge and size",
deployment: newDeployment("foo", 50, nil, maxSurge(6), nil, nil),
oldDeployment: newDeployment("foo", 10, nil, maxSurge(3), nil, nil),
deployment: newDeployment("foo", 50, nil, intOrStrP(6), nil, nil),
oldDeployment: newDeployment("foo", 10, nil, intOrStrP(3), nil, nil),
newRS: rs("foo-v2", 5, nil, newTimestamp),
oldRSs: []*extensions.ReplicaSet{rs("foo-v1", 8, nil, oldTimestamp)},
@ -249,6 +249,21 @@ func TestScale(t *testing.T) {
expectedNew: nil,
expectedOld: []*extensions.ReplicaSet{rs("foo-v2", 10, nil, newTimestamp), rs("foo-v1", 4, nil, oldTimestamp)},
},
{
name: "saturated but broken new replica set does not affect old pods",
deployment: newDeployment("foo", 2, nil, intOrStrP(1), intOrStrP(1), nil),
oldDeployment: newDeployment("foo", 2, nil, intOrStrP(1), intOrStrP(1), nil),
newRS: func() *extensions.ReplicaSet {
rs := rs("foo-v2", 2, nil, newTimestamp)
rs.Status.AvailableReplicas = 0
return rs
}(),
oldRSs: []*extensions.ReplicaSet{rs("foo-v1", 1, nil, oldTimestamp)},
expectedNew: rs("foo-v2", 2, nil, newTimestamp),
expectedOld: []*extensions.ReplicaSet{rs("foo-v1", 1, nil, oldTimestamp)},
},
}
for _, test := range tests {

View File

@ -920,7 +920,8 @@ func NewRSNewReplicas(deployment *extensions.Deployment, allRSs []*extensions.Re
// IsSaturated checks if the new replica set is saturated by comparing its size with its deployment size.
// Both the deployment and the replica set have to believe this replica set can own all of the desired
// replicas in the deployment and the annotation helps in achieving that.
// replicas in the deployment and the annotation helps in achieving that. All pods of the ReplicaSet
// need to be available.
func IsSaturated(deployment *extensions.Deployment, rs *extensions.ReplicaSet) bool {
if rs == nil {
return false
@ -930,7 +931,9 @@ func IsSaturated(deployment *extensions.Deployment, rs *extensions.ReplicaSet) b
if err != nil {
return false
}
return *(rs.Spec.Replicas) == *(deployment.Spec.Replicas) && int32(desired) == *(deployment.Spec.Replicas)
return *(rs.Spec.Replicas) == *(deployment.Spec.Replicas) &&
int32(desired) == *(deployment.Spec.Replicas) &&
rs.Status.AvailableReplicas == *(deployment.Spec.Replicas)
}
// WaitForObservedDeployment polls for deployment to be updated so that deployment.Status.ObservedGeneration >= desiredGeneration.