From 92798408af90569f492be3a1a4d8de02538a6787 Mon Sep 17 00:00:00 2001 From: mqliang Date: Wed, 13 Jan 2016 11:27:26 +0800 Subject: [PATCH] implement reconcileRecreateDeployment --- .../deployment/deployment_controller.go | 60 ++++++++++++- .../deployment/deployment_controller_test.go | 2 +- test/e2e/deployment.go | 87 +++++++++++++++++-- 3 files changed, 138 insertions(+), 11 deletions(-) diff --git a/pkg/controller/deployment/deployment_controller.go b/pkg/controller/deployment/deployment_controller.go index 848af47835..736b794b99 100644 --- a/pkg/controller/deployment/deployment_controller.go +++ b/pkg/controller/deployment/deployment_controller.go @@ -406,7 +406,38 @@ func (dc *DeploymentController) syncDeployment(key string) error { } func (dc *DeploymentController) syncRecreateDeployment(deployment extensions.Deployment) error { - // TODO: implement me. + newRC, err := dc.getNewRC(deployment) + if err != nil { + return err + } + + oldRCs, err := dc.getOldRCs(deployment) + if err != nil { + return err + } + + allRCs := append(oldRCs, newRC) + + // scale down old rcs + scaledDown, err := dc.scaleDownOldRCsForRecreate(oldRCs, deployment) + if err != nil { + return err + } + if scaledDown { + // Update DeploymentStatus + return dc.updateDeploymentStatus(allRCs, newRC, deployment) + } + + // scale up new rc + scaledUp, err := dc.scaleUpNewRCForRecreate(newRC, deployment) + if err != nil { + return err + } + if scaledUp { + // Update DeploymentStatus + return dc.updateDeploymentStatus(allRCs, newRC, deployment) + } + return nil } @@ -599,6 +630,33 @@ func (dc *DeploymentController) reconcileOldRCs(allRCs []*api.ReplicationControl return true, err } +// scaleDownOldRCsForRecreate scales down old rcs when deployment strategy is "Recreate" +func (dc *DeploymentController) scaleDownOldRCsForRecreate(oldRCs []*api.ReplicationController, deployment extensions.Deployment) (bool, error) { + scaled := false + for _, rc := range oldRCs { + // Scaling not required. + if rc.Spec.Replicas == 0 { + continue + } + _, err := dc.scaleRCAndRecordEvent(rc, 0, deployment) + if err != nil { + return false, err + } + scaled = true + } + return scaled, nil +} + +// scaleUpNewRCForRecreate scales up new rc when deployment strategy is "Recreate" +func (dc *DeploymentController) scaleUpNewRCForRecreate(newRC *api.ReplicationController, deployment extensions.Deployment) (bool, error) { + if newRC.Spec.Replicas == deployment.Spec.Replicas { + // Scaling not required. + return false, nil + } + _, err := dc.scaleRCAndRecordEvent(newRC, deployment.Spec.Replicas, deployment) + return true, err +} + func (dc *DeploymentController) updateDeploymentStatus(allRCs []*api.ReplicationController, newRC *api.ReplicationController, deployment extensions.Deployment) error { totalReplicas := deploymentutil.GetReplicaCountForRCs(allRCs) updatedReplicas := deploymentutil.GetReplicaCountForRCs([]*api.ReplicationController{newRC}) diff --git a/pkg/controller/deployment/deployment_controller_test.go b/pkg/controller/deployment/deployment_controller_test.go index 06c91cfb52..98402bf36b 100644 --- a/pkg/controller/deployment/deployment_controller_test.go +++ b/pkg/controller/deployment/deployment_controller_test.go @@ -317,7 +317,7 @@ func newReplicationController(d *exp.Deployment, name string, replicas int) *api Namespace: api.NamespaceDefault, }, Spec: api.ReplicationControllerSpec{ - Replicas: 0, + Replicas: replicas, Template: &d.Spec.Template, }, } diff --git a/test/e2e/deployment.go b/test/e2e/deployment.go index 138bb0bdc6..926ac4e713 100644 --- a/test/e2e/deployment.go +++ b/test/e2e/deployment.go @@ -34,12 +34,15 @@ var _ = Describe("Deployment", func() { It("deployment should create new pods", func() { testNewDeployment(f) }) - It("deployment should delete old pods and create new ones [Flaky]", func() { + It("RollingUpdateDeployment should delete old pods and create new ones [Flaky]", func() { testRollingUpdateDeployment(f) }) - It("deployment should scale up and down in the right order [Flaky]", func() { + It("RollingUpdateDeployment should scale up and down in the right order [Flaky]", func() { testRollingUpdateDeploymentEvents(f) }) + It("RecreateDeployment should delete old pods and create new ones [Flaky]", func() { + testRecreateDeployment(f) + }) It("deployment should support rollover [Flaky]", func() { testRolloverDeployment(f) }) @@ -70,14 +73,17 @@ func newRC(rcName string, replicas int, rcPodLabels map[string]string, imageName } } -func newDeployment(deploymentName string, replicas int, podLabels map[string]string, imageName string, image string) *extensions.Deployment { +func newDeployment(deploymentName string, replicas int, podLabels map[string]string, imageName string, image string, strategyType extensions.DeploymentStrategyType) *extensions.Deployment { return &extensions.Deployment{ ObjectMeta: api.ObjectMeta{ Name: deploymentName, }, Spec: extensions.DeploymentSpec{ - Replicas: replicas, - Selector: podLabels, + Replicas: replicas, + Selector: podLabels, + Strategy: extensions.DeploymentStrategy{ + Type: strategyType, + }, UniqueLabelKey: extensions.DefaultDeploymentUniqueLabelKey, Template: api.PodTemplateSpec{ ObjectMeta: api.ObjectMeta{ @@ -103,7 +109,7 @@ func testNewDeployment(f *Framework) { podLabels := map[string]string{"name": "nginx"} replicas := 1 Logf("Creating simple deployment %s", deploymentName) - _, err := c.Deployments(ns).Create(newDeployment(deploymentName, replicas, podLabels, "nginx", "nginx")) + _, err := c.Deployments(ns).Create(newDeployment(deploymentName, replicas, podLabels, "nginx", "nginx", extensions.RollingUpdateDeploymentStrategyType)) Expect(err).NotTo(HaveOccurred()) defer func() { deployment, err := c.Deployments(ns).Get(deploymentName) @@ -160,7 +166,7 @@ func testRollingUpdateDeployment(f *Framework) { // Create a deployment to delete nginx pods and instead bring up redis pods. deploymentName := "redis-deployment" Logf("Creating deployment %s", deploymentName) - _, err = c.Deployments(ns).Create(newDeployment(deploymentName, replicas, deploymentPodLabels, "redis", "redis")) + _, err = c.Deployments(ns).Create(newDeployment(deploymentName, replicas, deploymentPodLabels, "redis", "redis", extensions.RollingUpdateDeploymentStrategyType)) Expect(err).NotTo(HaveOccurred()) defer func() { deployment, err := c.Deployments(ns).Get(deploymentName) @@ -204,7 +210,7 @@ func testRollingUpdateDeploymentEvents(f *Framework) { // Create a deployment to delete nginx pods and instead bring up redis pods. deploymentName := "redis-deployment-2" Logf("Creating deployment %s", deploymentName) - _, err = c.Deployments(ns).Create(newDeployment(deploymentName, replicas, deploymentPodLabels, "redis", "redis")) + _, err = c.Deployments(ns).Create(newDeployment(deploymentName, replicas, deploymentPodLabels, "redis", "redis", extensions.RollingUpdateDeploymentStrategyType)) Expect(err).NotTo(HaveOccurred()) defer func() { deployment, err := c.Deployments(ns).Get(deploymentName) @@ -237,6 +243,68 @@ func testRollingUpdateDeploymentEvents(f *Framework) { Expect(events.Items[1].Message).Should(Equal(fmt.Sprintf("Scaled down rc %s to 0", rcName))) } +func testRecreateDeployment(f *Framework) { + ns := f.Namespace.Name + c := f.Client + // Create nginx pods. + deploymentPodLabels := map[string]string{"name": "sample-pod-3"} + rcPodLabels := map[string]string{ + "name": "sample-pod-3", + "pod": "nginx", + } + + rcName := "nginx-controller" + replicas := 3 + _, err := c.ReplicationControllers(ns).Create(newRC(rcName, replicas, rcPodLabels, "nginx", "nginx")) + Expect(err).NotTo(HaveOccurred()) + defer func() { + Logf("deleting replication controller %s", rcName) + Expect(c.ReplicationControllers(ns).Delete(rcName)).NotTo(HaveOccurred()) + }() + // Verify that the required pods have come up. + err = verifyPods(c, ns, "sample-pod-3", false, 3) + if err != nil { + Logf("error in waiting for pods to come up: %s", err) + Expect(err).NotTo(HaveOccurred()) + } + + // Create a deployment to delete nginx pods and instead bring up redis pods. + deploymentName := "redis-deployment-3" + Logf("Creating deployment %s", deploymentName) + _, err = c.Deployments(ns).Create(newDeployment(deploymentName, replicas, deploymentPodLabels, "redis", "redis", extensions.RecreateDeploymentStrategyType)) + Expect(err).NotTo(HaveOccurred()) + defer func() { + deployment, err := c.Deployments(ns).Get(deploymentName) + Expect(err).NotTo(HaveOccurred()) + Logf("deleting deployment %s", deploymentName) + Expect(c.Deployments(ns).Delete(deploymentName, nil)).NotTo(HaveOccurred()) + // TODO: remove this once we can delete rcs with deployment + newRC, err := deploymentutil.GetNewRC(*deployment, c) + Expect(err).NotTo(HaveOccurred()) + Expect(c.ReplicationControllers(ns).Delete(newRC.Name)).NotTo(HaveOccurred()) + }() + + err = waitForDeploymentStatus(c, ns, deploymentName, replicas, 0, replicas, 0) + Expect(err).NotTo(HaveOccurred()) + + // Verify that the pods were scaled up and down as expected. We use events to verify that. + deployment, err := c.Deployments(ns).Get(deploymentName) + Expect(err).NotTo(HaveOccurred()) + waitForEvents(c, ns, deployment, 2) + events, err := c.Events(ns).Search(deployment) + if err != nil { + Logf("error in listing events: %s", err) + Expect(err).NotTo(HaveOccurred()) + } + // There should be 2 events, one to scale up the new RC and then to scale down the old RC. + Expect(len(events.Items)).Should(Equal(2)) + newRC, err := deploymentutil.GetNewRC(*deployment, c) + Expect(err).NotTo(HaveOccurred()) + Expect(newRC).NotTo(Equal(nil)) + Expect(events.Items[0].Message).Should(Equal(fmt.Sprintf("Scaled down rc %s to 0", rcName))) + Expect(events.Items[1].Message).Should(Equal(fmt.Sprintf("Scaled up rc %s to 3", newRC.Name))) +} + // testRolloverDeployment tests that deployment supports rollover. // i.e. we can change desired state and kick off rolling update, then change desired state again before it finishes. func testRolloverDeployment(f *Framework) { @@ -269,8 +337,9 @@ func testRolloverDeployment(f *Framework) { deploymentReplicas := 4 deploymentImage := "gcr.io/google_samples/gb-redisslave:v1" deploymentMinReadySeconds := 5 + deploymentStrategyType := extensions.RollingUpdateDeploymentStrategyType Logf("Creating deployment %s", deploymentName) - newDeployment := newDeployment(deploymentName, deploymentReplicas, deploymentPodLabels, deploymentImageName, deploymentImage) + newDeployment := newDeployment(deploymentName, deploymentReplicas, deploymentPodLabels, deploymentImageName, deploymentImage, deploymentStrategyType) newDeployment.Spec.Strategy.RollingUpdate = &extensions.RollingUpdateDeployment{ MaxUnavailable: intstr.FromInt(1), MaxSurge: intstr.FromInt(1),