Max in flight flag (cloudfoundry#3085) (cloudfoundry#3096)

* Allow the user to set max-in-flight while pushing app
* Add max-in-flight flag to the restart command
* Add max-in-flight flag to restage command
* Add max-in-flight flag to rollback command
* Add max-in-flight flag to copy-src command
* Remove `--no-wait` to ensure no flakes

---------

Signed-off-by: João Pereira <joaod@vmware.com>
Co-authored-by: Pavel Busko <pavel.busko@sap.com>

actor/v7pushaction/actor.go

@@ -71,7 +71,7 @@ func NewActor(v3Actor V7Actor, sharedActor SharedActor) *Actor {
 		SetDefaultBitsPathForPushPlan,
 		SetupDropletPathForPushPlan,
 		actor.SetupAllResourcesForPushPlan,
-		SetupDeploymentStrategyForPushPlan,
+		SetupDeploymentInformationForPushPlan,
 		SetupNoStartForPushPlan,
 		SetupNoWaitForPushPlan,
 		SetupTaskAppForPushPlan,

actor/v7pushaction/actor_test.go

@@ -22,7 +22,7 @@ var _ = Describe("Actor", func() {
 				SetDefaultBitsPathForPushPlan,
 				SetupDropletPathForPushPlan,
 				actor.SetupAllResourcesForPushPlan,
-				SetupDeploymentStrategyForPushPlan,
+				SetupDeploymentInformationForPushPlan,
 				SetupNoStartForPushPlan,
 				SetupNoWaitForPushPlan,
 				SetupTaskAppForPushPlan,

actor/v7pushaction/create_deployment_for_push_plan.go

@@ -8,10 +8,16 @@ import (
 func (actor Actor) CreateDeploymentForApplication(pushPlan PushPlan, eventStream chan<- *PushEvent, progressBar ProgressBar) (PushPlan, Warnings, error) {
 	eventStream <- &PushEvent{Plan: pushPlan, Event: StartingDeployment}
 
-	var dep resources.Deployment
-	dep.DropletGUID = pushPlan.DropletGUID
-	dep.Strategy = pushPlan.Strategy
-	dep.Relationships = resources.Relationships{constant.RelationshipTypeApplication: resources.Relationship{GUID: pushPlan.Application.GUID}}
+	dep := resources.Deployment{
+		Strategy:      pushPlan.Strategy,
+		DropletGUID:   pushPlan.DropletGUID,
+		Relationships: resources.Relationships{constant.RelationshipTypeApplication: resources.Relationship{GUID: pushPlan.Application.GUID}},
+	}
+
+	if pushPlan.MaxInFlight != 0 {
+		dep.Options = resources.DeploymentOpts{MaxInFlight: pushPlan.MaxInFlight}
+	}
+
 	deploymentGUID, warnings, err := actor.V7Actor.CreateDeployment(dep)
 
 	if err != nil {

actor/v7pushaction/create_deployment_for_push_plan_test.go

@@ -6,6 +6,7 @@ import (
 	"code.cloudfoundry.org/cli/actor/v7action"
 	. "code.cloudfoundry.org/cli/actor/v7pushaction"
 	"code.cloudfoundry.org/cli/actor/v7pushaction/v7pushactionfakes"
+	"code.cloudfoundry.org/cli/api/cloudcontroller/ccv3/constant"
 	"code.cloudfoundry.org/cli/resources"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -106,6 +107,51 @@ var _ = Describe("CreateDeploymentForApplication()", func() {
 				Expect(events).To(ConsistOf(StartingDeployment))
 			})
 		})
+
+		When("strategy is provided", func() {
+			BeforeEach(func() {
+				fakeV7Actor.PollStartForDeploymentCalls(func(_ resources.Application, _ string, _ bool, handleInstanceDetails func(string)) (warnings v7action.Warnings, err error) {
+					handleInstanceDetails("Instances starting...")
+					return nil, nil
+				})
+
+				fakeV7Actor.CreateDeploymentReturns(
+					"some-deployment-guid",
+					v7action.Warnings{"some-deployment-warning"},
+					nil,
+				)
+				paramPlan.Strategy = "rolling"
+				paramPlan.MaxInFlight = 10
+			})
+
+			It("waits for the app to start", func() {
+				Expect(fakeV7Actor.PollStartForDeploymentCallCount()).To(Equal(1))
+				givenApp, givenDeploymentGUID, noWait, _ := fakeV7Actor.PollStartForDeploymentArgsForCall(0)
+				Expect(givenApp).To(Equal(resources.Application{GUID: "some-app-guid"}))
+				Expect(givenDeploymentGUID).To(Equal("some-deployment-guid"))
+				Expect(noWait).To(Equal(false))
+				Expect(events).To(ConsistOf(StartingDeployment, InstanceDetails, WaitingForDeployment))
+				Expect(fakeV7Actor.CreateDeploymentCallCount()).To(Equal(1))
+				dep := fakeV7Actor.CreateDeploymentArgsForCall(0)
+				Expect(dep).To(Equal(resources.Deployment{
+					Strategy: "rolling",
+					Options:  resources.DeploymentOpts{MaxInFlight: 10},
+					Relationships: resources.Relationships{
+						constant.RelationshipTypeApplication: resources.Relationship{GUID: "some-app-guid"},
+					},
+				}))
+			})
+
+			It("returns errors and warnings", func() {
+				Expect(returnedPushPlan).To(Equal(paramPlan))
+				Expect(executeErr).NotTo(HaveOccurred())
+				Expect(warnings).To(ConsistOf("some-deployment-warning"))
+			})
+
+			It("records deployment events", func() {
+				Expect(events).To(ConsistOf(StartingDeployment, InstanceDetails, WaitingForDeployment))
+			})
+		})
 	})
 
 	Describe("waiting for app to start", func() {

actor/v7pushaction/push_plan.go

@@ -20,6 +20,7 @@ type PushPlan struct {
 	NoStart             bool
 	NoWait              bool
 	Strategy            constant.DeploymentStrategy
+	MaxInFlight         int
 	TaskTypeApplication bool
 
 	DockerImageCredentials v7action.DockerImageCredentials
@@ -47,6 +48,7 @@ type FlagOverrides struct {
 	HealthCheckType     constant.HealthCheckType
 	Instances           types.NullInt
 	Memory              string
+	MaxInFlight         *int
 	NoStart             bool
 	NoWait              bool
 	ProvidedAppPath     string

actor/v7pushaction/setup_deployment_information_for_push_plan.go

@@ -0,0 +1,13 @@
+package v7pushaction
+
+import "code.cloudfoundry.org/cli/api/cloudcontroller/ccv3/constant"
+
+func SetupDeploymentInformationForPushPlan(pushPlan PushPlan, overrides FlagOverrides) (PushPlan, error) {
+	pushPlan.Strategy = overrides.Strategy
+
+	if overrides.Strategy != constant.DeploymentStrategyDefault && overrides.MaxInFlight != nil {
+		pushPlan.MaxInFlight = *overrides.MaxInFlight
+	}
+
+	return pushPlan, nil
+}

actor/v7pushaction/setup_deployment_strategy_for_push_plan_test.go → actor/v7pushaction/setup_deployment_information_for_push_plan_test.go

@@ -9,7 +9,7 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-var _ = Describe("SetupDeploymentStrategyForPushPlan", func() {
+var _ = Describe("SetupDeploymentInformationForPushPlan", func() {
 	var (
 		pushPlan  PushPlan
 		overrides FlagOverrides
@@ -24,24 +24,47 @@ var _ = Describe("SetupDeploymentStrategyForPushPlan", func() {
 	})
 
 	JustBeforeEach(func() {
-		expectedPushPlan, executeErr = SetupDeploymentStrategyForPushPlan(pushPlan, overrides)
+		expectedPushPlan, executeErr = SetupDeploymentInformationForPushPlan(pushPlan, overrides)
 	})
 
 	When("flag overrides specifies strategy", func() {
 		BeforeEach(func() {
 			overrides.Strategy = "rolling"
+			maxInFlight := 5
+			overrides.MaxInFlight = &maxInFlight
 		})
 
 		It("sets the strategy on the push plan", func() {
 			Expect(executeErr).ToNot(HaveOccurred())
 			Expect(expectedPushPlan.Strategy).To(Equal(constant.DeploymentStrategyRolling))
 		})
+
+		It("sets the max in flight on the push plan", func() {
+			Expect(executeErr).ToNot(HaveOccurred())
+			Expect(expectedPushPlan.MaxInFlight).To(Equal(5))
+		})
 	})
 
 	When("flag overrides does not specify strategy", func() {
+		BeforeEach(func() {
+			maxInFlight := 10
+			overrides.MaxInFlight = &maxInFlight
+		})
 		It("leaves the strategy as its default value on the push plan", func() {
 			Expect(executeErr).ToNot(HaveOccurred())
 			Expect(expectedPushPlan.Strategy).To(Equal(constant.DeploymentStrategyDefault))
 		})
+
+		It("does not set MaxInFlight", func() {
+			Expect(executeErr).ToNot(HaveOccurred())
+			Expect(expectedPushPlan.MaxInFlight).To(Equal(0))
+		})
+	})
+
+	When("flag not provided", func() {
+		It("does not set MaxInFlight", func() {
+			Expect(executeErr).ToNot(HaveOccurred())
+			Expect(expectedPushPlan.MaxInFlight).To(Equal(0))
+		})
 	})
 })

command/translatableerror/convert_to_translatable_error.go

@@ -188,6 +188,13 @@ func ConvertToTranslatableError(err error) error {
 			return RunTaskError{Message: "App is not staged."}
 		}
 
+		if strings.Contains(e.Message, "Unknown field(s): 'options'") {
+			return MinimumCFAPIVersionNotMetError{
+				Command:        "'--max-in-flight' flag",
+				MinimumVersion: "3.173.0",
+			}
+		}
+
 	// JSON Errors
 	case *json.SyntaxError:
 		return JSONSyntaxError{Err: e}

command/v7/copy_source_command.go

@@ -17,6 +17,7 @@ type CopySourceCommand struct {
 	RequiredArgs        flag.CopySourceArgs     `positional-args:"yes"`
 	usage               interface{}             `usage:"CF_NAME copy-source SOURCE_APP DESTINATION_APP [-s TARGET_SPACE [-o TARGET_ORG]] [--no-restart] [--strategy STRATEGY] [--no-wait]"`
 	Strategy            flag.DeploymentStrategy `long:"strategy" description:"Deployment strategy can be canary, rolling or null"`
+	MaxInFlight         *int                    `long:"max-in-flight" description:"Defines the maximum number of instances that will be actively being started. Only applies when --strategy flag is specified."`
 	NoWait              bool                    `long:"no-wait" description:"Exit when the first instance of the web process is healthy"`
 	NoRestart           bool                    `long:"no-restart" description:"Do not restage the destination application"`
 	Organization        string                  `short:"o" long:"organization" description:"Org that contains the destination application"`
@@ -48,6 +49,14 @@ func (cmd *CopySourceCommand) ValidateFlags() error {
 		}
 	}
 
+	if cmd.Strategy.Name == constant.DeploymentStrategyDefault && cmd.MaxInFlight != nil {
+		return translatableerror.RequiredFlagsError{Arg1: "--max-in-flight", Arg2: "--strategy"}
+	}
+
+	if cmd.Strategy.Name != constant.DeploymentStrategyDefault && cmd.MaxInFlight != nil && *cmd.MaxInFlight < 1 {
+		return translatableerror.IncorrectUsageError{Message: "--max-in-flight must be greater than or equal to 1"}
+	}
+
 	return nil
 }
 
@@ -160,10 +169,15 @@ func (cmd CopySourceCommand) Execute(args []string) error {
 		cmd.UI.DisplayNewline()
 
 		opts := shared.AppStartOpts{
-			Strategy:  cmd.Strategy.Name,
-			NoWait:    cmd.NoWait,
 			AppAction: constant.ApplicationRestarting,
+			NoWait:    cmd.NoWait,
+			Strategy:  cmd.Strategy.Name,
 		}
+
+		if cmd.MaxInFlight != nil {
+			opts.MaxInFlight = *cmd.MaxInFlight
+		}
+
 		err = cmd.Stager.StageAndStart(targetApp, targetSpace, targetOrg, pkg.GUID, opts)
 		if err != nil {
 			return mapErr(cmd.Config, targetApp.Name, err)

command/v7/copy_source_command_test.go

@@ -125,49 +125,6 @@ var _ = Describe("copy-source Command", func() {
 		})
 	})
 
-	When("the target organization is specified but the targeted space isn't", func() {
-		BeforeEach(func() {
-			cmd.Organization = "some-other-organization"
-		})
-
-		It("returns an error", func() {
-			Expect(executeErr).To(MatchError(translatableerror.RequiredFlagsError{
-				Arg1: "--organization, -o",
-				Arg2: "--space, -s",
-			}))
-		})
-	})
-
-	When("the no restart and strategy flags are both provided", func() {
-		BeforeEach(func() {
-			cmd.NoRestart = true
-			cmd.Strategy = flag.DeploymentStrategy{Name: constant.DeploymentStrategyRolling}
-		})
-
-		It("returns an error", func() {
-			Expect(executeErr).To(MatchError(translatableerror.ArgumentCombinationError{
-				Args: []string{
-					"--no-restart", "--strategy",
-				},
-			}))
-		})
-	})
-
-	When("the no restart and no wait flags are both provided", func() {
-		BeforeEach(func() {
-			cmd.NoRestart = true
-			cmd.NoWait = true
-		})
-
-		It("returns an error", func() {
-			Expect(executeErr).To(MatchError(translatableerror.ArgumentCombinationError{
-				Args: []string{
-					"--no-restart", "--no-wait",
-				},
-			}))
-		})
-	})
-
 	When("a target org and space is provided", func() {
 		BeforeEach(func() {
 			cmd.Organization = "destination-org"
@@ -329,6 +286,8 @@ var _ = Describe("copy-source Command", func() {
 			cmd.Strategy = flag.DeploymentStrategy{
 				Name: constant.DeploymentStrategyRolling,
 			}
+			maxInFlight := 5
+			cmd.MaxInFlight = &maxInFlight
 		})
 
 		It("stages and starts the app with the appropriate strategy", func() {
@@ -338,9 +297,10 @@ var _ = Describe("copy-source Command", func() {
 			Expect(spaceForApp).To(Equal(configv3.Space{Name: "some-space", GUID: "some-space-guid"}))
 			Expect(orgForApp).To(Equal(configv3.Organization{Name: "some-org"}))
 			Expect(pkgGUID).To(Equal("target-package-guid"))
-			Expect(opts.Strategy).To(Equal(constant.DeploymentStrategyRolling))
-			Expect(opts.NoWait).To(Equal(false))
 			Expect(opts.AppAction).To(Equal(constant.ApplicationRestarting))
+			Expect(opts.MaxInFlight).To(Equal(5))
+			Expect(opts.NoWait).To(Equal(false))
+			Expect(opts.Strategy).To(Equal(constant.DeploymentStrategyRolling))
 		})
 	})
 
@@ -349,6 +309,8 @@ var _ = Describe("copy-source Command", func() {
 			cmd.Strategy = flag.DeploymentStrategy{
 				Name: constant.DeploymentStrategyCanary,
 			}
+			maxInFlight := 1
+			cmd.MaxInFlight = &maxInFlight
 		})
 
 		It("stages and starts the app with the appropriate strategy", func() {
@@ -417,4 +379,66 @@ var _ = Describe("copy-source Command", func() {
 	It("succeeds", func() {
 		Expect(executeErr).To(Not(HaveOccurred()))
 	})
+
+	DescribeTable("ValidateFlags returns an error",
+		func(setup func(), expectedErr error) {
+			setup()
+			err := cmd.ValidateFlags()
+			if expectedErr == nil {
+				Expect(err).To(BeNil())
+			} else {
+				Expect(err).To(MatchError(expectedErr))
+			}
+		},
+		Entry("the target organization is specified but the targeted space isn't",
+			func() {
+				cmd.Organization = "some-other-organization"
+			},
+			translatableerror.RequiredFlagsError{
+				Arg1: "--organization, -o",
+				Arg2: "--space, -s",
+			}),
+
+		Entry("the no restart and strategy flags are both provided",
+			func() {
+				cmd.NoRestart = true
+				cmd.Strategy = flag.DeploymentStrategy{Name: constant.DeploymentStrategyRolling}
+			},
+			translatableerror.ArgumentCombinationError{
+				Args: []string{
+					"--no-restart", "--strategy",
+				},
+			}),
+
+		Entry("the no restart and no wait flags are both provided",
+			func() {
+				cmd.NoRestart = true
+				cmd.NoWait = true
+			},
+			translatableerror.ArgumentCombinationError{
+				Args: []string{
+					"--no-restart", "--no-wait",
+				},
+			}),
+
+		Entry("max-in-flight is passed without strategy",
+			func() {
+				maxInFlight := 5
+				cmd.MaxInFlight = &maxInFlight
+			},
+			translatableerror.RequiredFlagsError{
+				Arg1: "--max-in-flight",
+				Arg2: "--strategy",
+			}),
+
+		Entry("max-in-flight is smaller than 1",
+			func() {
+				cmd.Strategy = flag.DeploymentStrategy{Name: constant.DeploymentStrategyRolling}
+				maxInFlight := 0
+				cmd.MaxInFlight = &maxInFlight
+			},
+			translatableerror.IncorrectUsageError{
+				Message: "--max-in-flight must be greater than or equal to 1",
+			}),
+	)
 })