Skip to content

Commit

Permalink
Allow users to push apps using a weighted canary deployment
Browse files Browse the repository at this point in the history
Signed-off-by: João Pereira <joao.pereira@broadcom.com>
  • Loading branch information
joaopapereira committed Jan 30, 2025
1 parent 9249609 commit d6001ca
Show file tree
Hide file tree
Showing 10 changed files with 174 additions and 10 deletions.
9 changes: 8 additions & 1 deletion actor/v7pushaction/create_deployment_for_push_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,17 @@ func (actor Actor) CreateDeploymentForApplication(pushPlan PushPlan, eventStream
Relationships: resources.Relationships{constant.RelationshipTypeApplication: resources.Relationship{GUID: pushPlan.Application.GUID}},
}

if pushPlan.MaxInFlight != 0 {
if pushPlan.MaxInFlight > 0 {
dep.Options = resources.DeploymentOpts{MaxInFlight: pushPlan.MaxInFlight}
}

if len(pushPlan.InstanceSteps) > 0 {
dep.Options.CanaryDeploymentOptions = resources.CanaryDeploymentOptions{Steps: []resources.CanaryStep{}}
for _, w := range pushPlan.InstanceSteps {
dep.Options.CanaryDeploymentOptions.Steps = append(dep.Options.CanaryDeploymentOptions.Steps, resources.CanaryStep{InstanceWeight: w})
}
}

deploymentGUID, warnings, err := actor.V7Actor.CreateDeployment(dep)

if err != nil {
Expand Down
38 changes: 38 additions & 0 deletions actor/v7pushaction/create_deployment_for_push_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,44 @@ var _ = Describe("CreateDeploymentForApplication()", func() {
Expect(events).To(ConsistOf(StartingDeployment, InstanceDetails, WaitingForDeployment))
})
})

When("canary weights are 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 = "canary"
paramPlan.InstanceSteps = []int64{1, 2, 3, 4}
})

It("creates the correct deployment from the object", func() {
Expect(fakeV7Actor.CreateDeploymentCallCount()).To(Equal(1))
dep := fakeV7Actor.CreateDeploymentArgsForCall(0)
Expect(dep).To(Equal(resources.Deployment{
Strategy: "canary",
Options: resources.DeploymentOpts{
CanaryDeploymentOptions: resources.CanaryDeploymentOptions{
Steps: []resources.CanaryStep{
{InstanceWeight: 1},
{InstanceWeight: 2},
{InstanceWeight: 3},
{InstanceWeight: 4},
},
},
},
Relationships: resources.Relationships{
constant.RelationshipTypeApplication: resources.Relationship{GUID: "some-app-guid"},
},
}))
})
})
})

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 @@ -22,6 +22,7 @@ type PushPlan struct {
Strategy constant.DeploymentStrategy
MaxInFlight int
TaskTypeApplication bool
InstanceSteps []int64

DockerImageCredentials v7action.DockerImageCredentials

Expand All @@ -48,6 +49,7 @@ type FlagOverrides struct {
HealthCheckTimeout int64
HealthCheckType constant.HealthCheckType
Instances types.NullInt
InstanceSteps []int64
Memory string
MaxInFlight *int
NoStart bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,9 @@ func SetupDeploymentInformationForPushPlan(pushPlan PushPlan, overrides FlagOver
pushPlan.MaxInFlight = *overrides.MaxInFlight
}

if overrides.Strategy == constant.DeploymentStrategyCanary && overrides.InstanceSteps != nil {
pushPlan.InstanceSteps = overrides.InstanceSteps
}

return pushPlan, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ var _ = Describe("SetupDeploymentInformationForPushPlan", func() {
overrides.Strategy = "rolling"
maxInFlight := 5
overrides.MaxInFlight = &maxInFlight
overrides.InstanceSteps = []int64{1, 2, 3, 4}
})

It("sets the strategy on the push plan", func() {
Expand All @@ -43,12 +44,35 @@ var _ = Describe("SetupDeploymentInformationForPushPlan", func() {
Expect(executeErr).ToNot(HaveOccurred())
Expect(expectedPushPlan.MaxInFlight).To(Equal(5))
})

When("strategy is rolling", func() {
BeforeEach(func() {
overrides.Strategy = "rolling"
})

It("does not set the canary steps", func() {
Expect(executeErr).ToNot(HaveOccurred())
Expect(expectedPushPlan.InstanceSteps).To(BeEmpty())
})
})

When("strategy is canary", func() {
BeforeEach(func() {
overrides.Strategy = "canary"
})

It("does set the canary steps", func() {
Expect(executeErr).ToNot(HaveOccurred())
Expect(expectedPushPlan.InstanceSteps).To(ContainElements(int64(1), int64(2), int64(3), int64(4)))
})
})
})

When("flag overrides does not specify strategy", func() {
BeforeEach(func() {
maxInFlight := 10
overrides.MaxInFlight = &maxInFlight
overrides.InstanceSteps = []int64{1, 2, 3, 4}
})
It("leaves the strategy as its default value on the push plan", func() {
Expect(executeErr).ToNot(HaveOccurred())
Expand All @@ -59,12 +83,22 @@ var _ = Describe("SetupDeploymentInformationForPushPlan", func() {
Expect(executeErr).ToNot(HaveOccurred())
Expect(expectedPushPlan.MaxInFlight).To(Equal(0))
})

It("does not set canary steps", func() {
Expect(executeErr).ToNot(HaveOccurred())
Expect(expectedPushPlan.InstanceSteps).To(BeEmpty())
})
})

When("flag not provided", func() {
It("does not set MaxInFlight", func() {
Expect(executeErr).ToNot(HaveOccurred())
Expect(expectedPushPlan.MaxInFlight).To(Equal(0))
})

It("does not set the canary steps", func() {
Expect(executeErr).ToNot(HaveOccurred())
Expect(expectedPushPlan.InstanceSteps).To(BeEmpty())
})
})
})
6 changes: 3 additions & 3 deletions api/cloudcontroller/ccv3/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ var _ = Describe("Deployment", func() {
dep.Strategy = constant.DeploymentStrategyCanary
dep.RevisionGUID = revisionGUID
dep.Relationships = resources.Relationships{constant.RelationshipTypeApplication: resources.Relationship{GUID: "some-app-guid"}}
dep.Options.CanaryDeploymentOptions = resources.CanaryDeploymentOptions{Steps: []resources.CanaryStep{{InstanceWeight: 1}, {InstanceWeight: 2}}}
deploymentGUID, warnings, executeErr = client.CreateApplicationDeployment(dep)
})

Expand All @@ -277,7 +278,7 @@ var _ = Describe("Deployment", func() {
server.AppendHandlers(
CombineHandlers(
VerifyRequest(http.MethodPost, "/v3/deployments"),
VerifyJSON(`{"revision":{ "guid":"some-revision-guid" }, "strategy": "canary", "relationships":{"app":{"data":{"guid":"some-app-guid"}}}}`),
VerifyJSON(`{"revision":{ "guid":"some-revision-guid" }, "strategy": "canary", "relationships":{"app":{"data":{"guid":"some-app-guid"}}},"options":{"canary": {"steps": [{"instance_weight": 1}, {"instance_weight": 2}]}}}`),
RespondWith(http.StatusAccepted, response, http.Header{"X-Cf-Warnings": {"warning"}}),
),
)
Expand Down Expand Up @@ -306,14 +307,13 @@ var _ = Describe("Deployment", func() {
server.AppendHandlers(
CombineHandlers(
VerifyRequest(http.MethodPost, "/v3/deployments"),
VerifyJSON(`{"revision":{ "guid":"some-revision-guid" }, "strategy": "canary", "relationships":{"app":{"data":{"guid":"some-app-guid"}}}}`),
VerifyJSON(`{"revision":{ "guid":"some-revision-guid" }, "strategy": "canary","options":{"canary": {"steps": [{"instance_weight": 1}, {"instance_weight": 2}]}}, "relationships":{"app":{"data":{"guid":"some-app-guid"}}}}`),
RespondWith(http.StatusTeapot, response, http.Header{}),
),
)
})

It("returns an error", func() {
fmt.Printf("executeErr: %v\n", executeErr)
Expect(executeErr).To(HaveOccurred())
Expect(executeErr).To(MatchError(ccerror.V3UnexpectedResponseError{
ResponseCode: http.StatusTeapot,
Expand Down
28 changes: 28 additions & 0 deletions command/v7/push_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"os"
"strconv"
"strings"

"github.com/cloudfoundry/bosh-cli/director/template"
Expand Down Expand Up @@ -88,6 +89,7 @@ type PushCommand struct {
HealthCheckHTTPEndpoint string `long:"endpoint" description:"Valid path on the app for an HTTP health check. Only used when specifying --health-check-type=http"`
HealthCheckType flag.HealthCheckType `long:"health-check-type" short:"u" description:"Application health check type. Defaults to 'port'. 'http' requires a valid endpoint, for example, '/health'."`
Instances flag.Instances `long:"instances" short:"i" description:"Number of instances"`
InstanceSteps string `long:"instance-steps" description:"An array of percentage steps to deploy when using deployment strategy canary. (e.g. 20,40,60)"`
Lifecycle constant.AppLifecycleType `long:"lifecycle" description:"App lifecycle type to stage and run the app" default:""`
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"`
Expand Down Expand Up @@ -343,6 +345,17 @@ func (cmd PushCommand) GetFlagOverrides() (v7pushaction.FlagOverrides, error) {
pathsToVarsFiles = append(pathsToVarsFiles, string(varFilePath))
}

var instanceSteps []int64
if len(cmd.InstanceSteps) > 0 {
for _, v := range strings.Split(cmd.InstanceSteps, ",") {
parsedInt, err := strconv.ParseInt(v, 0, 64)
if err != nil {
return v7pushaction.FlagOverrides{}, err
}
instanceSteps = append(instanceSteps, parsedInt)
}
}

return v7pushaction.FlagOverrides{
AppName: cmd.OptionalArgs.AppName,
Buildpacks: cmd.Buildpacks,
Expand All @@ -355,6 +368,7 @@ func (cmd PushCommand) GetFlagOverrides() (v7pushaction.FlagOverrides, error) {
HealthCheckType: cmd.HealthCheckType.Type,
HealthCheckTimeout: cmd.HealthCheckTimeout.Value,
Instances: cmd.Instances.NullInt,
InstanceSteps: instanceSteps,
MaxInFlight: cmd.MaxInFlight,
Memory: cmd.Memory,
NoStart: cmd.NoStart,
Expand Down Expand Up @@ -558,11 +572,25 @@ func (cmd PushCommand) ValidateFlags() error {
return translatableerror.RequiredFlagsError{Arg1: "--max-in-flight", Arg2: "--strategy"}
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"}
case len(cmd.InstanceSteps) > 0 && cmd.Strategy.Name != constant.DeploymentStrategyCanary:
return translatableerror.ArgumentCombinationError{Args: []string{"--instance-steps", "--strategy=canary"}}
case len(cmd.InstanceSteps) > 0 && !cmd.validateInstanceSteps():
return translatableerror.ParseArgumentError{ArgumentName: "--instance-steps", ExpectedType: "list of weights"}
}

return nil
}

func (cmd PushCommand) validateInstanceSteps() bool {
for _, v := range strings.Split(cmd.InstanceSteps, ",") {
_, err := strconv.ParseInt(v, 0, 64)
if err != nil {
return false
}
}
return true
}

func (cmd PushCommand) validBuildpacks() bool {
for _, buildpack := range cmd.Buildpacks {
if (buildpack == "null" || buildpack == "default") && len(cmd.Buildpacks) > 1 {
Expand Down
41 changes: 41 additions & 0 deletions command/v7/push_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,28 @@ var _ = Describe("push Command", func() {
})
})

Describe("canary steps", func() {
BeforeEach(func() {
cmd.InstanceSteps = "1,2,3,4"
})

When("canary strategy is provided", func() {
BeforeEach(func() {
cmd.Strategy = flag.DeploymentStrategy{Name: "canary"}
})

It("should succeed", func() {
Expect(executeErr).ToNot(HaveOccurred())
})
})

When("canary strategy is NOT provided", func() {
It("it just fails", func() {
Expect(executeErr).To(HaveOccurred())
})
})
})

When("when getting the application summary succeeds", func() {
BeforeEach(func() {
summary := v7action.DetailedApplicationSummary{
Expand Down Expand Up @@ -1399,5 +1421,24 @@ var _ = Describe("push Command", func() {
"--docker-username",
},
}),

Entry("instance-steps is not a list of ints",
func() {
cmd.Strategy = flag.DeploymentStrategy{Name: constant.DeploymentStrategyCanary}
cmd.InstanceSteps = "impossible"
},
translatableerror.ParseArgumentError{
ArgumentName: "--instance-steps",
ExpectedType: "list of weights",
}),

Entry("instance-steps can only be used with canary strategy",
func() {
cmd.InstanceSteps = "impossible"
},
translatableerror.ArgumentCombinationError{
Args: []string{
"--instance-steps", "--strategy=canary",
}}),
)
})
1 change: 1 addition & 0 deletions integration/v7/push/help_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ var _ = Describe("help", func() {
Eventually(session).Should(Say(`--endpoint`))
Eventually(session).Should(Say(`--health-check-type, -u`))
Eventually(session).Should(Say(`--instances, -i`))
Eventually(session).Should(Say(`--instance-steps`))
Eventually(session).Should(Say(`--log-rate-limit, -l\s+Log rate limit per second, in bytes \(e.g. 128B, 4K, 1M\). -l=-1 represents unlimited`))
Eventually(session).Should(Say(`--manifest, -f`))
Eventually(session).Should(Say(`--max-in-flight`))
Expand Down
21 changes: 15 additions & 6 deletions resources/deployment_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,20 @@ type Deployment struct {
}

type DeploymentOpts struct {
MaxInFlight int `json:"max_in_flight"`
MaxInFlight int `json:"max_in_flight,omitempty"`
CanaryDeploymentOptions CanaryDeploymentOptions `json:"canary,omitempty"`
}

func (d DeploymentOpts) IsEmpty() bool {
return d.MaxInFlight == 0 && len(d.CanaryDeploymentOptions.Steps) == 0
}

type CanaryDeploymentOptions struct {
Steps []CanaryStep `json:"steps"`
}

type CanaryStep struct {
InstanceWeight int64 `json:"instance_weight"`
}

// MarshalJSON converts a Deployment into a Cloud Controller Deployment.
Expand Down Expand Up @@ -56,12 +69,8 @@ func (d Deployment) MarshalJSON() ([]byte, error) {
ccDeployment.Strategy = d.Strategy
}

var b DeploymentOpts
if d.Options != b {
if !d.Options.IsEmpty() {
ccDeployment.Options = &d.Options
if d.Options.MaxInFlight < 1 {
ccDeployment.Options.MaxInFlight = 1
}
}

ccDeployment.Relationships = d.Relationships
Expand Down

0 comments on commit d6001ca

Please sign in to comment.