BREAKING CHANGE: make AMISelectorTerms required

aws · Apr 5, 2024 · 049bb1d · 049bb1d
1 parent 67a88b7
commit 049bb1d
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 3 deletions.
diff --git a/cmd/controller/main.go b/cmd/controller/main.go
@@ -15,8 +15,12 @@ limitations under the License.
 package main
 
 import (
+	"fmt"
+	"strings"
+
 	"github.com/samber/lo"
 
+	"github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1"
 	"github.com/aws/karpenter-provider-aws/pkg/cloudprovider"
 	"github.com/aws/karpenter-provider-aws/pkg/controllers"
 	"github.com/aws/karpenter-provider-aws/pkg/operator"
@@ -43,6 +47,27 @@ func main() {
 	lo.Must0(op.AddHealthzCheck("cloud-provider", awsCloudProvider.LivenessProbe))
 	cloudProvider := metrics.Decorate(awsCloudProvider)
 
+	client := op.Manager.GetAPIReader()
+	if client == nil {
+		panic("panic: failed to initialize read-client on startup")
+	}
+	nodeClassList := v1beta1.EC2NodeClassList{}
+	err := client.List(ctx, &nodeClassList)
+	if err != nil {
+		panic(fmt.Sprintf("panic: failed to list ec2nodeclasses on startup, %s", err.Error()))
+	}
+
+	ec2ncNames := []string{}
+	for i := range nodeClassList.Items {
+		nc := nodeClassList.Items[i]
+		if nc.Spec.AMISelectorTerms == nil || len(nc.Spec.AMISelectorTerms) == 0 {
+			ec2ncNames = append(ec2ncNames, nc.Name)
+		}
+	}
+	if len(ec2ncNames) != 0 {
+		panic(fmt.Sprintf("panic: detected nodeclasses {%s} with un-set AMISelectorTerms. Upgrade your EC2NodeClass to include AMISelectorTerms to continue.", strings.Join(ec2ncNames, ",")))
+	}
+
 	op.
 		WithControllers(ctx, corecontrollers.NewControllers(
 			op.Clock,

diff --git a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml
@@ -446,6 +446,7 @@ spec:
                 type: string
             required:
             - amiFamily
+            - amiSelectorTerms
             - securityGroupSelectorTerms
             - subnetSelectorTerms
             type: object

diff --git a/pkg/apis/v1beta1/ec2nodeclass.go b/pkg/apis/v1beta1/ec2nodeclass.go
@@ -48,8 +48,8 @@ type EC2NodeClassSpec struct {
 	// +kubebuilder:validation:XValidation:message="expected at least one, got none, ['tags', 'id', 'name']",rule="self.all(x, has(x.tags) || has(x.id) || has(x.name))"
 	// +kubebuilder:validation:XValidation:message="'id' is mutually exclusive, cannot be set with a combination of other fields in amiSelectorTerms",rule="!self.all(x, has(x.id) && (has(x.tags) || has(x.name) || has(x.owner)))"
 	// +kubebuilder:validation:MaxItems:=30
-	// +optional
-	AMISelectorTerms []AMISelectorTerm `json:"amiSelectorTerms,omitempty" hash:"ignore"`
+	// +required
+	AMISelectorTerms []AMISelectorTerm `json:"amiSelectorTerms" hash:"ignore"`
 	// AMIFamily is the AMI family that instances use.
 	// +kubebuilder:validation:Enum:={AL2,AL2023,Bottlerocket,Ubuntu,Custom,Windows2019,Windows2022}
 	// +required

diff --git a/pkg/providers/amifamily/ami.go b/pkg/providers/amifamily/ami.go
@@ -115,7 +115,7 @@ func NewProvider(versionProvider *version.Provider, ssm ssmiface.SSMAPI, ec2api
 func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, options *Options) (AMIs, error) {
 	var err error
 	var amis AMIs
-	if len(nodeClass.Spec.AMISelectorTerms) == 0 {
+	if nodeClass.Spec.AMISelectorTerms == nil || len(nodeClass.Spec.AMISelectorTerms) == 0 {
 		amis, err = p.getDefaultAMIs(ctx, nodeClass, options)
 		if err != nil {
 			return nil, err