Commit 13889199 authored by Michail Kargakis's avatar Michail Kargakis

controller: resolve unavailable in conjuction with surge for deployments

Due to rounding down for maxUnavailable, we may end up with deployments that have zero surge and unavailable pods something that 1) is not allowed as per validation, 2) blocks deployments. If we end up in such a situation set maxUnavailable to 1 on the theory that surge might not work due to quota.
parent 77097d9a
...@@ -38,7 +38,6 @@ import ( ...@@ -38,7 +38,6 @@ import (
deploymentutil "k8s.io/kubernetes/pkg/util/deployment" deploymentutil "k8s.io/kubernetes/pkg/util/deployment"
utilerrors "k8s.io/kubernetes/pkg/util/errors" utilerrors "k8s.io/kubernetes/pkg/util/errors"
"k8s.io/kubernetes/pkg/util/integer" "k8s.io/kubernetes/pkg/util/integer"
intstrutil "k8s.io/kubernetes/pkg/util/intstr"
labelsutil "k8s.io/kubernetes/pkg/util/labels" labelsutil "k8s.io/kubernetes/pkg/util/labels"
podutil "k8s.io/kubernetes/pkg/util/pod" podutil "k8s.io/kubernetes/pkg/util/pod"
utilruntime "k8s.io/kubernetes/pkg/util/runtime" utilruntime "k8s.io/kubernetes/pkg/util/runtime"
...@@ -843,7 +842,7 @@ func (dc *DeploymentController) reconcileOldReplicaSets(allRSs []*extensions.Rep ...@@ -843,7 +842,7 @@ func (dc *DeploymentController) reconcileOldReplicaSets(allRSs []*extensions.Rep
return false, fmt.Errorf("could not find available pods: %v", err) return false, fmt.Errorf("could not find available pods: %v", err)
} }
maxUnavailable, err := intstrutil.GetValueFromIntOrPercent(&deployment.Spec.Strategy.RollingUpdate.MaxUnavailable, deployment.Spec.Replicas, false) _, maxUnavailable, err := deploymentutil.ResolveFenceposts(&deployment.Spec.Strategy.RollingUpdate.MaxSurge, &deployment.Spec.Strategy.RollingUpdate.MaxUnavailable, deployment.Spec.Replicas)
if err != nil { if err != nil {
return false, err return false, err
} }
...@@ -940,7 +939,7 @@ func (dc *DeploymentController) cleanupUnhealthyReplicas(oldRSs []*extensions.Re ...@@ -940,7 +939,7 @@ func (dc *DeploymentController) cleanupUnhealthyReplicas(oldRSs []*extensions.Re
// scaleDownOldReplicaSetsForRollingUpdate scales down old replica sets when deployment strategy is "RollingUpdate". // scaleDownOldReplicaSetsForRollingUpdate scales down old replica sets when deployment strategy is "RollingUpdate".
// Need check maxUnavailable to ensure availability // Need check maxUnavailable to ensure availability
func (dc *DeploymentController) scaleDownOldReplicaSetsForRollingUpdate(allRSs []*extensions.ReplicaSet, oldRSs []*extensions.ReplicaSet, deployment *extensions.Deployment) (int, error) { func (dc *DeploymentController) scaleDownOldReplicaSetsForRollingUpdate(allRSs []*extensions.ReplicaSet, oldRSs []*extensions.ReplicaSet, deployment *extensions.Deployment) (int, error) {
maxUnavailable, err := intstrutil.GetValueFromIntOrPercent(&deployment.Spec.Strategy.RollingUpdate.MaxUnavailable, deployment.Spec.Replicas, false) _, maxUnavailable, err := deploymentutil.ResolveFenceposts(&deployment.Spec.Strategy.RollingUpdate.MaxSurge, &deployment.Spec.Strategy.RollingUpdate.MaxUnavailable, deployment.Spec.Replicas)
if err != nil { if err != nil {
return 0, err return 0, err
} }
......
...@@ -230,13 +230,14 @@ func TestDeploymentController_reconcileOldReplicaSets(t *testing.T) { ...@@ -230,13 +230,14 @@ func TestDeploymentController_reconcileOldReplicaSets(t *testing.T) {
expectedOldReplicas int expectedOldReplicas int
}{ }{
{ {
deploymentReplicas: 10, deploymentReplicas: 10,
maxUnavailable: intstr.FromInt(0), maxUnavailable: intstr.FromInt(0),
oldReplicas: 10, oldReplicas: 10,
newReplicas: 0, newReplicas: 0,
readyPodsFromOldRS: 10, readyPodsFromOldRS: 10,
readyPodsFromNewRS: 0, readyPodsFromNewRS: 0,
scaleExpected: false, scaleExpected: true,
expectedOldReplicas: 9,
}, },
{ {
deploymentReplicas: 10, deploymentReplicas: 10,
...@@ -492,11 +493,12 @@ func TestDeploymentController_scaleDownOldReplicaSetsForRollingUpdate(t *testing ...@@ -492,11 +493,12 @@ func TestDeploymentController_scaleDownOldReplicaSetsForRollingUpdate(t *testing
expectedOldReplicas int expectedOldReplicas int
}{ }{
{ {
deploymentReplicas: 10, deploymentReplicas: 10,
maxUnavailable: intstr.FromInt(0), maxUnavailable: intstr.FromInt(0),
readyPods: 10, readyPods: 10,
oldReplicas: 10, oldReplicas: 10,
scaleExpected: false, scaleExpected: true,
expectedOldReplicas: 9,
}, },
{ {
deploymentReplicas: 10, deploymentReplicas: 10,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment