Skip to content

Commit

Permalink
chore: remove excessive validation for fargate spot for ARM (#5943)
Browse files Browse the repository at this point in the history
Address #5934 and #5941


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
  • Loading branch information
iamhopaul123 authored Sep 30, 2024
1 parent 1d7ec1d commit 656acea
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 153 deletions.
44 changes: 0 additions & 44 deletions internal/pkg/manifest/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
104 changes: 0 additions & 104 deletions internal/pkg/manifest/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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
Expand Down
5 changes: 0 additions & 5 deletions internal/pkg/manifest/workload_ecs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 656acea

Please sign in to comment.