Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[main] Max in flight flag #3096

Merged
merged 1 commit into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading