From 656aceaf47e91b241ae76abe6bd68ddeb4622b1f Mon Sep 17 00:00:00 2001 From: Penghao He Date: Mon, 30 Sep 2024 11:16:30 -0700 Subject: [PATCH] chore: remove excessive validation for fargate spot for ARM (#5943) Address https://github.com/aws/copilot-cli/issues/5934 and https://github.com/aws/copilot-cli/pull/5941 By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License. --- internal/pkg/manifest/validate.go | 44 ----------- internal/pkg/manifest/validate_test.go | 104 ------------------------- internal/pkg/manifest/workload_ecs.go | 5 -- 3 files changed, 153 deletions(-) diff --git a/internal/pkg/manifest/validate.go b/internal/pkg/manifest/validate.go index e3c28c21a8e..911e7f84213 100644 --- a/internal/pkg/manifest/validate.go +++ b/internal/pkg/manifest/validate.go @@ -276,14 +276,6 @@ func (l LoadBalancedWebServiceConfig) validate() error { return fmt.Errorf("validate Windows: service connect (`network.connect`) is not supported for Windows") } } - if l.TaskConfig.IsARM() { - if err = validateARM(validateARMOpts{ - Spot: l.Count.AdvancedCount.Spot, - SpotFrom: l.Count.AdvancedCount.Range.RangeConfig.SpotFrom, - }); err != nil { - return fmt.Errorf("validate ARM: %w", err) - } - } if err = l.NLBConfig.validate(); err != nil { return fmt.Errorf(`validate "nlb": %w`, err) } @@ -409,14 +401,6 @@ func (b BackendServiceConfig) validate() error { return fmt.Errorf("validate Windows: service connect (`network.connect`) is not supported for Windows") } } - if b.TaskConfig.IsARM() { - if err = validateARM(validateARMOpts{ - Spot: b.Count.AdvancedCount.Spot, - SpotFrom: b.Count.AdvancedCount.Range.RangeConfig.SpotFrom, - }); err != nil { - return fmt.Errorf("validate ARM: %w", err) - } - } return nil } @@ -530,14 +514,6 @@ func (w WorkerServiceConfig) validate() error { return fmt.Errorf(`validate Windows: %w`, err) } } - if w.TaskConfig.IsARM() { - if err = validateARM(validateARMOpts{ - Spot: w.Count.AdvancedCount.Spot, - SpotFrom: w.Count.AdvancedCount.Range.RangeConfig.SpotFrom, - }); err != nil { - return fmt.Errorf("validate ARM: %w", err) - } - } return nil } @@ -611,14 +587,6 @@ func (s ScheduledJobConfig) validate() error { return fmt.Errorf(`validate Windows: %w`, err) } } - if s.TaskConfig.IsARM() { - if err = validateARM(validateARMOpts{ - Spot: s.Count.AdvancedCount.Spot, - SpotFrom: s.Count.AdvancedCount.Range.RangeConfig.SpotFrom, - }); err != nil { - return fmt.Errorf("validate ARM: %w", err) - } - } return nil } @@ -2032,11 +2000,6 @@ type validateWindowsOpts struct { efsVolumes map[string]*Volume } -type validateARMOpts struct { - Spot *int - SpotFrom *int -} - func validateHealthCheckPorts(opts validateHealthCheckPortsOpts) error { for _, rule := range opts.alb.RoutingRules() { healthCheckPort := rule.HealthCheckPort(opts.mainContainerPort) @@ -2394,13 +2357,6 @@ func validateWindows(opts validateWindowsOpts) error { return nil } -func validateARM(opts validateARMOpts) error { - if opts.Spot != nil || opts.SpotFrom != nil { - return errors.New(`'Fargate Spot' is not supported when deploying on ARM architecture`) - } - return nil -} - // validate returns nil if ImageLocationOrBuild is configured correctly. func (i ImageLocationOrBuild) validate() error { if err := i.Build.validate(); err != nil { diff --git a/internal/pkg/manifest/validate_test.go b/internal/pkg/manifest/validate_test.go index 7bb32232231..9983dbd893d 100644 --- a/internal/pkg/manifest/validate_test.go +++ b/internal/pkg/manifest/validate_test.go @@ -416,33 +416,6 @@ func TestLoadBalancedWebService_validate(t *testing.T) { }, wantedErrorMsgPrefix: "validate Windows: service connect (`network.connect`) is not supported for Window", }, - "error if fail to validate ARM": { - lbConfig: LoadBalancedWebService{ - Workload: Workload{ - Name: aws.String("mockName"), - }, - LoadBalancedWebServiceConfig: LoadBalancedWebServiceConfig{ - ImageConfig: testImageConfig, - TaskConfig: TaskConfig{ - Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("linux/arm64"))}, - Count: Count{ - AdvancedCount: AdvancedCount{ - Spot: aws.Int(123), - workloadType: manifestinfo.LoadBalancedWebServiceType, - }, - }, - }, - HTTPOrBool: HTTPOrBool{ - HTTP: HTTP{ - Main: RoutingRule{ - Path: stringP("/"), - }, - }, - }, - }, - }, - wantedErrorMsgPrefix: `validate ARM: `, - }, "error if neither of http or nlb is enabled": { lbConfig: LoadBalancedWebService{ Workload: Workload{ @@ -729,26 +702,6 @@ func TestBackendService_validate(t *testing.T) { }, wantedErrorMsgPrefix: "validate Windows: service connect (`network.connect`) is not supported for Window", }, - "error if fail to validate ARM": { - config: BackendService{ - Workload: Workload{ - Name: aws.String("mockName"), - }, - BackendServiceConfig: BackendServiceConfig{ - ImageConfig: testImageConfig, - TaskConfig: TaskConfig{ - Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("linux/arm64"))}, - Count: Count{ - AdvancedCount: AdvancedCount{ - Spot: aws.Int(123), - workloadType: manifestinfo.BackendServiceType, - }, - }, - }, - }, - }, - wantedErrorMsgPrefix: `validate ARM: `, - }, "error if fail to validate deployment": { config: BackendService{ Workload: Workload{ @@ -1177,26 +1130,6 @@ func TestWorkerService_validate(t *testing.T) { }, wantedErrorMsgPrefix: `validate Windows: `, }, - "error if fail to validate ARM": { - config: WorkerService{ - Workload: Workload{ - Name: aws.String("mockName"), - }, - WorkerServiceConfig: WorkerServiceConfig{ - ImageConfig: testImageConfig, - TaskConfig: TaskConfig{ - Platform: PlatformArgsOrString{PlatformString: (*PlatformString)(aws.String("linux/arm64"))}, - Count: Count{ - AdvancedCount: AdvancedCount{ - Spot: aws.Int(123), - workloadType: manifestinfo.WorkerServiceType, - }, - }, - }, - }, - }, - wantedErrorMsgPrefix: `validate ARM: `, - }, "error if fail to validate deployment": { config: WorkerService{ Workload: Workload{ @@ -3925,43 +3858,6 @@ func TestValidateWindows(t *testing.T) { } } -func TestValidateARM(t *testing.T) { - testCases := map[string]struct { - in validateARMOpts - wantedError error - }{ - "should return an error if Spot specified inline": { - in: validateARMOpts{ - Spot: aws.Int(2), - }, - wantedError: fmt.Errorf(`'Fargate Spot' is not supported when deploying on ARM architecture`), - }, - "should return an error if Spot specified with spot_from": { - in: validateARMOpts{ - SpotFrom: aws.Int(2), - }, - wantedError: fmt.Errorf(`'Fargate Spot' is not supported when deploying on ARM architecture`), - }, - "should return nil if Spot not specified": { - in: validateARMOpts{ - Spot: nil, - }, - wantedError: nil, - }, - } - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - err := validateARM(tc.in) - - if tc.wantedError != nil { - require.EqualError(t, err, tc.wantedError.Error()) - } else { - require.NoError(t, err) - } - }) - } -} - func TestDeploymentConfig_validate(t *testing.T) { testCases := map[string]struct { deployConfig DeploymentConfig diff --git a/internal/pkg/manifest/workload_ecs.go b/internal/pkg/manifest/workload_ecs.go index 7abb107fbb7..11f833c0a3b 100644 --- a/internal/pkg/manifest/workload_ecs.go +++ b/internal/pkg/manifest/workload_ecs.go @@ -201,11 +201,6 @@ func (t TaskConfig) IsWindows() bool { return isWindowsPlatform(t.Platform) } -// IsARM returns whether or not the service is building with an ARM Arch. -func (t TaskConfig) IsARM() bool { - return IsArmArch(t.Platform.Arch()) -} - // Secret represents an identifier for sensitive data stored in either SSM or SecretsManager. type Secret struct { from StringOrFromCFN // SSM Parameter name or ARN to a secret or secret ARN imported from another CloudFormation stack.