PR review

aws · Apr 26, 2024 · f59eff2 · f59eff2
1 parent a04f4d9
commit f59eff2
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 10 deletions.
diff --git a/cmd/controller/main.go b/cmd/controller/main.go
@@ -51,14 +51,14 @@ func main() {
 	nodeClassList := v1beta1.EC2NodeClassList{}
 	err := client.List(ctx, &nodeClassList)
 	if err != nil {
-		logging.FromContext(ctx).Fatalf("failed to list ec2nodeclasses on startup, %s", err.Error())
+		logging.FromContext(ctx).Fatalf("failed to list EC2NodeClasses on startup, %s", err.Error())
 	}
 
 	invalidNodeClasses := lo.FilterMap(nodeClassList.Items, func(nc v1beta1.EC2NodeClass, _ int) (string, bool) {
 		return nc.Name, len(nc.Spec.AMISelectorTerms) == 0
 	})
 	if len(invalidNodeClasses) != 0 {
-		logging.FromContext(ctx).Fatalf("detected nodeclasses {%s} with un-set AMISelectorTerms. Upgrade your EC2NodeClass to include AMISelectorTerms to continue.", strings.Join(invalidNodeClasses, ","))
+		logging.FromContext(ctx).With("ec2nodeclasses", strings.Join(invalidNodeClasses, ",")).Fatalf("detected EC2NodeClasses with un-set AMISelectorTerms. Upgrade your EC2NodeClasses to include AMISelectorTerms to continue.")
 	}
 
 	op.

diff --git a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml
@@ -66,11 +66,13 @@ spec:
                     If multiple fields are used for selection, the requirements are ANDed.
                   properties:
                     eksOptimized:
-                      description: EKSOptimized test
+                      description: EKSOptimized is an object which, when configured,
+                        allows Karpenter to automatically select the latest AMI from
+                        an optimized family.
                       properties:
                         family:
                           description: Family is the AMI family to use when selecting
-                            AMIs.
+                            EKS optimized AMIs.
                           enum:
                           - AL2
                           - AL2023
@@ -124,8 +126,8 @@ spec:
                     with a combination of other fields in amiSelectorTerms'
                   rule: '!self.all(x, has(x.eksOptimized) && (has(x.id) || has(x.tags)
                     || has(x.name) || has(x.owner)))'
-                - message: '`eksOptimized'' is mutually exclusive, cannot be set with
-                    other terms'
+                - message: '''eksOptimized'' is mutually exclusive, cannot be set
+                    with other terms'
                   rule: '!(self.exists(x, has(x.eksOptimized)) && self.size() != 1)'
               associatePublicIPAddress:
                 description: AssociatePublicIPAddress controls if public IP addresses

diff --git a/pkg/apis/v1beta1/ec2nodeclass.go b/pkg/apis/v1beta1/ec2nodeclass.go
@@ -49,7 +49,7 @@ type EC2NodeClassSpec struct {
 	// +kubebuilder:validation:XValidation:message="expected at least one, got none, ['eksOptimized', 'tags', 'id', 'name']",rule="self.all(x, has(x.eksOptimized) || 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.eksOptimized) || has(x.tags) || has(x.name) || has(x.owner)))"
 	// +kubebuilder:validation:XValidation:message="'eksOptimized' is mutually exclusive, cannot be set with a combination of other fields in amiSelectorTerms",rule="!self.all(x, has(x.eksOptimized) && (has(x.id) || has(x.tags) || has(x.name) || has(x.owner)))"
-	// +kubebuilder:validation:XValidation:message="`eksOptimized' is mutually exclusive, cannot be set with other terms",rule="!(self.exists(x, has(x.eksOptimized)) && self.size() != 1)"
+	// +kubebuilder:validation:XValidation:message="'eksOptimized' is mutually exclusive, cannot be set with other terms",rule="!(self.exists(x, has(x.eksOptimized)) && self.size() != 1)"
 	// +kubebuilder:validation:MinItems:=1
 	// +kubebuilder:validation:MaxItems:=30
 	// +required
@@ -158,7 +158,7 @@ type SecurityGroupSelectorTerm struct {
 // AMISelectorTerm defines selection logic for an ami used by Karpenter to launch nodes.
 // If multiple fields are used for selection, the requirements are ANDed.
 type AMISelectorTerm struct {
-	// EKSOptimized is an object which when configured allows Karpenter to select the latest AMI from an optimized family.
+	// EKSOptimized is an object which, when configured, allows Karpenter to automatically select the latest AMI from an optimized family.
 	// +optional
 	EKSOptimized *EKSOptimized `json:"eksOptimized,omitempty"`
 	// Tags is a map of key/value tags used to select subnets
@@ -182,7 +182,7 @@ type AMISelectorTerm struct {
 }
 
 type EKSOptimized struct {
-	// Family is the AMI family to use when selecting AMIs.
+	// Family is the AMI family to use when selecting EKS optimized AMIs.
 	// +kubebuilder:validation:Enum:={AL2,AL2023,Bottlerocket,Ubuntu,Windows2019,Windows2022}
 	Family string `json:"family"`
 }

diff --git a/pkg/providers/amifamily/ami.go b/pkg/providers/amifamily/ami.go
@@ -115,7 +115,7 @@ func NewDefaultProvider(versionProvider version.Provider, ssm ssmiface.SSMAPI, e
 	}
 }
 
-// Get Returning a list of AMIs with its associated requirements
+// Get returns a list of AMIs with their associated requirements
 func (p *DefaultProvider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, options *Options) (AMIs, error) {
 	amis, err := p.getAMIs(ctx, nodeClass.Spec.AMISelectorTerms)
 	if err != nil {