From b0b1a35aae98b76d841a1bd8f9f843b1ce4679ea Mon Sep 17 00:00:00 2001 From: nikhiljindal Date: Wed, 7 Oct 2015 13:13:18 -0700 Subject: [PATCH] Adding logic to scale down new RC --- .../deployment/deployment_controller.go | 38 +++++++++++++------ .../deployment/deployment_controller_test.go | 18 +++++++-- 2 files changed, 40 insertions(+), 16 deletions(-) diff --git a/pkg/controller/deployment/deployment_controller.go b/pkg/controller/deployment/deployment_controller.go index a13965f4a6c..aae11ac5fe7 100644 --- a/pkg/controller/deployment/deployment_controller.go +++ b/pkg/controller/deployment/deployment_controller.go @@ -102,7 +102,7 @@ func (d *DeploymentController) reconcileRollingUpdateDeployment(deployment exper allRCs := append(oldRCs, newRC) // Scale up, if we can. - scaledUp, err := d.scaleUp(allRCs, newRC, deployment) + scaledUp, err := d.reconcileNewRC(allRCs, newRC, deployment) if err != nil { return err } @@ -112,7 +112,7 @@ func (d *DeploymentController) reconcileRollingUpdateDeployment(deployment exper } // Scale down, if we can. - scaledDown, err := d.scaleDown(allRCs, oldRCs, newRC, deployment) + scaledDown, err := d.reconcileOldRCs(allRCs, oldRCs, newRC, deployment) if err != nil { return err } @@ -158,11 +158,17 @@ func (d *DeploymentController) getNewRC(deployment experimental.Deployment) (*ap return createdRC, nil } -func (d *DeploymentController) scaleUp(allRCs []*api.ReplicationController, newRC *api.ReplicationController, deployment experimental.Deployment) (bool, error) { +func (d *DeploymentController) reconcileNewRC(allRCs []*api.ReplicationController, newRC *api.ReplicationController, deployment experimental.Deployment) (bool, error) { if newRC.Spec.Replicas == deployment.Spec.Replicas { - // Scaling up not required. + // Scaling not required. return false, nil } + if newRC.Spec.Replicas > deployment.Spec.Replicas { + // Scale down. + _, err := d.scaleRCAndRecordEvent(newRC, deployment.Spec.Replicas, deployment) + return true, err + } + // Check if we can scale up. maxSurge, isPercent, err := util.GetIntOrPercentValue(&deployment.Spec.Strategy.RollingUpdate.MaxSurge) if err != nil { return false, fmt.Errorf("invalid value for MaxSurge: %v", err) @@ -172,7 +178,6 @@ func (d *DeploymentController) scaleUp(allRCs []*api.ReplicationController, newR } // Find the total number of pods currentPodCount := deploymentUtil.GetReplicaCountForRCs(allRCs) - // Check if we can scale up. maxTotalPods := deployment.Spec.Replicas + maxSurge if currentPodCount >= maxTotalPods { // Cannot scale up. @@ -180,16 +185,14 @@ func (d *DeploymentController) scaleUp(allRCs []*api.ReplicationController, newR } // Scale up. scaleUpCount := maxTotalPods - currentPodCount + // Do not exceed the number of desired replicas. scaleUpCount = int(math.Min(float64(scaleUpCount), float64(deployment.Spec.Replicas-newRC.Spec.Replicas))) newReplicasCount := newRC.Spec.Replicas + scaleUpCount - _, err = d.scaleRC(newRC, newReplicasCount) - if err == nil { - d.eventRecorder.Eventf(&deployment, "ScalingRC", "Scaled up rc %s to %d", newRC.Name, newReplicasCount) - } + _, err = d.scaleRCAndRecordEvent(newRC, newReplicasCount, deployment) return true, err } -func (d *DeploymentController) scaleDown(allRCs []*api.ReplicationController, oldRCs []*api.ReplicationController, newRC *api.ReplicationController, deployment experimental.Deployment) (bool, error) { +func (d *DeploymentController) reconcileOldRCs(allRCs []*api.ReplicationController, oldRCs []*api.ReplicationController, newRC *api.ReplicationController, deployment experimental.Deployment) (bool, error) { oldPodsCount := deploymentUtil.GetReplicaCountForRCs(oldRCs) if oldPodsCount == 0 { // Cant scale down further @@ -227,11 +230,10 @@ func (d *DeploymentController) scaleDown(allRCs []*api.ReplicationController, ol // Scale down. scaleDownCount := int(math.Min(float64(targetRC.Spec.Replicas), float64(totalScaleDownCount))) newReplicasCount := targetRC.Spec.Replicas - scaleDownCount - _, err = d.scaleRC(targetRC, newReplicasCount) + _, err = d.scaleRCAndRecordEvent(targetRC, newReplicasCount, deployment) if err != nil { return false, err } - d.eventRecorder.Eventf(&deployment, "ScalingRC", "Scaled down rc %s to %d", targetRC.Name, newReplicasCount) totalScaleDownCount -= scaleDownCount } return true, err @@ -250,6 +252,18 @@ func (d *DeploymentController) updateDeploymentStatus(allRCs []*api.ReplicationC return err } +func (d *DeploymentController) scaleRCAndRecordEvent(rc *api.ReplicationController, newScale int, deployment experimental.Deployment) (*api.ReplicationController, error) { + scalingOperation := "down" + if rc.Spec.Replicas < newScale { + scalingOperation = "up" + } + newRC, err := d.scaleRC(rc, newScale) + if err == nil { + d.eventRecorder.Eventf(&deployment, "ScalingRC", "Scaled %s rc %s to %d", scalingOperation, rc.Name, newScale) + } + return newRC, err +} + func (d *DeploymentController) scaleRC(rc *api.ReplicationController, newScale int) (*api.ReplicationController, error) { // TODO: Using client for now, update to use store when it is ready. rc.Spec.Replicas = newScale diff --git a/pkg/controller/deployment/deployment_controller_test.go b/pkg/controller/deployment/deployment_controller_test.go index d1516932ca4..591ac142427 100644 --- a/pkg/controller/deployment/deployment_controller_test.go +++ b/pkg/controller/deployment/deployment_controller_test.go @@ -28,7 +28,7 @@ import ( "k8s.io/kubernetes/pkg/util" ) -func TestDeploymentController_scaleUp(t *testing.T) { +func TestDeploymentController_reconcileNewRC(t *testing.T) { tests := []struct { deploymentReplicas int maxSurge util.IntOrString @@ -38,6 +38,7 @@ func TestDeploymentController_scaleUp(t *testing.T) { expectedNewReplicas int }{ { + // Should not scale up. deploymentReplicas: 10, maxSurge: util.NewIntOrStringFromInt(0), oldReplicas: 10, @@ -67,6 +68,15 @@ func TestDeploymentController_scaleUp(t *testing.T) { newReplicas: 2, scaleExpected: false, }, + { + // Should scale down. + deploymentReplicas: 10, + maxSurge: util.NewIntOrStringFromInt(2), + oldReplicas: 2, + newReplicas: 11, + scaleExpected: true, + expectedNewReplicas: 10, + }, } for i, test := range tests { @@ -80,7 +90,7 @@ func TestDeploymentController_scaleUp(t *testing.T) { client: fake, eventRecorder: &record.FakeRecorder{}, } - scaled, err := controller.scaleUp(allRcs, newRc, deployment) + scaled, err := controller.reconcileNewRC(allRcs, newRc, deployment) if err != nil { t.Errorf("unexpected error: %v", err) continue @@ -106,7 +116,7 @@ func TestDeploymentController_scaleUp(t *testing.T) { } } -func TestDeploymentController_scaleDown(t *testing.T) { +func TestDeploymentController_reconcileOldRCs(t *testing.T) { tests := []struct { deploymentReplicas int maxUnavailable util.IntOrString @@ -180,7 +190,7 @@ func TestDeploymentController_scaleDown(t *testing.T) { client: fake, eventRecorder: &record.FakeRecorder{}, } - scaled, err := controller.scaleDown(allRcs, oldRcs, nil, deployment) + scaled, err := controller.reconcileOldRCs(allRcs, oldRcs, nil, deployment) if err != nil { t.Errorf("unexpected error: %v", err) continue