From a1fa7f6007c84ab15208d809c86d599ac8c17cb6 Mon Sep 17 00:00:00 2001
From: penghaoh <penghaoh@amazon.com>
Date: Tue, 15 Mar 2022 14:44:25 -0700
Subject: [PATCH] fix(stack): range min max should not enable spot

---
 .../cloudformation/stack/transformers.go      | 36 ++++++++-----------
 .../cloudformation/stack/transformers_test.go | 11 ++++++
 internal/pkg/manifest/svc.go                  |  7 ----
 internal/pkg/manifest/svc_test.go             | 11 ------
 4 files changed, 26 insertions(+), 39 deletions(-)

diff --git a/internal/pkg/deploy/cloudformation/stack/transformers.go b/internal/pkg/deploy/cloudformation/stack/transformers.go
index 1a19667461e..65e7995753e 100644
--- a/internal/pkg/deploy/cloudformation/stack/transformers.go
+++ b/internal/pkg/deploy/cloudformation/stack/transformers.go
@@ -134,11 +134,7 @@ func convertAdvancedCount(a manifest.AdvancedCount) (*template.AdvancedCount, er
 // convertCapacityProviders transforms the manifest fields into a format
 // parsable by the templates pkg.
 func convertCapacityProviders(a manifest.AdvancedCount) []*template.CapacityProviderStrategy {
-	if a.IsEmpty() {
-		return nil
-	}
-	// return if autoscaling range specified without spot scaling
-	if !a.Range.IsEmpty() && a.Range.Value != nil {
+	if a.Spot == nil && a.Range.RangeConfig.SpotFrom == nil {
 		return nil
 	}
 	var cps []*template.CapacityProviderStrategy
@@ -147,27 +143,25 @@ func convertCapacityProviders(a manifest.AdvancedCount) []*template.CapacityProv
 		Weight:           aws.Int(1),
 		CapacityProvider: capacityProviderFargateSpot,
 	})
+	rc := a.Range.RangeConfig
 	// Return if only spot is specifed as count
-	if a.Range.IsEmpty() {
+	if rc.SpotFrom == nil {
 		return cps
 	}
 	// Scaling with spot
-	rc := a.Range.RangeConfig
-	if !rc.IsEmpty() {
-		spotFrom := aws.IntValue(rc.SpotFrom)
-		min := aws.IntValue(rc.Min)
-		// If spotFrom value is not equal to the autoscaling min, then
-		// the base value on the Fargate Capacity provider must be set
-		// to one less than spotFrom
-		if spotFrom > min {
-			base := spotFrom - 1
-			fgCapacity := &template.CapacityProviderStrategy{
-				Base:             aws.Int(base),
-				Weight:           aws.Int(0),
-				CapacityProvider: capacityProviderFargate,
-			}
-			cps = append(cps, fgCapacity)
+	spotFrom := aws.IntValue(rc.SpotFrom)
+	min := aws.IntValue(rc.Min)
+	// If spotFrom value is not equal to the autoscaling min, then
+	// the base value on the Fargate Capacity provider must be set
+	// to one less than spotFrom
+	if spotFrom > min {
+		base := spotFrom - 1
+		fgCapacity := &template.CapacityProviderStrategy{
+			Base:             aws.Int(base),
+			Weight:           aws.Int(0),
+			CapacityProvider: capacityProviderFargate,
 		}
+		cps = append(cps, fgCapacity)
 	}
 	return cps
 }
diff --git a/internal/pkg/deploy/cloudformation/stack/transformers_test.go b/internal/pkg/deploy/cloudformation/stack/transformers_test.go
index 8f256e5a0f9..ddb1409e635 100644
--- a/internal/pkg/deploy/cloudformation/stack/transformers_test.go
+++ b/internal/pkg/deploy/cloudformation/stack/transformers_test.go
@@ -384,6 +384,17 @@ func Test_convertCapacityProviders(t *testing.T) {
 			},
 			expected: nil,
 		},
+		"returns nil if no spot config specified with min max": {
+			input: manifest.AdvancedCount{
+				Range: manifest.Range{
+					RangeConfig: manifest.RangeConfig{
+						Min: aws.Int(1),
+						Max: aws.Int(10),
+					},
+				},
+			},
+			expected: nil,
+		},
 	}
 
 	for name, tc := range testCases {
diff --git a/internal/pkg/manifest/svc.go b/internal/pkg/manifest/svc.go
index 91be4759827..c6cde618e00 100644
--- a/internal/pkg/manifest/svc.go
+++ b/internal/pkg/manifest/svc.go
@@ -138,13 +138,6 @@ func (c *Count) UnmarshalYAML(value *yaml.Node) error {
 		}
 	}
 
-	if c.AdvancedCount.Spot != nil && (c.AdvancedCount.hasAutoscaling()) {
-		return &errFieldMutualExclusive{
-			firstField:  "spot",
-			secondField: "range/cpu_percentage/memory_percentage/requests/response_time/queue_delay",
-		}
-	}
-
 	if !c.AdvancedCount.IsEmpty() {
 		// Successfully unmarshalled AdvancedCount fields, return
 		return nil
diff --git a/internal/pkg/manifest/svc_test.go b/internal/pkg/manifest/svc_test.go
index 9dc2769d114..710d92082a1 100644
--- a/internal/pkg/manifest/svc_test.go
+++ b/internal/pkg/manifest/svc_test.go
@@ -473,17 +473,6 @@ func TestCount_UnmarshalYAML(t *testing.T) {
 				},
 			},
 		},
-
-		"Error if mutually exclusive fields are specified": {
-			inContent: []byte(`count:
-  spot: 1
-  cpu_percentage: 30
-`),
-			wantedError: &errFieldMutualExclusive{
-				firstField:  "spot",
-				secondField: "range/cpu_percentage/memory_percentage/requests/response_time/queue_delay",
-			},
-		},
 		"Error if unmarshalable": {
 			inContent: []byte(`count: badNumber
 `),