Addressed feedback

aws · Nov 25, 2024 · b7f3847 · b7f3847
1 parent d68866f
commit b7f3847
Show file tree

Hide file tree

Showing 6 changed files with 14 additions and 18 deletions.
diff --git a/pkg/controllers/nodeclass/status/ami_test.go b/pkg/controllers/nodeclass/status/ami_test.go
@@ -37,7 +37,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
 	var k8sVersion string
 	BeforeEach(func() {
 		ExpectSingletonReconciled(ctx, versionController)
-		k8sVersion = lo.Must(awsEnv.VersionProvider.Get(ctx))
+		k8sVersion = awsEnv.VersionProvider.Get(ctx)
 		nodeClass = test.EC2NodeClass(v1.EC2NodeClass{
 			Spec: v1.EC2NodeClassSpec{
 				SubnetSelectorTerms: []v1.SubnetSelectorTerm{

diff --git a/pkg/providers/amifamily/ami.go b/pkg/providers/amifamily/ami.go
@@ -90,10 +90,7 @@ func (p *DefaultProvider) DescribeImageQueries(ctx context.Context, nodeClass *v
 	// Aliases are mutually exclusive, both on the term level and field level within a term.
 	// This is enforced by a CEL validation, we will treat this as an invariant.
 	if alias := nodeClass.Alias(); alias != nil {
-		kubernetesVersion, err := p.versionProvider.Get(ctx)
-		if err != nil {
-			return nil, fmt.Errorf("getting kubernetes version, %w", err)
-		}
+		kubernetesVersion := p.versionProvider.Get(ctx)
 		query, err := GetAMIFamily(alias.Family, nil).DescribeImageQuery(ctx, p.ssmProvider, kubernetesVersion, alias.Version)
 		if err != nil {
 			return []DescribeImageQuery{}, err

diff --git a/pkg/providers/amifamily/suite_test.go b/pkg/providers/amifamily/suite_test.go
@@ -136,7 +136,7 @@ var _ = Describe("AMIProvider", func() {
 	var version string
 	BeforeEach(func() {
 		ExpectSingletonReconciled(ctx, versionController)
-		version = lo.Must(awsEnv.VersionProvider.Get(ctx))
+		version = awsEnv.VersionProvider.Get(ctx)
 		nodeClass = test.EC2NodeClass()
 	})
 	It("should succeed to resolve AMIs (AL2)", func() {

diff --git a/pkg/providers/launchtemplate/suite_test.go b/pkg/providers/launchtemplate/suite_test.go
@@ -2080,7 +2080,7 @@ essential = true
 			It("should choose amis from SSM if no selector specified in EC2NodeClass", func() {
 				versionController := controllersversion.NewController(awsEnv.VersionProvider)
 				ExpectSingletonReconciled(ctx, versionController)
-				version := lo.Must(awsEnv.VersionProvider.Get(ctx))
+				version := awsEnv.VersionProvider.Get(ctx)
 				awsEnv.SSMAPI.Parameters = map[string]string{
 					fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2/recommended/image_id", version): "test-ami-123",
 				}

diff --git a/pkg/providers/version/suite_test.go b/pkg/providers/version/suite_test.go
@@ -80,8 +80,7 @@ var _ = Describe("Operator", func() {
 		It("should resolve Kubernetes Version via Describe Cluster with no errors", func() {
 			options.FromContext(ctx).EKSControlPlane = true
 			ExpectSingletonReconciled(ctx, versionController)
-			version, err := awsEnv.VersionProvider.Get(ctx)
-			Expect(err).ToNot(HaveOccurred())
+			version := awsEnv.VersionProvider.Get(ctx)
 			Expect(version).To(Equal("1.29"))
 		})
 	})
@@ -90,8 +89,7 @@ var _ = Describe("Operator", func() {
 		It("should resolve Kubernetes Version via K8s API", func() {
 			options.FromContext(ctx).EKSControlPlane = false
 			ExpectSingletonReconciled(ctx, versionController)
-			version, err := awsEnv.VersionProvider.Get(ctx)
-			Expect(err).ToNot(HaveOccurred())
+			version := awsEnv.VersionProvider.Get(ctx)
 			Expect(version).To(Equal(testEnv.K8sVersion()))
 		})
 	})

diff --git a/pkg/providers/version/version.go b/pkg/providers/version/version.go
@@ -45,7 +45,7 @@ const (
 )
 
 type Provider interface {
-	Get(ctx context.Context) (string, error)
+	Get(ctx context.Context) string
 }
 
 // DefaultProvider get the APIServer version. This will be initialized at start up and allows karpenter to have an understanding of the cluster version
@@ -67,8 +67,8 @@ func NewDefaultProvider(kubernetesInterface kubernetes.Interface, cache *cache.C
 	}
 }
 
-func (p *DefaultProvider) Get(ctx context.Context) (string, error) {
-	return *p.version.Load(), nil
+func (p *DefaultProvider) Get(ctx context.Context) string {
+	return *p.version.Load()
 }
 
 func (p *DefaultProvider) UpdateVersion(ctx context.Context) error {
@@ -126,13 +126,14 @@ func (p *DefaultProvider) getEKSVersion(ctx context.Context) (string, error) {
 	output, err := p.eksapi.DescribeCluster(ctx, &eks.DescribeClusterInput{
 		Name: lo.ToPtr(options.FromContext(ctx).ClusterName),
 	})
-	if err == nil && lo.FromPtr(output.Cluster.Version) != "" {
-		return *output.Cluster.Version, err
-	}
 	if err != nil {
 		return "", err
 	}
-	return "", nil
+	version := lo.FromPtr(output.Cluster.Version)
+	if version == "" {
+		return "", fmt.Errorf("failed to get cluster version")
+	}
+	return version, nil
 }
 
 func (p *DefaultProvider) getK8sVersion() (string, error) {