Merge pull request #12188 from mfojtik/recreate-proportional

Merged by openshift-bot

pkg/deploy/api/types.go

@@ -10,6 +10,8 @@ import (
 const (
 	// DefaultRollingTimeoutSeconds is the default TimeoutSeconds for RollingDeploymentStrategyParams.
 	DefaultRollingTimeoutSeconds int64 = 10 * 60
+	// DefaultRecreateTimeoutSeconds is the default TimeoutSeconds for RecreateDeploymentStrategyParams.
+	DefaultRecreateTimeoutSeconds int64 = 2 * 60
 	// DefaultRollingIntervalSeconds is the default IntervalSeconds for RollingDeploymentStrategyParams.
 	DefaultRollingIntervalSeconds int64 = 1
 	// DefaultRollingUpdatePeriodSeconds is the default PeriodSeconds for RollingDeploymentStrategyParams.

pkg/deploy/strategy/recreate/recreate.go

@@ -54,18 +54,21 @@ type RecreateDeploymentStrategy struct {
 	decoder runtime.Decoder
 	// hookExecutor can execute a lifecycle hook.
 	hookExecutor stratsupport.HookExecutor
-	// retryTimeout is how long to wait for the replica count update to succeed
-	// before giving up.
-	retryTimeout time.Duration
 	// retryPeriod is how often to try updating the replica count.
 	retryPeriod time.Duration
+	// retryParams encapsulates the retry parameters
+	retryParams *kubectl.RetryParams
 	// events records the events
 	events record.EventSink
+	// now returns the current time
+	now func() time.Time
 }
 
-// acceptorInterval is how often the UpdateAcceptor should check for
-// readiness.
-const acceptorInterval = 1 * time.Second
+const (
+	// acceptorInterval is how often the UpdateAcceptor should check for
+	// readiness.
+	acceptorInterval = 1 * time.Second
+)
 
 // NewRecreateDeploymentStrategy makes a RecreateDeploymentStrategy backed by
 // a real HookExecutor and client.
@@ -79,23 +82,23 @@ func NewRecreateDeploymentStrategy(oldClient kclient.Interface, tagClient client
 	scaler, _ := kubectl.ScalerFor(kapi.Kind("ReplicationController"), oldClient)
 	// TODO internalclientset: get rid of oldClient after next rebase
 	client := adapter.FromUnversionedClient(oldClient.(*kclient.Client))
+
 	return &RecreateDeploymentStrategy{
 		out:         out,
 		errOut:      errOut,
 		events:      events,
 		until:       until,
 		rcClient:    client.Core(),
-		podClient:   client.Core(),
 		eventClient: client.Core(),
+		podClient:   client.Core(),
 		getUpdateAcceptor: func(timeout time.Duration, minReadySeconds int32) strat.UpdateAcceptor {
 			return stratsupport.NewAcceptAvailablePods(out, client.Core(), timeout, acceptorInterval, minReadySeconds)
 		},
 		scaler:       scaler,
 		decoder:      decoder,
 		hookExecutor: stratsupport.NewHookExecutor(client.Core(), tagClient, client.Core(), os.Stdout, decoder),
-		// TODO: Should be config.Spec.Strategy.RecreateParams.TimeoutSeconds - (time.Now - deployerPodStartTime)
-		retryTimeout: 120 * time.Second,
 		retryPeriod:  1 * time.Second,
+		now:          func() time.Time { return time.Now() },
 	}
 }
 
@@ -117,12 +120,42 @@ func (s *RecreateDeploymentStrategy) DeployWithAcceptor(from *kapi.ReplicationCo
 		return fmt.Errorf("couldn't decode config from deployment %s: %v", to.Name, err)
 	}
 
+	var retryTimeout time.Duration
 	params := config.Spec.Strategy.RecreateParams
-	retryParams := kubectl.NewRetryParams(s.retryPeriod, s.retryTimeout)
-	waitParams := kubectl.NewRetryParams(s.retryPeriod, s.retryTimeout)
+
+	// for rolling strategy recreate might be the "initial strategy" and for that we need to
+	// set the TimeoutSeconds to rolling params.
+	rollingParams := config.Spec.Strategy.RollingParams
+
+	if params != nil {
+		if params.TimeoutSeconds != nil {
+			retryTimeout = time.Duration(*params.TimeoutSeconds) * time.Second
+		} else {
+			retryTimeout = time.Duration(deployapi.DefaultRecreateTimeoutSeconds) * time.Second
+		}
+	}
+
+	if retryTimeout == 0 && rollingParams != nil {
+		if rollingParams.TimeoutSeconds != nil {
+			retryTimeout = time.Duration(*rollingParams.TimeoutSeconds) * time.Second
+		} else {
+			retryTimeout = time.Duration(deployapi.DefaultRollingIntervalSeconds) * time.Second
+		}
+	}
+
+	deployerPod, err := s.podClient.Pods(to.Namespace).Get(deployutil.DeployerPodNameForDeployment(to.Name))
+	if err == nil {
+		deployerRunningSeconds := deployerPod.Status.StartTime.Time.Sub(s.now())
+		retryTimeout -= deployerRunningSeconds
+	}
+	fmt.Fprintf(s.out, "--> Waiting up to %s for rollout to finish\n", retryTimeout)
+
+	s.retryParams = kubectl.NewRetryParams(s.retryPeriod, retryTimeout)
+	waitParams := kubectl.NewRetryParams(s.retryPeriod, retryTimeout)
 
 	if updateAcceptor == nil {
-		updateAcceptor = s.getUpdateAcceptor(time.Duration(*params.TimeoutSeconds)*time.Second, config.Spec.MinReadySeconds)
+		// updateAcceptor = s.getUpdateAcceptor(time.Duration(*params.TimeoutSeconds)*time.Second, config.Spec.MinReadySeconds)
+		updateAcceptor = s.getUpdateAcceptor(retryTimeout, config.Spec.MinReadySeconds)
 	}
 
 	// Execute any pre-hook.
@@ -143,7 +176,7 @@ func (s *RecreateDeploymentStrategy) DeployWithAcceptor(from *kapi.ReplicationCo
 	// Scale down the from deployment.
 	if from != nil {
 		fmt.Fprintf(s.out, "--> Scaling %s down to zero\n", from.Name)
-		_, err := s.scaleAndWait(from, 0, retryParams, waitParams)
+		_, err := s.scaleAndWait(from, 0, s.retryParams, waitParams)
 		if err != nil {
 			return fmt.Errorf("couldn't scale %s to 0: %v", from.Name, err)
 		}
@@ -173,7 +206,7 @@ func (s *RecreateDeploymentStrategy) DeployWithAcceptor(from *kapi.ReplicationCo
 			// Scale up to 1 and validate the replica,
 			// aborting if the replica isn't acceptable.
 			fmt.Fprintf(s.out, "--> Scaling %s to 1 before performing acceptance check\n", to.Name)
-			updatedTo, err := s.scaleAndWait(to, 1, retryParams, waitParams)
+			updatedTo, err := s.scaleAndWait(to, 1, s.retryParams, waitParams)
 			if err != nil {
 				return fmt.Errorf("couldn't scale %s to 1: %v", to.Name, err)
 			}
@@ -191,7 +224,7 @@ func (s *RecreateDeploymentStrategy) DeployWithAcceptor(from *kapi.ReplicationCo
 		// Complete the scale up.
 		if to.Spec.Replicas != int32(desiredReplicas) {
 			fmt.Fprintf(s.out, "--> Scaling %s to %d\n", to.Name, desiredReplicas)
-			updatedTo, err := s.scaleAndWait(to, desiredReplicas, retryParams, waitParams)
+			updatedTo, err := s.scaleAndWait(to, desiredReplicas, s.retryParams, waitParams)
 			if err != nil {
 				return fmt.Errorf("couldn't scale %s to %d: %v", to.Name, desiredReplicas, err)
 			}