Change the way we check for presence of the --max-in-flight flag

- Provide a better error when talking to older versions of CAPI
- Address some comments in the PR review

Signed-off-by: João Pereira <[email protected]>

actor/v7pushaction/create_deployment_for_push_plan.go

@@ -9,14 +9,15 @@ func (actor Actor) CreateDeploymentForApplication(pushPlan PushPlan, eventStream
 	eventStream <- &PushEvent{Plan: pushPlan, Event: StartingDeployment}
 
 	dep := resources.Deployment{
-		Strategy: pushPlan.Strategy,
-		Options: resources.DeploymentOpts{
-			MaxInFlight: pushPlan.MaxInFlight,
-		},
+		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/push_plan.go

@@ -48,7 +48,7 @@ type FlagOverrides struct {
 	HealthCheckType     constant.HealthCheckType
 	Instances           types.NullInt
 	Memory              string
-	MaxInFlight         int
+	MaxInFlight         *int
 	NoStart             bool
 	NoWait              bool
 	ProvidedAppPath     string

actor/v7pushaction/setup_deployment_information_for_push_plan.go

@@ -5,8 +5,8 @@ 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 {
-		pushPlan.MaxInFlight = overrides.MaxInFlight
+	if overrides.Strategy != constant.DeploymentStrategyDefault && overrides.MaxInFlight != nil {
+		pushPlan.MaxInFlight = *overrides.MaxInFlight
 	}
 
 	return pushPlan, nil

actor/v7pushaction/setup_deployment_information_for_push_plan_test.go

@@ -30,7 +30,8 @@ var _ = Describe("SetupDeploymentInformationForPushPlan", func() {
 	When("flag overrides specifies strategy", func() {
 		BeforeEach(func() {
 			overrides.Strategy = "rolling"
-			overrides.MaxInFlight = 5
+			maxInFlight := 5
+			overrides.MaxInFlight = &maxInFlight
 		})
 
 		It("sets the strategy on the push plan", func() {
@@ -46,7 +47,8 @@ var _ = Describe("SetupDeploymentInformationForPushPlan", func() {
 
 	When("flag overrides does not specify strategy", func() {
 		BeforeEach(func() {
-			overrides.MaxInFlight = 10
+			maxInFlight := 10
+			overrides.MaxInFlight = &maxInFlight
 		})
 		It("leaves the strategy as its default value on the push plan", func() {
 			Expect(executeErr).ToNot(HaveOccurred())
@@ -58,4 +60,11 @@ var _ = Describe("SetupDeploymentInformationForPushPlan", func() {
 			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,7 +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" default:"-1" description:"Defines the maximum number of instances that will be actively being started. Only applies when --strategy flag is specified."`
+	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"`
@@ -49,11 +49,11 @@ func (cmd *CopySourceCommand) ValidateFlags() error {
 		}
 	}
 
-	if cmd.Strategy.Name == constant.DeploymentStrategyDefault && cmd.MaxInFlight > 0 {
+	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 < -1 || cmd.MaxInFlight == 0) {
+	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"}
 	}
 
@@ -169,11 +169,15 @@ func (cmd CopySourceCommand) Execute(args []string) error {
 		cmd.UI.DisplayNewline()
 
 		opts := shared.AppStartOpts{
-			AppAction:   constant.ApplicationRestarting,
-			MaxInFlight: cmd.MaxInFlight,
-			NoWait:      cmd.NoWait,
-			Strategy:    cmd.Strategy.Name,
+			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

@@ -286,7 +286,8 @@ var _ = Describe("copy-source Command", func() {
 			cmd.Strategy = flag.DeploymentStrategy{
 				Name: constant.DeploymentStrategyRolling,
 			}
-			cmd.MaxInFlight = 5
+			maxInFlight := 5
+			cmd.MaxInFlight = &maxInFlight
 		})
 
 		It("stages and starts the app with the appropriate strategy", func() {
@@ -308,7 +309,8 @@ var _ = Describe("copy-source Command", func() {
 			cmd.Strategy = flag.DeploymentStrategy{
 				Name: constant.DeploymentStrategyCanary,
 			}
-			cmd.MaxInFlight = 1
+			maxInFlight := 1
+			cmd.MaxInFlight = &maxInFlight
 		})
 
 		It("stages and starts the app with the appropriate strategy", func() {
@@ -421,7 +423,8 @@ var _ = Describe("copy-source Command", func() {
 
 		Entry("max-in-flight is passed without strategy",
 			func() {
-				cmd.MaxInFlight = 10
+				maxInFlight := 5
+				cmd.MaxInFlight = &maxInFlight
 			},
 			translatableerror.RequiredFlagsError{
 				Arg1: "--max-in-flight",
@@ -431,7 +434,8 @@ var _ = Describe("copy-source Command", func() {
 		Entry("max-in-flight is smaller than 1",
 			func() {
 				cmd.Strategy = flag.DeploymentStrategy{Name: constant.DeploymentStrategyRolling}
-				cmd.MaxInFlight = 0
+				maxInFlight := 0
+				cmd.MaxInFlight = &maxInFlight
 			},
 			translatableerror.IncorrectUsageError{
 				Message: "--max-in-flight must be greater than or equal to 1",

command/v7/push_command.go

@@ -90,7 +90,7 @@ type PushCommand struct {
 	Instances               flag.Instances                      `long:"instances" short:"i" description:"Number of instances"`
 	LogRateLimit            string                              `long:"log-rate-limit" short:"l" description:"Log rate limit per second, in bytes (e.g. 128B, 4K, 1M). -l=-1 represents unlimited"`
 	PathToManifest          flag.ManifestPathWithExistenceCheck `long:"manifest" short:"f" description:"Path to manifest"`
-	MaxInFlight             int                                 `long:"max-in-flight" default:"-1" description:"Defines the maximum number of instances that will be actively being started. Only applies when --strategy flag is specified."`
+	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."`
 	Memory                  string                              `long:"memory" short:"m" description:"Memory limit (e.g. 256M, 1024M, 1G)"`
 	NoManifest              bool                                `long:"no-manifest" description:"Ignore manifest file"`
 	NoRoute                 bool                                `long:"no-route" description:"Do not map a route to this app"`
@@ -488,9 +488,9 @@ func (cmd PushCommand) ValidateFlags() error {
 	case !cmd.validBuildpacks():
 		return translatableerror.InvalidBuildpacksError{}
 
-	case cmd.Strategy.Name == constant.DeploymentStrategyDefault && cmd.MaxInFlight > 0:
+	case cmd.Strategy.Name == constant.DeploymentStrategyDefault && cmd.MaxInFlight != nil:
 		return translatableerror.RequiredFlagsError{Arg1: "--max-in-flight", Arg2: "--strategy"}
-	case cmd.Strategy.Name != constant.DeploymentStrategyDefault && (cmd.MaxInFlight < -1 || cmd.MaxInFlight == 0):
+	case 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"}
 	}
 

command/v7/push_command_test.go

@@ -1072,7 +1072,8 @@ var _ = Describe("push Command", func() {
 			cmd.RandomRoute = false
 			cmd.NoStart = true
 			cmd.NoWait = true
-			cmd.MaxInFlight = 1
+			maxInFlight := 1
+			cmd.MaxInFlight = &maxInFlight
 			cmd.Strategy = flag.DeploymentStrategy{Name: constant.DeploymentStrategyRolling}
 			cmd.Instances = flag.Instances{NullInt: types.NullInt{Value: 10, IsSet: true}}
 			cmd.PathToManifest = "/manifest/path"
@@ -1109,7 +1110,7 @@ var _ = Describe("push Command", func() {
 			Expect(overrides.Vars).To(Equal([]template.VarKV{{Name: "key", Value: "val"}}))
 			Expect(overrides.Task).To(BeTrue())
 			Expect(overrides.LogRateLimit).To(Equal("512M"))
-			Expect(overrides.MaxInFlight).To(Equal(1))
+			Expect(*overrides.MaxInFlight).To(Equal(1))
 		})
 
 		When("a docker image is provided", func() {
@@ -1295,7 +1296,8 @@ var _ = Describe("push Command", func() {
 
 		Entry("max-in-flight is passed without strategy",
 			func() {
-				cmd.MaxInFlight = 10
+				maxInFlight := 10
+				cmd.MaxInFlight = &maxInFlight
 			},
 			translatableerror.RequiredFlagsError{
 				Arg1: "--max-in-flight",
@@ -1305,7 +1307,8 @@ var _ = Describe("push Command", func() {
 		Entry("max-in-flight is smaller than 1",
 			func() {
 				cmd.Strategy = flag.DeploymentStrategy{Name: constant.DeploymentStrategyRolling}
-				cmd.MaxInFlight = 0
+				maxInFlight := 0
+				cmd.MaxInFlight = &maxInFlight
 			},
 			translatableerror.IncorrectUsageError{
 				Message: "--max-in-flight must be greater than or equal to 1",

command/v7/restage_command.go

@@ -16,7 +16,7 @@ type RestageCommand struct {
 
 	RequiredArgs        flag.AppName            `positional-args:"yes"`
 	Strategy            flag.DeploymentStrategy `long:"strategy" description:"Deployment strategy can be canary, rolling or null."`
-	MaxInFlight         int                     `long:"max-in-flight" default:"-1" description:"Defines the maximum number of instances that will be actively being restaged. Only applies when --strategy flag is specified."`
+	MaxInFlight         *int                    `long:"max-in-flight" description:"Defines the maximum number of instances that will be actively being restaged. Only applies when --strategy flag is specified."`
 	NoWait              bool                    `long:"no-wait" description:"Exit when the first instance of the web process is healthy"`
 	usage               interface{}             `usage:"CF_NAME restage APP_NAME\n\n   This command will cause downtime unless you use '--strategy' flag.\n\nEXAMPLES:\n   CF_NAME restage APP_NAME\n   CF_NAME restage APP_NAME --strategy rolling\n   CF_NAME restage APP_NAME --strategy canary --no-wait"`
 	relatedCommands     interface{}             `related_commands:"restart"`
@@ -84,11 +84,15 @@ func (cmd RestageCommand) Execute(args []string) error {
 	}
 
 	opts := shared.AppStartOpts{
-		AppAction:   constant.ApplicationRestarting,
-		MaxInFlight: cmd.MaxInFlight,
-		NoWait:      cmd.NoWait,
-		Strategy:    cmd.Strategy.Name,
+		AppAction: constant.ApplicationRestarting,
+		NoWait:    cmd.NoWait,
+		Strategy:  cmd.Strategy.Name,
 	}
+
+	if cmd.MaxInFlight != nil {
+		opts.MaxInFlight = *cmd.MaxInFlight
+	}
+
 	err = cmd.Stager.StageAndStart(app, cmd.Config.TargetedSpace(), cmd.Config.TargetedOrganization(), pkg.GUID, opts)
 	if err != nil {
 		return mapErr(cmd.Config, cmd.RequiredArgs.AppName, err)
@@ -99,9 +103,9 @@ func (cmd RestageCommand) Execute(args []string) error {
 
 func (cmd RestageCommand) ValidateFlags() error {
 	switch {
-	case cmd.Strategy.Name == constant.DeploymentStrategyDefault && cmd.MaxInFlight > 0:
+	case cmd.Strategy.Name == constant.DeploymentStrategyDefault && cmd.MaxInFlight != nil:
 		return translatableerror.RequiredFlagsError{Arg1: "--max-in-flight", Arg2: "--strategy"}
-	case cmd.Strategy.Name != constant.DeploymentStrategyDefault && (cmd.MaxInFlight < -1 || cmd.MaxInFlight == 0):
+	case 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"}
 	}
 

command/v7/restage_command_test.go

@@ -76,7 +76,8 @@ var _ = Describe("restage Command", func() {
 		)
 
 		cmd.Strategy = flag.DeploymentStrategy{Name: constant.DeploymentStrategyRolling}
-		cmd.MaxInFlight = 4
+		maxInFlight := 4
+		cmd.MaxInFlight = &maxInFlight
 	})
 
 	JustBeforeEach(func() {
@@ -112,7 +113,7 @@ var _ = Describe("restage Command", func() {
 	When("No strategy flag is given", func() {
 		BeforeEach(func() {
 			cmd.Strategy.Name = constant.DeploymentStrategyDefault
-			cmd.MaxInFlight = 0
+			cmd.MaxInFlight = nil
 		})
 		It("warns that there will be app downtime", func() {
 			Expect(testUI.Err).To(Say("This action will cause app downtime."))
@@ -208,7 +209,8 @@ var _ = Describe("restage Command", func() {
 		Entry("max-in-flight is passed without strategy",
 			func() {
 				cmd.Strategy = flag.DeploymentStrategy{Name: constant.DeploymentStrategyDefault}
-				cmd.MaxInFlight = 10
+				maxInFlight := 10
+				cmd.MaxInFlight = &maxInFlight
 			},
 			translatableerror.RequiredFlagsError{
 				Arg1: "--max-in-flight",
@@ -218,7 +220,8 @@ var _ = Describe("restage Command", func() {
 		Entry("max-in-flight is smaller than 1",
 			func() {
 				cmd.Strategy = flag.DeploymentStrategy{Name: constant.DeploymentStrategyRolling}
-				cmd.MaxInFlight = 0
+				maxInFlight := 0
+				cmd.MaxInFlight = &maxInFlight
 			},
 			translatableerror.IncorrectUsageError{
 				Message: "--max-in-flight must be greater than or equal to 1",

command/v7/restart_command.go

@@ -13,7 +13,7 @@ import (
 type RestartCommand struct {
 	BaseCommand
 
-	MaxInFlight         int                     `long:"max-in-flight" default:"-1" description:"Defines the maximum number of instances that will be actively restarted at any given time. Only applies when --strategy flag is specified."`
+	MaxInFlight         *int                    `long:"max-in-flight" description:"Defines the maximum number of instances that will be actively restarted at any given time. Only applies when --strategy flag is specified."`
 	RequiredArgs        flag.AppName            `positional-args:"yes"`
 	Strategy            flag.DeploymentStrategy `long:"strategy" description:"Deployment strategy can be canary, rolling or null."`
 	NoWait              bool                    `long:"no-wait" description:"Exit when the first instance of the web process is healthy"`
@@ -80,11 +80,15 @@ func (cmd RestartCommand) Execute(args []string) error {
 	}
 
 	opts := shared.AppStartOpts{
-		Strategy:    cmd.Strategy.Name,
-		NoWait:      cmd.NoWait,
-		MaxInFlight: cmd.MaxInFlight,
-		AppAction:   constant.ApplicationRestarting,
+		Strategy:  cmd.Strategy.Name,
+		NoWait:    cmd.NoWait,
+		AppAction: constant.ApplicationRestarting,
 	}
+
+	if cmd.MaxInFlight != nil {
+		opts.MaxInFlight = *cmd.MaxInFlight
+	}
+
 	if packageGUID != "" {
 		err = cmd.Stager.StageAndStart(app, cmd.Config.TargetedSpace(), cmd.Config.TargetedOrganization(), packageGUID, opts)
 		if err != nil {
@@ -102,9 +106,9 @@ func (cmd RestartCommand) Execute(args []string) error {
 
 func (cmd RestartCommand) ValidateFlags() error {
 	switch true {
-	case cmd.Strategy.Name == constant.DeploymentStrategyDefault && cmd.MaxInFlight > 0:
+	case cmd.Strategy.Name == constant.DeploymentStrategyDefault && cmd.MaxInFlight != nil:
 		return translatableerror.RequiredFlagsError{Arg1: "--max-in-flight", Arg2: "--strategy"}
-	case cmd.Strategy.Name != constant.DeploymentStrategyDefault && (cmd.MaxInFlight < -1 || cmd.MaxInFlight == 0):
+	case 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"}
 	}
 

command/v7/restart_command_test.go

@@ -201,7 +201,8 @@ var _ = Describe("restart Command", func() {
 
 		Entry("max-in-flight is passed without strategy",
 			func() {
-				cmd.MaxInFlight = 10
+				maxInFlight := 10
+				cmd.MaxInFlight = &maxInFlight
 			},
 			translatableerror.RequiredFlagsError{
 				Arg1: "--max-in-flight",
@@ -211,7 +212,8 @@ var _ = Describe("restart Command", func() {
 		Entry("max-in-flight is smaller than 1",
 			func() {
 				cmd.Strategy = flag.DeploymentStrategy{Name: constant.DeploymentStrategyRolling}
-				cmd.MaxInFlight = 0
+				maxInFlight := 0
+				cmd.MaxInFlight = &maxInFlight
 			},
 			translatableerror.IncorrectUsageError{
 				Message: "--max-in-flight must be greater than or equal to 1",