@@ -60,6 +60,9 @@ type RollingUpdaterConfig struct {

 	// CleanupPolicy defines the cleanup action to take after the deployment is

 	// complete.

 	CleanupPolicy RollingUpdaterCleanupPolicy

+	// UpdateAcceptor is optional and drives acceptance of the first controller

+	// during scale-up. If nil, controllers are always accepted.

+	UpdateAcceptor UpdateAcceptor

 }

 // RollingUpdaterCleanupPolicy is a cleanup action to take after the

@@ -76,6 +79,20 @@ const (

 	RenameRollingUpdateCleanupPolicy RollingUpdaterCleanupPolicy = "Rename"

 )

+// UpdateAcceptor is given a chance to accept or reject the first controller

+// during a deployment.

+//

+// After the successful scale-up of the first replica, the replica is given

+// the the UpdateAcceptor. If the UpdateAcceptor rejects the replica, the

+// deployment is stopped with an error.

+//

+// Only the first replica scaled up during the deployment is checked for

+// acceptance.

+type UpdateAcceptor interface {

+	// Accept returns nil if the controller is okay, otherwise returns an error.

+	Accept(*api.ReplicationController) error

+}

+

 func LoadExistingNextReplicationController(c *client.Client, namespace, newName string) (*api.ReplicationController, error) {

 	if len(newName) == 0 {

 		return nil, nil

@@ -354,6 +371,7 @@ func (r *RollingUpdater) Update(config *RollingUpdaterConfig) error {

 	}

 	// +1, -1 on oldRc, newRc until newRc has desired number of replicas or oldRc has 0 replicas

+	updateAccepted := false

 	for newRc.Spec.Replicas < desired && oldRc.Spec.Replicas != 0 {

 		newRc.Spec.Replicas += 1

 		oldRc.Spec.Replicas -= 1

@@ -368,6 +386,14 @@ func (r *RollingUpdater) Update(config *RollingUpdaterConfig) error {

 		if err != nil {

 			return err

 		}

+		// Perform the update acceptance check exactly once.

+		if config.UpdateAcceptor != nil && !updateAccepted {

+			err := config.UpdateAcceptor.Accept(newRc)

+			if err != nil {

+				return fmt.Errorf("Update rejected for %s: %v", newRc.Name, err)

+			}

+			updateAccepted = true

+		}

 		time.Sleep(updatePeriod)

 		oldRc, err = r.scaleAndWait(oldRc, retry, waitForReplicas)

 		if err != nil {

@@ -2,9 +2,12 @@ package deployer

 import (

 	"fmt"

+	"sort"

+	"time"

 	kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"

 	kclient "github.com/GoogleCloudPlatform/kubernetes/pkg/client"

+	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl"

 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"

 	"github.com/golang/glog"

 	"github.com/spf13/cobra"

@@ -32,11 +35,6 @@ type config struct {

 	Namespace      string

 }

-type replicationControllerGetter interface {

-	Get(namespace, name string) (*kapi.ReplicationController, error)

-	List(namespace string, selector labels.Selector) (*kapi.ReplicationControllerList, error)

-}

-

 // NewCommandDeployer provides a CLI handler for deploy.

 func NewCommandDeployer(name string) *cobra.Command {

 	cfg := &config{

@@ -61,7 +59,8 @@ func NewCommandDeployer(name string) *cobra.Command {

 				glog.Fatal("namespace is required")

 			}

-			if err = deploy(kClient, cfg.Namespace, cfg.DeploymentName); err != nil {

+			deployer := NewDeployer(kClient)

+			if err = deployer.Deploy(cfg.Namespace, cfg.DeploymentName); err != nil {

 				glog.Fatal(err)

 			}

 		},

@@ -77,88 +76,124 @@ func NewCommandDeployer(name string) *cobra.Command {

 	return cmd

 }

-// deploy executes a deployment strategy.

-func deploy(kClient kclient.Interface, namespace, deploymentName string) error {

-	deployment, oldDeployments, err := getDeployerContext(&realReplicationControllerGetter{kClient}, namespace, deploymentName)

+// NewDeployer makes a new Deployer from a kube client.

+func NewDeployer(client kclient.Interface) *Deployer {

+	scaler, _ := kubectl.ScalerFor("ReplicationController", kubectl.NewScalerClient(client))

+	return &Deployer{

+		getDeployment: func(namespace, name string) (*kapi.ReplicationController, error) {

+			return client.ReplicationControllers(namespace).Get(name)

+		},

+		getControllers: func(namespace string) (*kapi.ReplicationControllerList, error) {

+			return client.ReplicationControllers(namespace).List(labels.Everything())

+		},

+		scaler: scaler,

+		strategyFor: func(config *deployapi.DeploymentConfig) (strategy.DeploymentStrategy, error) {

+			switch config.Template.Strategy.Type {

+			case deployapi.DeploymentStrategyTypeRecreate:

+				return recreate.NewRecreateDeploymentStrategy(client, latest.Codec), nil

+			case deployapi.DeploymentStrategyTypeRolling:

+				recreate := recreate.NewRecreateDeploymentStrategy(client, latest.Codec)

+				return rolling.NewRollingDeploymentStrategy(config.Namespace, client, latest.Codec, recreate), nil

+			default:

+				return nil, fmt.Errorf("unsupported strategy type: %s", config.Template.Strategy.Type)

+			}

+		},

+	}

+}

+

+// Deployer prepares and executes the deployment process. It will:

+//

+// 1. Validate the deployment has a desired replica count and strategy.

+// 2. Find the last completed deployment.

+// 3. Scale down to 0 any old deployments which aren't the new deployment or

+// the last complete deployment.

+// 4. Pass the last completed deployment and the new deployment to a strategy

+// to perform the deployment.

+type Deployer struct {

+	// strategyFor returns a DeploymentStrategy for config.

+	strategyFor func(config *deployapi.DeploymentConfig) (strategy.DeploymentStrategy, error)

+	// getDeployment finds the named deployment.

+	getDeployment func(namespace, name string) (*kapi.ReplicationController, error)

+	// getControllers finds all controllers in namespace.

+	getControllers func(namespace string) (*kapi.ReplicationControllerList, error)

+	// scaler is used to scale replication controllers.

+	scaler kubectl.Scaler

+}

+

+// Deploy starts the deployment process for deploymentName.

+func (d *Deployer) Deploy(namespace, deploymentName string) error {

+	// Look up the new deployment.

+	deployment, err := d.getDeployment(namespace, deploymentName)

 	if err != nil {

-		return err

+		return fmt.Errorf("couldn't get deployment %s/%s: %v", namespace, deploymentName, err)

 	}

+	// Decode the config from the deployment.

 	config, err := deployutil.DecodeDeploymentConfig(deployment, latest.Codec)

 	if err != nil {

 		return fmt.Errorf("couldn't decode DeploymentConfig from deployment %s/%s: %v", deployment.Namespace, deployment.Name, err)

 	}

-	var strategy strategy.DeploymentStrategy

-

-	switch config.Template.Strategy.Type {

-	case deployapi.DeploymentStrategyTypeRecreate:

-		strategy = recreate.NewRecreateDeploymentStrategy(kClient, latest.Codec)

-	case deployapi.DeploymentStrategyTypeRolling:

-		recreate := recreate.NewRecreateDeploymentStrategy(kClient, latest.Codec)

-		strategy = rolling.NewRollingDeploymentStrategy(deployment.Namespace, kClient, latest.Codec, recreate)

-	default:

-		return fmt.Errorf("unsupported strategy type: %s", config.Template.Strategy.Type)

+	// Get a strategy for the deployment.

+	strategy, err := d.strategyFor(config)

+	if err != nil {

+		return err

 	}

-	return strategy.Deploy(deployment, oldDeployments)

-}

-

-// getDeployerContext finds the target deployment and any deployments it considers to be prior to the

-// target deployment. Only deployments whose LatestVersion is less than the target deployment are

-// considered to be prior.

-func getDeployerContext(controllerGetter replicationControllerGetter, namespace, deploymentName string) (*kapi.ReplicationController, []*kapi.ReplicationController, error) {

-	var err error

-	var newDeployment *kapi.ReplicationController

-	var newConfig *deployapi.DeploymentConfig

-

-	// Look up the new deployment and its associated config.

-	if newDeployment, err = controllerGetter.Get(namespace, deploymentName); err != nil {

-		return nil, nil, err

+	// New deployments must have a desired replica count.

+	desiredReplicas, hasDesired := deployutil.DeploymentDesiredReplicas(deployment)

+	if !hasDesired {

+		return fmt.Errorf("deployment %s has no desired replica count", deployutil.LabelForDeployment(deployment))

 	}

-	if newConfig, err = deployutil.DecodeDeploymentConfig(newDeployment, latest.Codec); err != nil {

-		return nil, nil, err

+	// Find all controllers in order to pick out the deployments.

+	controllers, err := d.getControllers(namespace)

+	if err != nil {

+		return fmt.Errorf("couldn't get controllers in namespace %s: %v", namespace, err)

 	}

-	glog.Infof("Found new Deployment %s for DeploymentConfig %s/%s with latestVersion %d", newDeployment.Name, newConfig.Namespace, newConfig.Name, newConfig.LatestVersion)

-

-	// Collect all deployments that predate the new one by comparing all old ReplicationControllers with

-	// encoded DeploymentConfigs to the new one by LatestVersion. Treat a failure to interpret a given

-	// old deployment as a fatal error to prevent overlapping deployments.

-	var allControllers *kapi.ReplicationControllerList

-	oldDeployments := []*kapi.ReplicationController{}

+	// Find all deployments sorted by version.

+	deployments := deployutil.ConfigSelector(config.Name, controllers.Items)

+	sort.Sort(deployutil.DeploymentsByLatestVersionDesc(deployments))

-	if allControllers, err = controllerGetter.List(newDeployment.Namespace, labels.Everything()); err != nil {

-		return nil, nil, fmt.Errorf("unable to get list replication controllers in deployment namespace %s: %v", newDeployment.Namespace, err)

+	// Find any last completed deployment.

+	var lastDeployment *kapi.ReplicationController

+	for _, candidate := range deployments {

+		if candidate.Name == deployment.Name {

+			continue

+		}

+		if deployutil.DeploymentStatusFor(&candidate) == deployapi.DeploymentStatusComplete {

+			lastDeployment = &candidate

+			glog.Infof("Picked %s as the last completed deployment", deployutil.LabelForDeployment(&candidate))

+			break

+		}

+	}

+	if lastDeployment == nil {

+		glog.Info("No last completed deployment found")

 	}

-	glog.Infof("Inspecting %d potential prior deployments", len(allControllers.Items))

-	for i, controller := range allControllers.Items {

-		if oldName := deployutil.DeploymentConfigNameFor(&controller); oldName != newConfig.Name {

-			glog.Infof("Disregarding deployment %s (doesn't match target DeploymentConfig %s)", controller.Name, oldName)

+	// Scale down any deployments which aren't the new or last deployment.

+	for _, candidate := range deployments {

+		// Skip the from/to deployments.

+		if candidate.Name == deployment.Name {

 			continue

 		}

-

-		if deployutil.DeploymentVersionFor(&controller) < newConfig.LatestVersion {

-			glog.Infof("Marking deployment %s as a prior deployment", controller.Name)

-			oldDeployments = append(oldDeployments, &allControllers.Items[i])

+		if lastDeployment != nil && candidate.Name == lastDeployment.Name {

+			continue

+		}

+		// Skip the deployment if it's already scaled down.

+		if candidate.Spec.Replicas == 0 {

+			continue

+		}

+		// Scale the deployment down to zero.

+		retryWaitParams := kubectl.NewRetryParams(1*time.Second, 120*time.Second)

+		if err := d.scaler.Scale(candidate.Namespace, candidate.Name, uint(0), &kubectl.ScalePrecondition{-1, ""}, retryWaitParams, retryWaitParams); err != nil {

+			glog.Infof("Couldn't scale down prior deployment %s: %v", deployutil.LabelForDeployment(&candidate), err)

 		} else {

-			glog.Infof("Disregarding deployment %s (same as or newer than target)", controller.Name)

+			glog.Infof("Scaled down prior deployment %s", deployutil.LabelForDeployment(&candidate))

 		}

 	}

-	return newDeployment, oldDeployments, nil

-}

-

-type realReplicationControllerGetter struct {

-	kClient kclient.Interface

-}

-

-func (r *realReplicationControllerGetter) Get(namespace, name string) (*kapi.ReplicationController, error) {

-	return r.kClient.ReplicationControllers(namespace).Get(name)

-}

-

-func (r *realReplicationControllerGetter) List(namespace string, selector labels.Selector) (*kapi.ReplicationControllerList, error) {

-	return r.kClient.ReplicationControllers(namespace).List(selector)

+	// Perform the deployment.

+	return strategy.Deploy(lastDeployment, deployment, desiredReplicas)

 }

@@ -1,172 +1,204 @@

 package deployer

 import (

+	"fmt"

+	"strconv"

 	"testing"

 	kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"

-	kerrors "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"

-	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"

 	deployapi "github.com/openshift/origin/pkg/deploy/api"

 	deploytest "github.com/openshift/origin/pkg/deploy/api/test"

+	scalertest "github.com/openshift/origin/pkg/deploy/scaler/test"

+	"github.com/openshift/origin/pkg/deploy/strategy"

 	deployutil "github.com/openshift/origin/pkg/deploy/util"

 )

-func TestGetDeploymentContextMissingDeployment(t *testing.T) {

-	getter := &testReplicationControllerGetter{

-		getFunc: func(namespace, name string) (*kapi.ReplicationController, error) {

-			return nil, kerrors.NewNotFound("replicationController", name)

-		},

-		listFunc: func(namespace string, selector labels.Selector) (*kapi.ReplicationControllerList, error) {

-			t.Fatal("unexpected list call")

+func TestDeployer_getDeploymentFail(t *testing.T) {

+	deployer := &Deployer{

+		strategyFor: func(config *deployapi.DeploymentConfig) (strategy.DeploymentStrategy, error) {

+			t.Fatal("unexpected call")

 			return nil, nil

 		},

-	}

-

-	newDeployment, oldDeployments, err := getDeployerContext(getter, kapi.NamespaceDefault, "deployment")

-

-	if newDeployment != nil {

-		t.Fatalf("unexpected newDeployment: %#v", newDeployment)

-	}

-

-	if oldDeployments != nil {

-		t.Fatalf("unexpected oldDeployments: %#v", oldDeployments)

-	}

-

-	if err == nil {

-		t.Fatal("expected an error")

-	}

-}

-

-func TestGetDeploymentContextInvalidEncodedConfig(t *testing.T) {

-	getter := &testReplicationControllerGetter{

-		getFunc: func(namespace, name string) (*kapi.ReplicationController, error) {

-			return &kapi.ReplicationController{}, nil

+		getDeployment: func(namespace, name string) (*kapi.ReplicationController, error) {

+			return nil, fmt.Errorf("get error")

 		},

-		listFunc: func(namespace string, selector labels.Selector) (*kapi.ReplicationControllerList, error) {

-			return &kapi.ReplicationControllerList{}, nil

+		getControllers: func(namespace string) (*kapi.ReplicationControllerList, error) {

+			t.Fatal("unexpected call")

+			return nil, nil

 		},

+		scaler: &scalertest.FakeScaler{},

 	}

-	newDeployment, oldDeployments, err := getDeployerContext(getter, kapi.NamespaceDefault, "deployment")

-

-	if newDeployment != nil {

-		t.Fatalf("unexpected newDeployment: %#v", newDeployment)

-	}

-

-	if oldDeployments != nil {

-		t.Fatalf("unexpected oldDeployments: %#v", oldDeployments)

-	}

-

+	err := deployer.Deploy("namespace", "name")

 	if err == nil {

-		t.Fatal("expected an error")

+		t.Fatalf("expected an error")

 	}

+	t.Logf("got expected error: %v", err)

 }

-func TestGetDeploymentContextNoPriorDeployments(t *testing.T) {

-	getter := &testReplicationControllerGetter{

-		getFunc: func(namespace, name string) (*kapi.ReplicationController, error) {

-			deployment, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(1), kapi.Codec)

-			return deployment, nil

+func TestDeployer_deployScenarios(t *testing.T) {

+	mkd := func(version int, status deployapi.DeploymentStatus, replicas int, desired int) *kapi.ReplicationController {

+		deployment := mkdeployment(version, status)

+		deployment.Spec.Replicas = replicas

+		if desired > 0 {

+			deployment.Annotations[deployapi.DesiredReplicasAnnotation] = strconv.Itoa(desired)

+		}

+		return deployment

+	}

+	type scaleEvent struct {

+		version int

+		size    int

+	}

+	scenarios := []struct {

+		name        string

+		deployments []*kapi.ReplicationController

+		fromVersion int

+		toVersion   int

+		scaleEvents []scaleEvent

+	}{

+		{

+			"initial deployment",

+			// existing deployments

+			[]*kapi.ReplicationController{

+				mkd(1, deployapi.DeploymentStatusNew, 0, 3),

+			},

+			// from and to version

+			0, 1,

+			// expected scale events

+			[]scaleEvent{},

 		},

-		listFunc: func(namespace string, selector labels.Selector) (*kapi.ReplicationControllerList, error) {

-			return &kapi.ReplicationControllerList{}, nil

+		{

+			"last deploy failed",

+			// existing deployments

+			[]*kapi.ReplicationController{

+				mkd(1, deployapi.DeploymentStatusComplete, 3, 0),

+				mkd(2, deployapi.DeploymentStatusFailed, 1, 3),

+				mkd(3, deployapi.DeploymentStatusNew, 0, 3),

+			},

+			// from and to version

+			1, 3,

+			// expected scale events

+			[]scaleEvent{

+				{2, 0},

+			},

 		},

-	}

-

-	newDeployment, oldDeployments, err := getDeployerContext(getter, kapi.NamespaceDefault, "deployment")

-

-	if err != nil {

-		t.Fatalf("unexpected error: %v", err)

-	}

-

-	if newDeployment == nil {

-		t.Fatal("expected deployment")

-	}

-

-	if oldDeployments == nil {

-		t.Fatal("expected non-nil oldDeployments")

-	}

-

-	if len(oldDeployments) > 0 {

-		t.Fatalf("unexpected non-empty oldDeployments: %#v", oldDeployments)

-	}

-}

-

-func TestGetDeploymentContextWithPriorDeployments(t *testing.T) {

-	getter := &testReplicationControllerGetter{

-		getFunc: func(namespace, name string) (*kapi.ReplicationController, error) {

-			deployment, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(3), kapi.Codec)

-			return deployment, nil

+		{

+			"sequential complete",

+			// existing deployments

+			[]*kapi.ReplicationController{

+				mkd(1, deployapi.DeploymentStatusComplete, 0, 0),

+				mkd(2, deployapi.DeploymentStatusComplete, 3, 0),

+				mkd(3, deployapi.DeploymentStatusNew, 0, 3),

+			},

+			// from and to version

+			2, 3,

+			// expected scale events

+			[]scaleEvent{},

 		},

-		listFunc: func(namespace string, selector labels.Selector) (*kapi.ReplicationControllerList, error) {

-			deployment1, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(1), kapi.Codec)

-			deployment2, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(2), kapi.Codec)

-			deployment3, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(4), kapi.Codec)

-			deployment4, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(1), kapi.Codec)

-			deployment4.Annotations[deployapi.DeploymentConfigAnnotation] = "another-config"

-			return &kapi.ReplicationControllerList{

-				Items: []kapi.ReplicationController{

-					*deployment1,

-					*deployment2,

-					*deployment3,

-					*deployment4,

-					{},

-				},

-			}, nil

+		{

+			"sequential failure",

+			// existing deployments

+			[]*kapi.ReplicationController{

+				mkd(1, deployapi.DeploymentStatusFailed, 1, 3),

+				mkd(2, deployapi.DeploymentStatusFailed, 1, 3),

+				mkd(3, deployapi.DeploymentStatusNew, 0, 3),

+			},

+			// from and to version

+			0, 3,

+			// expected scale events

+			[]scaleEvent{

+				{1, 0},

+				{2, 0},

+			},

 		},

 	}

-	newDeployment, oldDeployments, err := getDeployerContext(getter, kapi.NamespaceDefault, "deployment")

-

-	if err != nil {

-		t.Fatalf("unexpected error: %v", err)

-	}

+	for _, s := range scenarios {

+		t.Logf("executing scenario %s", s.name)

+		findDeployment := func(version int) *kapi.ReplicationController {

+			for _, d := range s.deployments {

+				if deployutil.DeploymentVersionFor(d) == version {

+					return d

+				}

+			}

+			return nil

+		}

-	if newDeployment == nil {

-		t.Fatal("expected deployment")

-	}

+		var actualFrom, actualTo *kapi.ReplicationController

+		var actualDesired int

+		to := findDeployment(s.toVersion)

+		scaler := &scalertest.FakeScaler{}

+

+		deployer := &Deployer{

+			strategyFor: func(config *deployapi.DeploymentConfig) (strategy.DeploymentStrategy, error) {

+				return &testStrategy{

+					deployFunc: func(from *kapi.ReplicationController, to *kapi.ReplicationController, desiredReplicas int) error {

+						actualFrom = from

+						actualTo = to

+						actualDesired = desiredReplicas

+						return nil

+					},

+				}, nil

+			},

+			getDeployment: func(namespace, name string) (*kapi.ReplicationController, error) {

+				return to, nil

+			},

+			getControllers: func(namespace string) (*kapi.ReplicationControllerList, error) {

+				list := &kapi.ReplicationControllerList{}

+				for _, d := range s.deployments {

+					list.Items = append(list.Items, *d)

+				}

+				return list, nil

+			},

+			scaler: scaler,

+		}

-	if oldDeployments == nil {

-		t.Fatal("expected non-nil oldDeployments")

-	}

+		err := deployer.Deploy(to.Namespace, to.Name)

+		if err != nil {

+			t.Fatalf("unexpcted error: %v", err)

+		}

-	expected := []string{"config-1", "config-2"}

-	for _, e := range expected {

-		found := false

-		for _, d := range oldDeployments {

-			if d.Name == e {

-				found = true

-				break

+		if s.fromVersion > 0 {

+			if e, a := s.fromVersion, deployutil.DeploymentVersionFor(actualFrom); e != a {

+				t.Fatalf("expected from.latestVersion %d, got %d", e, a)

 			}

 		}

-		if !found {

-			t.Errorf("expected to find old deployment %s", e)

+		if e, a := s.toVersion, deployutil.DeploymentVersionFor(actualTo); e != a {

+			t.Fatalf("expected to.latestVersion %d, got %d", e, a)

 		}

-	}

-	for _, d := range oldDeployments {

-		ok := false

-		for _, e := range expected {

-			if d.Name == e {

-				ok = true

-				break

-			}

+		if e, a := len(s.scaleEvents), len(scaler.Events); e != a {

+			t.Fatalf("expected %d scale events, got %d", e, a)

 		}

-		if !ok {

-			t.Errorf("unexpected old deployment %s", d.Name)

+		for _, expected := range s.scaleEvents {

+			expectedTo := findDeployment(expected.version)

+			expectedWasScaled := false

+			for _, actual := range scaler.Events {

+				if actual.Name != expectedTo.Name {

+					continue

+				}

+				if e, a := uint(expected.size), actual.Size; e != a {

+					t.Fatalf("expected version %d to be scaled to %d, got %d", expected.version, e, a)

+				}

+				expectedWasScaled = true

+			}

+			if !expectedWasScaled {

+				t.Fatalf("expected version %d to be scaled to %d, but it wasn't scaled at all", expected.version, expected.size)

+			}

 		}

 	}

 }

-type testReplicationControllerGetter struct {

-	getFunc  func(namespace, name string) (*kapi.ReplicationController, error)

-	listFunc func(namespace string, selector labels.Selector) (*kapi.ReplicationControllerList, error)

+func mkdeployment(version int, status deployapi.DeploymentStatus) *kapi.ReplicationController {

+	deployment, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(version), kapi.Codec)

+	deployment.Annotations[deployapi.DeploymentStatusAnnotation] = string(status)

+	return deployment

 }

-func (t *testReplicationControllerGetter) Get(namespace, name string) (*kapi.ReplicationController, error) {

-	return t.getFunc(namespace, name)

+type testStrategy struct {

+	deployFunc func(from *kapi.ReplicationController, to *kapi.ReplicationController, desiredReplicas int) error

 }

-func (t *testReplicationControllerGetter) List(namespace string, selector labels.Selector) (*kapi.ReplicationControllerList, error) {

-	return t.listFunc(namespace, selector)

+func (t *testStrategy) Deploy(from *kapi.ReplicationController, to *kapi.ReplicationController, desiredReplicas int) error {

+	return t.deployFunc(from, to, desiredReplicas)

 }

@@ -2,6 +2,7 @@ package deploymentconfig

 import (

 	"fmt"

+	"sort"

 	"strconv"

 	"github.com/golang/glog"

@@ -116,21 +117,19 @@ func (c *DeploymentConfigController) Handle(config *deployapi.DeploymentConfig)

 		return fatalError(fmt.Sprintf("couldn't make Deployment from (potentially invalid) DeploymentConfig %s: %v", deployutil.LabelForDeploymentConfig(config), err))

 	}

-	// Compute the desired replicas for the deployment. The count should match

-	// the existing deployment replica count. To find this, simply sum the

-	// replicas of existing deployments for this config. Any deactivated

-	// deployments should already be scaled down to zero, and so the sum should

-	// reflect the count of the latest active deployment.

-	//

-	// If there are no existing deployments, use the replica count from the

-	// config template.

+	// Compute the desired replicas for the deployment. Use the last completed

+	// deployment's current replica count, or the config template if there is no

+	// prior completed deployment available.

 	desiredReplicas := config.Template.ControllerTemplate.Replicas

 	if len(existingDeployments.Items) > 0 {

-		desiredReplicas = 0

+		sort.Sort(deployutil.DeploymentsByLatestVersionDesc(existingDeployments.Items))

 		for _, existing := range existingDeployments.Items {

-			desiredReplicas += existing.Spec.Replicas

+			if deployutil.DeploymentStatusFor(&existing) == deployapi.DeploymentStatusComplete {

+				desiredReplicas = existing.Spec.Replicas

+				glog.V(4).Infof("Desired replicas for %s set to %d based on prior completed deployment %s", deployutil.LabelForDeploymentConfig(config), desiredReplicas, existing.Name)

+				break

+			}

 		}

-		glog.V(4).Infof("Desired replicas for %s adjusted to %d based on %d existing deployments", deployutil.LabelForDeploymentConfig(config), desiredReplicas, len(existingDeployments.Items))

 	}

 	deployment.Annotations[deployapi.DesiredReplicasAnnotation] = strconv.Itoa(desiredReplicas)

@@ -4,7 +4,6 @@ import (

 	"fmt"

 	"reflect"

 	"sort"

-	"strconv"

 	"testing"

 	kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"

@@ -81,6 +80,7 @@ func TestHandle_updateOk(t *testing.T) {

 	type existing struct {

 		version  int

 		replicas int

+		status   deployapi.DeploymentStatus

 	}

 	type scenario struct {

@@ -90,14 +90,29 @@ func TestHandle_updateOk(t *testing.T) {

 	}

 	scenarios := []scenario{

-		// No existing deployments

 		{1, 1, []existing{}},

-		// A single existing deployment

-		{2, 1, []existing{{1, 1}}},

-		// An active and deactivated existing deployment

-		{3, 2, []existing{{2, 2}, {1, 0}}},

-		// An active and deactivated existing deployment with weird ordering

-		{4, 3, []existing{{1, 0}, {2, 0}, {3, 3}}},

+		{2, 1, []existing{

+			{1, 1, deployapi.DeploymentStatusComplete},

+		}},

+		{3, 4, []existing{

+			{1, 0, deployapi.DeploymentStatusComplete},

+			{2, 4, deployapi.DeploymentStatusComplete},

+		}},

+		{3, 4, []existing{

+			{1, 4, deployapi.DeploymentStatusComplete},

+			{2, 1, deployapi.DeploymentStatusFailed},

+		}},

+		{4, 2, []existing{

+			{1, 0, deployapi.DeploymentStatusComplete},

+			{2, 0, deployapi.DeploymentStatusFailed},

+			{3, 2, deployapi.DeploymentStatusComplete},

+		}},

+		// Scramble the order of the previous to ensure we still get it right.

+		{4, 2, []existing{

+			{2, 0, deployapi.DeploymentStatusFailed},

+			{3, 2, deployapi.DeploymentStatusComplete},

+			{1, 0, deployapi.DeploymentStatusComplete},

+		}},

 	}

 	for _, scenario := range scenarios {

@@ -107,7 +122,7 @@ func TestHandle_updateOk(t *testing.T) {

 		for _, e := range scenario.existing {

 			d, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(e.version), api.Codec)

 			d.Spec.Replicas = e.replicas

-			d.Annotations[deployapi.DeploymentStatusAnnotation] = string(deployapi.DeploymentStatusComplete)

+			d.Annotations[deployapi.DeploymentStatusAnnotation] = string(e.status)

 			existingDeployments.Items = append(existingDeployments.Items, *d)

 		}

 		err := controller.Handle(config)

@@ -120,8 +135,12 @@ func TestHandle_updateOk(t *testing.T) {

 			t.Fatalf("unexpected error: %v", err)

 		}

-		if e, a := strconv.Itoa(scenario.expectedReplicas), deployed.Annotations[deployapi.DesiredReplicasAnnotation]; e != a {

-			t.Errorf("expected desired replicas %s, got %s", e, a)

+		desired, hasDesired := deployutil.DeploymentDesiredReplicas(deployed)

+		if !hasDesired {

+			t.Fatalf("expected desired replicas")

+		}

+		if e, a := scenario.expectedReplicas, desired; e != a {

+			t.Errorf("expected desired replicas %d, got %d", e, a)

 		}

 	}

 }

new file mode 100644

@@ -0,0 +1,25 @@

+package test

+

+import (

+	"fmt"

+

+	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl"

+)

+

+type FakeScaler struct {

+	Events []ScaleEvent

+}

+

+type ScaleEvent struct {

+	Name string

+	Size uint

+}

+

+func (t *FakeScaler) Scale(namespace, name string, newSize uint, preconditions *kubectl.ScalePrecondition, retry, wait *kubectl.RetryParams) error {

+	t.Events = append(t.Events, ScaleEvent{name, newSize})

+	return nil

+}

+

+func (t *FakeScaler) ScaleSimple(namespace, name string, preconditions *kubectl.ScalePrecondition, newSize uint) (string, error) {

+	return "error", fmt.Errorf("unexpected call to ScaleSimple")

+}

@@ -6,6 +6,6 @@ import (

 // DeploymentStrategy knows how to make a deployment active.

 type DeploymentStrategy interface {

-	// Deploy makes a deployment active.

-	Deploy(deployment *kapi.ReplicationController, oldDeployments []*kapi.ReplicationController) error

+	// Deploy transitions an old deployment to a new one.

+	Deploy(from *kapi.ReplicationController, to *kapi.ReplicationController, desiredReplicas int) error

 }

@@ -2,14 +2,13 @@ package recreate

 import (

 	"fmt"

-	"strconv"

 	"time"

 	"github.com/golang/glog"

 	kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"

-	kerrors "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"

 	kclient "github.com/GoogleCloudPlatform/kubernetes/pkg/client"

+	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl"

 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"

 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"

@@ -19,28 +18,36 @@ import (

 )

 // RecreateDeploymentStrategy is a simple strategy appropriate as a default.

-// Its behavior is to decrease the replica count of previous deployments to zero,

-// and to increase the replica count of the new deployment to 1.

+// Its behavior is to scale down the last deployment to 0, and to scale up the

+// new deployment to 1.

 //

 // A failure to disable any existing deployments will be considered a

 // deployment failure.

 type RecreateDeploymentStrategy struct {

-	// client is used to interact with ReplicatonControllers.

-	client replicationControllerClient

+	// getReplicationController knows how to get a replication controller.

+	getReplicationController func(namespace, name string) (*kapi.ReplicationController, error)

+	// scaler is used to scale replication controllers.

+	scaler kubectl.Scaler

 	// codec is used to decode DeploymentConfigs contained in deployments.

 	codec runtime.Codec

 	// hookExecutor can execute a lifecycle hook.

 	hookExecutor hookExecutor

-

+	// retryTimeout is how long to wait for the replica count update to succeed

+	// before giving up.

 	retryTimeout time.Duration

-	retryPeriod  time.Duration

+	// retryPeriod is how often to try updating the replica count.

+	retryPeriod time.Duration

 }

 // NewRecreateDeploymentStrategy makes a RecreateDeploymentStrategy backed by

 // a real HookExecutor and client.

 func NewRecreateDeploymentStrategy(client kclient.Interface, codec runtime.Codec) *RecreateDeploymentStrategy {

+	scaler, _ := kubectl.ScalerFor("ReplicationController", kubectl.NewScalerClient(client))

 	return &RecreateDeploymentStrategy{

-		client: &realReplicationControllerClient{client},

+		getReplicationController: func(namespace, name string) (*kapi.ReplicationController, error) {

+			return client.ReplicationControllers(namespace).Get(name)

+		},

+		scaler: scaler,

 		codec:  codec,

 		hookExecutor: &stratsupport.HookExecutor{

 			PodClient: &stratsupport.HookExecutorPodClientImpl{

@@ -52,126 +59,94 @@ func NewRecreateDeploymentStrategy(client kclient.Interface, codec runtime.Codec

 				},

 			},

 		},

-		retryTimeout: 10 * time.Second,

+		retryTimeout: 120 * time.Second,

 		retryPeriod:  1 * time.Second,

 	}

 }

 // Deploy makes deployment active and disables oldDeployments.

-func (s *RecreateDeploymentStrategy) Deploy(deployment *kapi.ReplicationController, oldDeployments []*kapi.ReplicationController) error {

-	var err error

-	var deploymentConfig *deployapi.DeploymentConfig

+func (s *RecreateDeploymentStrategy) Deploy(from *kapi.ReplicationController, to *kapi.ReplicationController, desiredReplicas int) error {

+	return s.DeployWithAcceptor(from, to, desiredReplicas, nil)

+}

-	if deploymentConfig, err = deployutil.DecodeDeploymentConfig(deployment, s.codec); err != nil {

-		return fmt.Errorf("couldn't decode DeploymentConfig from Deployment %s: %v", deployment.Name, err)

+// DeployWithAcceptor scales down from and then scales up to. If

+// updateAcceptor is provided and the desired replica count is >1, the first

+// replica of to is rolled out and validated before performing the full scale

+// up.

+//

+// This is currently only used in conjunction with the rolling update strategy

+// for initial deployments.

+func (s *RecreateDeploymentStrategy) DeployWithAcceptor(from *kapi.ReplicationController, to *kapi.ReplicationController, desiredReplicas int, updateAcceptor kubectl.UpdateAcceptor) error {

+	config, err := deployutil.DecodeDeploymentConfig(to, s.codec)

+	if err != nil {

+		return fmt.Errorf("couldn't decode config from deployment %s: %v", to.Name, err)

 	}

-	params := deploymentConfig.Template.Strategy.RecreateParams

+	params := config.Template.Strategy.RecreateParams

+	retryParams := kubectl.NewRetryParams(s.retryPeriod, s.retryTimeout)

+	waitParams := kubectl.NewRetryParams(s.retryPeriod, s.retryTimeout)

+

 	// Execute any pre-hook.

 	if params != nil && params.Pre != nil {

-		err := s.hookExecutor.Execute(params.Pre, deployment, "prehook")

-		if err != nil {

+		if err := s.hookExecutor.Execute(params.Pre, to, "prehook"); err != nil {

 			return fmt.Errorf("Pre hook failed: %s", err)

+		} else {

+			glog.Infof("Pre hook finished")

 		}

 	}

-	// Prefer to use an explicitly set desired replica count, falling back to

-	// the value defined on the config.

-	desiredReplicas := deploymentConfig.Template.ControllerTemplate.Replicas

-	if desired, hasDesired := deployment.Annotations[deployapi.DesiredReplicasAnnotation]; hasDesired {

-		val, err := strconv.Atoi(desired)

+	// Scale down the from deployment.