Skip to content

Commit

Permalink
Replace snow instance type exact match with regex validation (#4895)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jiayiwang7
jiayiwang7 authored Feb 9, 2023
1 parent ff25652 commit e0073df
Show file tree
Hide file tree
Showing 10 changed files with 77 additions and 46 deletions.
4 changes: 1 addition & 3 deletions config/crd/bases/anywhere.eks.amazonaws.com_snowmachineconfigs.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,7 @@ spec:
type: string
type: array
instanceType:
description: 'InstanceType is the type of instance to create. Valid
values: "sbe-c.large" (default), "sbe-c.xlarge", "sbe-c.2xlarge"
and "sbe-c.4xlarge".'
description: InstanceType is the type of instance to create.
type: string
network:
description: Network provides the custom network setting for the machine.
Expand Down
4 changes: 1 addition & 3 deletions config/manifest/eksa-components.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4960,9 +4960,7 @@ spec:
type: string
type: array
instanceType:
description: 'InstanceType is the type of instance to create. Valid
values: "sbe-c.large" (default), "sbe-c.xlarge", "sbe-c.2xlarge"
and "sbe-c.4xlarge".'
description: InstanceType is the type of instance to create.
type: string
network:
description: Network provides the custom network setting for the machine.
Expand Down
3 changes: 2 additions & 1 deletion internal/pkg/api/snow.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ func WithSnowAMIIDForAllMachines(id string) SnowFiller {
}
}

func WithSnowInstanceTypeForAllMachines(instanceType anywherev1.SnowInstanceType) SnowFiller {
// WithSnowInstanceTypeForAllMachines specifies an instance type for all the snow machine configs.
func WithSnowInstanceTypeForAllMachines(instanceType string) SnowFiller {
return func(config SnowConfig) {
for _, m := range config.machineConfigs {
m.Spec.InstanceType = instanceType
Expand Down
3 changes: 2 additions & 1 deletion internal/pkg/api/snowmachines.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ func WithSnowAMIID(id string) SnowMachineConfigFiller {
}
}

func WithSnowInstanceType(instanceType anywherev1.SnowInstanceType) SnowMachineConfigFiller {
// WithSnowInstanceType specifies an instance type for the snow machine config.
func WithSnowInstanceType(instanceType string) SnowMachineConfigFiller {
return func(m *anywherev1.SnowMachineConfig) {
m.Spec.InstanceType = instanceType
}
Expand Down
17 changes: 6 additions & 11 deletions pkg/api/v1alpha1/snowmachineconfig.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,24 @@ package v1alpha1
import (
"errors"
"fmt"
"strings"
"regexp"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/strings/slices"

"github.com/aws/eks-anywhere/pkg/logger"
)

const (
SnowMachineConfigKind = "SnowMachineConfig"
DefaultSnowSSHKeyName = ""
DefaultSnowInstanceType = SbeCLarge
DefaultSnowInstanceType = "sbe-c.large"
DefaultSnowPhysicalNetworkConnectorType = SFPPlus
DefaultOSFamily = Ubuntu
MinimumContainerVolumeSizeUbuntu = 8
MinimumContainerVolumeSizeBottlerocket = 25
)

var validInstanceTypes = []SnowInstanceType{SbeCLarge, SbeCXLarge, SbeC2XLarge, SbeC4XLarge, SbeC8XLarge, SbeC12XLarge, SbeC16XLarge, SbeC24XLarge}
var snowInstanceTypesRegex = regexp.MustCompile(`^sbe-[cg]\.\d*x?large$`)

// NewSnowMachineConfigGenerate generates snowMachineConfig example for generate clusterconfig command.
func NewSnowMachineConfigGenerate(name string) *SnowMachineConfigGenerate {
Expand Down Expand Up @@ -94,13 +93,9 @@ func validateSnowMachineConfig(config *SnowMachineConfig) error {
}

func validateSnowMachineConfigInstanceType(instanceType string) error {
validInstanceTypesStr := make([]string, 0, len(validInstanceTypes))
for _, i := range validInstanceTypes {
validInstanceTypesStr = append(validInstanceTypesStr, string(i))
}

if !slices.Contains(validInstanceTypesStr, instanceType) {
return fmt.Errorf("SnowMachineConfig InstanceType %s is not supported, please use one of the following: %s", instanceType, strings.Join(validInstanceTypesStr, ", "))
match := snowInstanceTypesRegex.FindStringSubmatch(instanceType)
if match == nil {
return fmt.Errorf("SnowMachineConfig InstanceType %s is not supported", instanceType)
}
return nil
}
Expand Down
62 changes: 56 additions & 6 deletions pkg/api/v1alpha1/snowmachineconfig_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func TestSnowMachineConfigValidate(t *testing.T) {
name: "valid without ami",
obj: &SnowMachineConfig{
Spec: SnowMachineConfigSpec{
InstanceType: DefaultSnowInstanceType,
InstanceType: "sbe-c.4xlarge",
PhysicalNetworkConnector: DefaultSnowPhysicalNetworkConnectorType,
Devices: []string{"1.2.3.4"},
OSFamily: Ubuntu,
Expand All @@ -151,11 +151,36 @@ func TestSnowMachineConfigValidate(t *testing.T) {
wantErr: "",
},
{
name: "invalid instance type",
name: "invalid instance type sbe-g.largex",
obj: &SnowMachineConfig{
Spec: SnowMachineConfigSpec{
AMIID: "ami-1",
InstanceType: "sbe-g.largex",
PhysicalNetworkConnector: DefaultSnowPhysicalNetworkConnectorType,
Devices: []string{"1.2.3.4"},
OSFamily: Bottlerocket,
Network: SnowNetwork{
DirectNetworkInterfaces: []SnowDirectNetworkInterface{
{
Index: 1,
DHCP: true,
Primary: true,
},
},
},
ContainersVolume: &snowv1.Volume{
Size: 25,
},
},
},
wantErr: "SnowMachineConfig InstanceType sbe-g.largex is not supported",
},
{
name: "invalid instance type sbe-c-xlarge",
obj: &SnowMachineConfig{
Spec: SnowMachineConfigSpec{
AMIID: "ami-1",
InstanceType: "invalid-instance-type",
InstanceType: "sbe-c-large",
PhysicalNetworkConnector: DefaultSnowPhysicalNetworkConnectorType,
Devices: []string{"1.2.3.4"},
OSFamily: Bottlerocket,
Expand All @@ -173,14 +198,39 @@ func TestSnowMachineConfigValidate(t *testing.T) {
},
},
},
wantErr: "InstanceType invalid-instance-type is not supported, please use one of the following: sbe-c.large, sbe-c.xlarge, sbe-c.2xlarge, sbe-c.4xlarge, sbe-c.8xlarge, sbe-c.12xlarge, sbe-c.16xlarge, sbe-c.24xlarge",
wantErr: "SnowMachineConfig InstanceType sbe-c-large is not supported",
},
{
name: "invalid instance type sbe-c.elarge",
obj: &SnowMachineConfig{
Spec: SnowMachineConfigSpec{
AMIID: "ami-1",
InstanceType: "sbe-c.elarge",
PhysicalNetworkConnector: DefaultSnowPhysicalNetworkConnectorType,
Devices: []string{"1.2.3.4"},
OSFamily: Bottlerocket,
Network: SnowNetwork{
DirectNetworkInterfaces: []SnowDirectNetworkInterface{
{
Index: 1,
DHCP: true,
Primary: true,
},
},
},
ContainersVolume: &snowv1.Volume{
Size: 25,
},
},
},
wantErr: "SnowMachineConfig InstanceType sbe-c.elarge is not supported",
},
{
name: "invalid physical network connector",
obj: &SnowMachineConfig{
Spec: SnowMachineConfigSpec{
AMIID: "ami-1",
InstanceType: DefaultSnowInstanceType,
InstanceType: "sbe-c.64xlarge",
PhysicalNetworkConnector: "invalid-physical-network",
Devices: []string{"1.2.3.4"},
OSFamily: Bottlerocket,
Expand All @@ -205,7 +255,7 @@ func TestSnowMachineConfigValidate(t *testing.T) {
obj: &SnowMachineConfig{
Spec: SnowMachineConfigSpec{
AMIID: "ami-1",
InstanceType: DefaultSnowInstanceType,
InstanceType: "sbe-g.large",
PhysicalNetworkConnector: DefaultSnowPhysicalNetworkConnectorType,
OSFamily: Bottlerocket,
Network: SnowNetwork{
Expand Down
14 changes: 1 addition & 13 deletions pkg/api/v1alpha1/snowmachineconfig_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,10 @@ const (
SFPPlus PhysicalNetworkConnectorType = "SFP_PLUS"
QSFP PhysicalNetworkConnectorType = "QSFP"
RJ45 PhysicalNetworkConnectorType = "RJ45"

SbeCLarge SnowInstanceType = "sbe-c.large"
SbeCXLarge SnowInstanceType = "sbe-c.xlarge"
SbeC2XLarge SnowInstanceType = "sbe-c.2xlarge"
SbeC4XLarge SnowInstanceType = "sbe-c.4xlarge"
SbeC8XLarge SnowInstanceType = "sbe-c.8xlarge"
SbeC12XLarge SnowInstanceType = "sbe-c.12xlarge"
SbeC16XLarge SnowInstanceType = "sbe-c.16xlarge"
SbeC24XLarge SnowInstanceType = "sbe-c.24xlarge"
)

type PhysicalNetworkConnectorType string

type SnowInstanceType string

// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized.
// Important: Run "make generate" to regenerate code after modifying this file

Expand All @@ -37,8 +26,7 @@ type SnowMachineConfigSpec struct {
AMIID string `json:"amiID,omitempty"`

// InstanceType is the type of instance to create.
// Valid values: "sbe-c.large" (default), "sbe-c.xlarge", "sbe-c.2xlarge" and "sbe-c.4xlarge".
InstanceType SnowInstanceType `json:"instanceType,omitempty"`
InstanceType string `json:"instanceType,omitempty"`

// PhysicalNetworkConnector is the physical network connector type to use for creating direct network interfaces (DNI).
// Valid values: "SFP_PLUS" (default), "QSFP" and "RJ45".
Expand Down
12 changes: 6 additions & 6 deletions pkg/api/v1alpha1/snowmachineconfig_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func TestSnowMachineConfigValidateCreateNoAMI(t *testing.T) {

sOld := snowMachineConfig()
sOld.Spec.SshKeyName = "testKey"
sOld.Spec.InstanceType = v1alpha1.SbeCLarge
sOld.Spec.InstanceType = v1alpha1.DefaultSnowInstanceType
sOld.Spec.PhysicalNetworkConnector = v1alpha1.SFPPlus
sOld.Spec.Devices = []string{"1.2.3.4"}
sOld.Spec.OSFamily = v1alpha1.Bottlerocket
Expand Down Expand Up @@ -58,7 +58,7 @@ func TestSnowMachineConfigValidateCreateInvalidInstanceType(t *testing.T) {
func TestSnowMachineConfigValidateCreateEmptySSHKeyName(t *testing.T) {
g := NewWithT(t)
s := snowMachineConfig()
s.Spec.InstanceType = v1alpha1.SbeCLarge
s.Spec.InstanceType = v1alpha1.DefaultSnowInstanceType
s.Spec.PhysicalNetworkConnector = v1alpha1.SFPPlus
s.Spec.Devices = []string{"1.2.3.4"}
s.Spec.OSFamily = v1alpha1.Ubuntu
Expand All @@ -80,7 +80,7 @@ func TestSnowMachineConfigValidateCreate(t *testing.T) {
sOld := snowMachineConfig()
sOld.Spec.AMIID = "testAMI"
sOld.Spec.SshKeyName = "testKey"
sOld.Spec.InstanceType = v1alpha1.SbeCLarge
sOld.Spec.InstanceType = v1alpha1.DefaultSnowInstanceType
sOld.Spec.PhysicalNetworkConnector = v1alpha1.SFPPlus
sOld.Spec.Devices = []string{"1.2.3.4"}
sOld.Spec.OSFamily = v1alpha1.Bottlerocket
Expand All @@ -107,7 +107,7 @@ func TestSnowMachineConfigValidateUpdate(t *testing.T) {
sNew := sOld.DeepCopy()
sNew.Spec.AMIID = "testAMI"
sNew.Spec.SshKeyName = "testKey"
sNew.Spec.InstanceType = v1alpha1.SbeCLarge
sNew.Spec.InstanceType = v1alpha1.DefaultSnowInstanceType
sNew.Spec.PhysicalNetworkConnector = v1alpha1.SFPPlus
sNew.Spec.Devices = []string{"1.2.3.4"}
sNew.Spec.OSFamily = v1alpha1.Bottlerocket
Expand All @@ -134,7 +134,7 @@ func TestSnowMachineConfigValidateUpdateNoDevices(t *testing.T) {
sNew := sOld.DeepCopy()
sNew.Spec.AMIID = "testAMI"
sNew.Spec.SshKeyName = "testKey"
sNew.Spec.InstanceType = v1alpha1.SbeCLarge
sNew.Spec.InstanceType = v1alpha1.DefaultSnowInstanceType
sNew.Spec.PhysicalNetworkConnector = v1alpha1.SFPPlus
sNew.Spec.OSFamily = v1alpha1.Bottlerocket

Expand All @@ -147,7 +147,7 @@ func TestSnowMachineConfigValidateUpdateEmptySSHKeyName(t *testing.T) {
sOld := snowMachineConfig()
sNew := sOld.DeepCopy()
sNew.Spec.AMIID = "testAMI"
sNew.Spec.InstanceType = v1alpha1.SbeCLarge
sNew.Spec.InstanceType = v1alpha1.DefaultSnowInstanceType
sNew.Spec.PhysicalNetworkConnector = v1alpha1.SFPPlus
sNew.Spec.OSFamily = v1alpha1.Bottlerocket

Expand Down
2 changes: 1 addition & 1 deletion pkg/providers/snow/validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func (v *Validator) ValidateInstanceType(ctx context.Context, m *v1alpha1.SnowMa
return fmt.Errorf("credentials not found for device [%s]", ip)
}

if err := validateInstanceTypeInDevice(ctx, client, string(m.Spec.InstanceType), ip); err != nil {
if err := validateInstanceTypeInDevice(ctx, client, m.Spec.InstanceType, ip); err != nil {
return err
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/snow_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func TestSnowKubernetes124To125UbuntuMultipleFieldsUpgrade(t *testing.T) {
api.WithWorkerNodeCount(2),
),
provider.WithProviderUpgrade(
api.WithSnowInstanceTypeForAllMachines(v1alpha1.SbeCXLarge),
api.WithSnowInstanceTypeForAllMachines("sbe-c.xlarge"),
api.WithSnowPhysicalNetworkConnectorForAllMachines(v1alpha1.QSFP),
),
framework.WithEnvVar(features.K8s125SupportEnvVar, "true"),
Expand Down

0 comments on commit e0073df

Please sign in to comment.