From 96947f0ba5b3504dd0e60e9d8f3a6aae536583f5 Mon Sep 17 00:00:00 2001 From: Clemens Hoffmann Date: Fri, 31 Jan 2025 18:31:01 +0100 Subject: [PATCH 1/2] Fix regex for timezone parsing to support 3-4 chars (#3379) --- command/v7/shared/app_summary_displayer_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/v7/shared/app_summary_displayer_test.go b/command/v7/shared/app_summary_displayer_test.go index 18fd016a264..2f72e095628 100644 --- a/command/v7/shared/app_summary_displayer_test.go +++ b/command/v7/shared/app_summary_displayer_test.go @@ -655,7 +655,7 @@ var _ = Describe("app summary displayer", func() { When("there is an active deployment", func() { var LastStatusChangeTimeString = "2024-07-29T17:32:29Z" - var dateTimeRegexPattern = `[a-zA-Z]{3}\s\d{2}\s[a-zA-Z]{3}\s\d{2}\:\d{2}\:\d{2}\s[A-Z]{3}\s\d{4}` + var dateTimeRegexPattern = `[a-zA-Z]{3}\s\d{2}\s[a-zA-Z]{3}\s\d{2}\:\d{2}\:\d{2}\s[A-Z]{3,4}\s\d{4}` var maxInFlightDefaultValue = 1 When("the deployment strategy is rolling", func() { From 8e76f4ddf849737f63711480840804355dbfc7e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Pereira?= Date: Mon, 3 Feb 2025 16:57:20 -0600 Subject: [PATCH 2/2] Allow users to push apps using a weighted canary deployment (#3390) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: João Pereira --- .../create_deployment_for_push_plan.go | 9 +++- .../create_deployment_for_push_plan_test.go | 38 +++++++++++++++++ actor/v7pushaction/push_plan.go | 2 + ...up_deployment_information_for_push_plan.go | 4 ++ ...ployment_information_for_push_plan_test.go | 34 +++++++++++++++ api/cloudcontroller/ccv3/deployment_test.go | 6 +-- command/v7/push_command.go | 28 +++++++++++++ command/v7/push_command_test.go | 41 +++++++++++++++++++ integration/v7/push/help_test.go | 1 + resources/deployment_resource.go | 21 +++++++--- 10 files changed, 174 insertions(+), 10 deletions(-) diff --git a/actor/v7pushaction/create_deployment_for_push_plan.go b/actor/v7pushaction/create_deployment_for_push_plan.go index 5b23178c3a5..9b369a56af0 100644 --- a/actor/v7pushaction/create_deployment_for_push_plan.go +++ b/actor/v7pushaction/create_deployment_for_push_plan.go @@ -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 { diff --git a/actor/v7pushaction/create_deployment_for_push_plan_test.go b/actor/v7pushaction/create_deployment_for_push_plan_test.go index 46031d80dc8..967cc36543c 100644 --- a/actor/v7pushaction/create_deployment_for_push_plan_test.go +++ b/actor/v7pushaction/create_deployment_for_push_plan_test.go @@ -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() { diff --git a/actor/v7pushaction/push_plan.go b/actor/v7pushaction/push_plan.go index dc4a181e2b2..089dad7fbee 100644 --- a/actor/v7pushaction/push_plan.go +++ b/actor/v7pushaction/push_plan.go @@ -22,6 +22,7 @@ type PushPlan struct { Strategy constant.DeploymentStrategy MaxInFlight int TaskTypeApplication bool + InstanceSteps []int64 DockerImageCredentials v7action.DockerImageCredentials @@ -48,6 +49,7 @@ type FlagOverrides struct { HealthCheckTimeout int64 HealthCheckType constant.HealthCheckType Instances types.NullInt + InstanceSteps []int64 Memory string MaxInFlight *int NoStart bool diff --git a/actor/v7pushaction/setup_deployment_information_for_push_plan.go b/actor/v7pushaction/setup_deployment_information_for_push_plan.go index 11e4dd26d8e..b610c7bae15 100644 --- a/actor/v7pushaction/setup_deployment_information_for_push_plan.go +++ b/actor/v7pushaction/setup_deployment_information_for_push_plan.go @@ -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 } diff --git a/actor/v7pushaction/setup_deployment_information_for_push_plan_test.go b/actor/v7pushaction/setup_deployment_information_for_push_plan_test.go index 14515c9f446..266edba6b32 100644 --- a/actor/v7pushaction/setup_deployment_information_for_push_plan_test.go +++ b/actor/v7pushaction/setup_deployment_information_for_push_plan_test.go @@ -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() { @@ -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()) @@ -59,6 +83,11 @@ 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() { @@ -66,5 +95,10 @@ var _ = Describe("SetupDeploymentInformationForPushPlan", 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()) + }) }) }) diff --git a/api/cloudcontroller/ccv3/deployment_test.go b/api/cloudcontroller/ccv3/deployment_test.go index ad2099583b1..87b37f8c7cc 100644 --- a/api/cloudcontroller/ccv3/deployment_test.go +++ b/api/cloudcontroller/ccv3/deployment_test.go @@ -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) }) @@ -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"}}), ), ) @@ -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, diff --git a/command/v7/push_command.go b/command/v7/push_command.go index 717e373b560..5ce6c75ec05 100644 --- a/command/v7/push_command.go +++ b/command/v7/push_command.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "strconv" "strings" "github.com/cloudfoundry/bosh-cli/director/template" @@ -89,6 +90,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"` @@ -349,6 +351,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, @@ -361,6 +374,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, @@ -564,11 +578,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 { diff --git a/command/v7/push_command_test.go b/command/v7/push_command_test.go index 1a58b4cfafe..6c034a59de1 100644 --- a/command/v7/push_command_test.go +++ b/command/v7/push_command_test.go @@ -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{ @@ -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", + }}), ) }) diff --git a/integration/v7/push/help_test.go b/integration/v7/push/help_test.go index 7503e313a38..951bbdb0ad6 100644 --- a/integration/v7/push/help_test.go +++ b/integration/v7/push/help_test.go @@ -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`)) diff --git a/resources/deployment_resource.go b/resources/deployment_resource.go index 6e5b6cc71a3..60dbdf4b9f1 100644 --- a/resources/deployment_resource.go +++ b/resources/deployment_resource.go @@ -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. @@ -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