From 093b7c2e25da84dfa9cbbdd9cddc9e5a22b6d481 Mon Sep 17 00:00:00 2001 From: Dan Mace Date: Fri, 17 Apr 2015 14:58:43 -0400 Subject: [PATCH] Add cleanup policy to RollingUpdater * Support configurable cleanup policies in RollingUpdater. Downstream library consumers don't necessarily have the same rules for post deployment cleanup; making the behavior policy driven is more flexible. * Refactor RollingUpdater to accept a config object during Update instead of a long argument list. * Add test coverage for cleanup policy. --- pkg/kubectl/cmd/rollingupdate.go | 10 ++- pkg/kubectl/rolling_updater.go | 77 +++++++++++++++----- pkg/kubectl/rolling_updater_test.go | 106 +++++++++++++++++++++++++++- 3 files changed, 174 insertions(+), 19 deletions(-) diff --git a/pkg/kubectl/cmd/rollingupdate.go b/pkg/kubectl/cmd/rollingupdate.go index 0b0293e506a..250339bf18f 100644 --- a/pkg/kubectl/cmd/rollingupdate.go +++ b/pkg/kubectl/cmd/rollingupdate.go @@ -134,7 +134,15 @@ func RunRollingUpdate(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, arg if newRc.Spec.Replicas == 0 { newRc.Spec.Replicas = oldRc.Spec.Replicas } - err = updater.Update(out, oldRc, newRc, period, interval, timeout) + err = updater.Update(&kubectl.RollingUpdaterConfig{ + Out: out, + OldRc: oldRc, + NewRc: newRc, + UpdatePeriod: period, + Interval: interval, + Timeout: timeout, + CleanupPolicy: kubectl.DeleteRollingUpdateCleanupPolicy, + }) if err != nil { return err } diff --git a/pkg/kubectl/rolling_updater.go b/pkg/kubectl/rolling_updater.go index 7172b8663b6..26afd6538d7 100644 --- a/pkg/kubectl/rolling_updater.go +++ b/pkg/kubectl/rolling_updater.go @@ -36,6 +36,38 @@ type RollingUpdater struct { ns string } +// RollingUpdaterConfig is the configuration for a rolling deployment process. +type RollingUpdaterConfig struct { + // Out is a writer for progress output. + Out io.Writer + // OldRC is an existing controller to be replaced. + OldRc *api.ReplicationController + // NewRc is a controller that will take ownership of updated pods (will be + // created if needed). + NewRc *api.ReplicationController + // UpdatePeriod is the time to wait between individual pod updates. + UpdatePeriod time.Duration + // Interval is the time to wait between polling controller status after + // update. + Interval time.Duration + // Timeout is the time to wait for controller updates before giving up. + Timeout time.Duration + // CleanupPolicy defines the cleanup action to take after the deployment is + // complete. + CleanupPolicy RollingUpdaterCleanupPolicy +} + +// RollingUpdaterCleanupPolicy is a cleanup action to take after the +// deployment is complete. +type RollingUpdaterCleanupPolicy string + +const ( + // DeleteRollingUpdateCleanupPolicy means delete the old controller. + DeleteRollingUpdateCleanupPolicy RollingUpdaterCleanupPolicy = "Delete" + // PreserveRollingUpdateCleanupPolicy means keep the old controller. + PreserveRollingUpdateCleanupPolicy RollingUpdaterCleanupPolicy = "Preserve" +) + // NewRollingUpdater creates a RollingUpdater from a client func NewRollingUpdater(namespace string, c RollingUpdaterClient) *RollingUpdater { return &RollingUpdater{ @@ -49,20 +81,25 @@ const ( desiredReplicasAnnotation = kubectlAnnotationPrefix + "desired-replicas" ) -// Update all pods for a ReplicationController (oldRc) by creating a new controller (newRc) -// with 0 replicas, and synchronously resizing oldRc,newRc by 1 until oldRc has 0 replicas -// and newRc has the original # of desired replicas. oldRc is then deleted. -// If an update from newRc to oldRc is already in progress, we attempt to drive it to completion. -// If an error occurs at any step of the update, the error will be returned. -// 'out' writer for progress output -// 'oldRc' existing controller to be replaced -// 'newRc' controller that will take ownership of updated pods (will be created if needed) -// 'updatePeriod' time to wait between individual pod updates -// 'interval' time to wait between polling controller status after update -// 'timeout' time to wait for controller updates before giving up +// Update all pods for a ReplicationController (oldRc) by creating a new +// controller (newRc) with 0 replicas, and synchronously resizing oldRc,newRc +// by 1 until oldRc has 0 replicas and newRc has the original # of desired +// replicas. Cleanup occurs based on a RollingUpdaterCleanupPolicy. // -// TODO: make this handle performing a rollback of a partially completed rollout. -func (r *RollingUpdater) Update(out io.Writer, oldRc, newRc *api.ReplicationController, updatePeriod, interval, timeout time.Duration) error { +// If an update from newRc to oldRc is already in progress, we attempt to +// drive it to completion. If an error occurs at any step of the update, the +// error will be returned. +// +// TODO: make this handle performing a rollback of a partially completed +// rollout. +func (r *RollingUpdater) Update(config *RollingUpdaterConfig) error { + out := config.Out + oldRc := config.OldRc + newRc := config.NewRc + updatePeriod := config.UpdatePeriod + interval := config.Interval + timeout := config.Timeout + oldName := oldRc.ObjectMeta.Name newName := newRc.ObjectMeta.Name retry := &RetryParams{interval, timeout} @@ -156,9 +193,17 @@ func (r *RollingUpdater) Update(out io.Writer, oldRc, newRc *api.ReplicationCont if err != nil { return err } - // delete old rc - fmt.Fprintf(out, "Update succeeded. Deleting %s\n", oldName) - return r.c.DeleteReplicationController(r.ns, oldName) + + switch config.CleanupPolicy { + case DeleteRollingUpdateCleanupPolicy: + // delete old rc + fmt.Fprintf(out, "Update succeeded. Deleting %s\n", oldName) + return r.c.DeleteReplicationController(r.ns, oldName) + case PreserveRollingUpdateCleanupPolicy: + return nil + default: + return nil + } } func (r *RollingUpdater) getExistingNewRc(sourceId, name string) (rc *api.ReplicationController, existing bool, err error) { diff --git a/pkg/kubectl/rolling_updater_test.go b/pkg/kubectl/rolling_updater_test.go index 144d7059c90..51bb2ea621a 100644 --- a/pkg/kubectl/rolling_updater_test.go +++ b/pkg/kubectl/rolling_updater_test.go @@ -19,12 +19,14 @@ package kubectl import ( "bytes" "fmt" + "io/ioutil" "testing" "time" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/client" "github.com/GoogleCloudPlatform/kubernetes/pkg/client/testclient" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util/wait" ) type updaterFake struct { @@ -257,7 +259,16 @@ Update succeeded. Deleting foo-v1 "default", } var buffer bytes.Buffer - if err := updater.Update(&buffer, test.oldRc, test.newRc, 0, time.Millisecond, time.Millisecond); err != nil { + config := &RollingUpdaterConfig{ + Out: &buffer, + OldRc: test.oldRc, + NewRc: test.newRc, + UpdatePeriod: 0, + Interval: time.Millisecond, + Timeout: time.Millisecond, + CleanupPolicy: DeleteRollingUpdateCleanupPolicy, + } + if err := updater.Update(config); err != nil { t.Errorf("Update failed: %v", err) } if buffer.String() != test.output { @@ -299,10 +310,101 @@ Update succeeded. Deleting foo-v1 updater := RollingUpdater{NewRollingUpdaterClient(fakeClientFor("default", responses)), "default"} var buffer bytes.Buffer - if err := updater.Update(&buffer, rc, rcExisting, 0, time.Millisecond, time.Millisecond); err != nil { + config := &RollingUpdaterConfig{ + Out: &buffer, + OldRc: rc, + NewRc: rcExisting, + UpdatePeriod: 0, + Interval: time.Millisecond, + Timeout: time.Millisecond, + CleanupPolicy: DeleteRollingUpdateCleanupPolicy, + } + if err := updater.Update(config); err != nil { t.Errorf("Update failed: %v", err) } if buffer.String() != output { t.Errorf("Output was not as expected. Expected:\n%s\nGot:\n%s", output, buffer.String()) } } + +// TestRollingUpdater_preserveCleanup ensures that the old controller isn't +// deleted following a successful deployment. +func TestRollingUpdater_preserveCleanup(t *testing.T) { + rc := oldRc(2) + rcExisting := newRc(1, 3) + + updater := &RollingUpdater{ + ns: "default", + c: &rollingUpdaterClientImpl{ + GetReplicationControllerFn: func(namespace, name string) (*api.ReplicationController, error) { + switch name { + case rc.Name: + return rc, nil + case rcExisting.Name: + return rcExisting, nil + default: + return nil, fmt.Errorf("unexpected get call for %s/%s", namespace, name) + } + }, + UpdateReplicationControllerFn: func(namespace string, rc *api.ReplicationController) (*api.ReplicationController, error) { + return rc, nil + }, + CreateReplicationControllerFn: func(namespace string, rc *api.ReplicationController) (*api.ReplicationController, error) { + t.Fatalf("unexpected call to create %s/rc:%#v", namespace, rc) + return nil, nil + }, + DeleteReplicationControllerFn: func(namespace, name string) error { + t.Fatalf("unexpected call to delete %s/%s", namespace, name) + return nil + }, + ControllerHasDesiredReplicasFn: func(rc *api.ReplicationController) wait.ConditionFunc { + return func() (done bool, err error) { + return true, nil + } + }, + }, + } + + config := &RollingUpdaterConfig{ + Out: ioutil.Discard, + OldRc: rc, + NewRc: rcExisting, + UpdatePeriod: 0, + Interval: time.Millisecond, + Timeout: time.Millisecond, + CleanupPolicy: PreserveRollingUpdateCleanupPolicy, + } + err := updater.Update(config) + if err != nil { + t.Errorf("unexpected error: %v", err) + } +} + +// rollingUpdaterClientImpl is a dynamic RollingUpdaterClient. +type rollingUpdaterClientImpl struct { + GetReplicationControllerFn func(namespace, name string) (*api.ReplicationController, error) + UpdateReplicationControllerFn func(namespace string, rc *api.ReplicationController) (*api.ReplicationController, error) + CreateReplicationControllerFn func(namespace string, rc *api.ReplicationController) (*api.ReplicationController, error) + DeleteReplicationControllerFn func(namespace, name string) error + ControllerHasDesiredReplicasFn func(rc *api.ReplicationController) wait.ConditionFunc +} + +func (c *rollingUpdaterClientImpl) GetReplicationController(namespace, name string) (*api.ReplicationController, error) { + return c.GetReplicationControllerFn(namespace, name) +} + +func (c *rollingUpdaterClientImpl) UpdateReplicationController(namespace string, rc *api.ReplicationController) (*api.ReplicationController, error) { + return c.UpdateReplicationControllerFn(namespace, rc) +} + +func (c *rollingUpdaterClientImpl) CreateReplicationController(namespace string, rc *api.ReplicationController) (*api.ReplicationController, error) { + return c.CreateReplicationControllerFn(namespace, rc) +} + +func (c *rollingUpdaterClientImpl) DeleteReplicationController(namespace, name string) error { + return c.DeleteReplicationControllerFn(namespace, name) +} + +func (c *rollingUpdaterClientImpl) ControllerHasDesiredReplicas(rc *api.ReplicationController) wait.ConditionFunc { + return c.ControllerHasDesiredReplicasFn(rc) +}