Skip to content

Commit

Permalink
Max in flight flag (cloudfoundry#3085)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
joaopapereira committed Aug 9, 2024
1 parent f0585b1 commit 9e424fa
Show file tree
Hide file tree
Showing 29 changed files with 471 additions and 98 deletions.
2 changes: 1 addition & 1 deletion actor/v7pushaction/actor.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func NewActor(v3Actor V7Actor, sharedActor SharedActor) *Actor {
SetDefaultBitsPathForPushPlan,
SetupDropletPathForPushPlan,
actor.SetupAllResourcesForPushPlan,
SetupDeploymentStrategyForPushPlan,
SetupDeploymentInformationForPushPlan,
SetupNoStartForPushPlan,
SetupNoWaitForPushPlan,
SetupTaskAppForPushPlan,
Expand Down
2 changes: 1 addition & 1 deletion actor/v7pushaction/actor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ var _ = Describe("Actor", func() {
SetDefaultBitsPathForPushPlan,
SetupDropletPathForPushPlan,
actor.SetupAllResourcesForPushPlan,
SetupDeploymentStrategyForPushPlan,
SetupDeploymentInformationForPushPlan,
SetupNoStartForPushPlan,
SetupNoWaitForPushPlan,
SetupTaskAppForPushPlan,
Expand Down
14 changes: 10 additions & 4 deletions actor/v7pushaction/create_deployment_for_push_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
46 changes: 46 additions & 0 deletions actor/v7pushaction/create_deployment_for_push_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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() {
Expand Down
2 changes: 2 additions & 0 deletions actor/v7pushaction/push_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type PushPlan struct {
NoStart bool
NoWait bool
Strategy constant.DeploymentStrategy
MaxInFlight int
TaskTypeApplication bool

DockerImageCredentials v7action.DockerImageCredentials
Expand Down Expand Up @@ -47,6 +48,7 @@ type FlagOverrides struct {
HealthCheckType constant.HealthCheckType
Instances types.NullInt
Memory string
MaxInFlight *int
NoStart bool
NoWait bool
ProvidedAppPath string
Expand Down
13 changes: 13 additions & 0 deletions actor/v7pushaction/setup_deployment_information_for_push_plan.go
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
. "github.com/onsi/gomega"
)

var _ = Describe("SetupDeploymentStrategyForPushPlan", func() {
var _ = Describe("SetupDeploymentInformationForPushPlan", func() {
var (
pushPlan PushPlan
overrides FlagOverrides
Expand All @@ -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))
})
})
})
7 changes: 0 additions & 7 deletions actor/v7pushaction/setup_deployment_strategy_for_push_plan.go

This file was deleted.

7 changes: 7 additions & 0 deletions command/translatableerror/convert_to_translatable_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
18 changes: 16 additions & 2 deletions command/v7/copy_source_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
Expand Down
114 changes: 69 additions & 45 deletions command/v7/copy_source_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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() {
Expand All @@ -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))
})
})

Expand All @@ -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() {
Expand Down Expand Up @@ -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",
}),
)
})
Loading

0 comments on commit 9e424fa

Please sign in to comment.