Skip to content

chore: remove excessive validation for fargate spot for ARM #5943

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.

Sign up for GitHub

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

Merged
mergify merged 1 commit into from
Sep 30, 2024
Merged

chore: remove excessive validation for fargate spot for ARM #5943

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
44 changes: 0 additions & 44 deletions internal/pkg/manifest/validate.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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