feedback addressed

pkg/controllers/providers/version/controller.go

@@ -20,8 +20,6 @@ import (
 	"time"
 
 	"github.com/awslabs/operatorpkg/singleton"
-	lop "github.com/samber/lo/parallel"
-	"go.uber.org/multierr"
 	controllerruntime "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -43,16 +41,7 @@ func NewController(versionProvider *version.DefaultProvider) *Controller {
 func (c *Controller) Reconcile(ctx context.Context) (reconcile.Result, error) {
 	ctx = injection.WithControllerName(ctx, "providers.version")
 
-	work := []func(ctx context.Context) error{
-		c.versionProvider.UpdateVersion,
-	}
-	errs := make([]error, len(work))
-	lop.ForEach(work, func(f func(ctx context.Context) error, i int) {
-		if err := f(ctx); err != nil {
-			errs[i] = err
-		}
-	})
-	if err := multierr.Combine(errs...); err != nil {
+	if err := c.versionProvider.UpdateVersion(ctx); err != nil {
 		return reconcile.Result{}, fmt.Errorf("updating version, %w", err)
 	}
 	return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil

pkg/operator/options/options.go

@@ -49,7 +49,7 @@ func (o *Options) AddFlags(fs *coreoptions.FlagSet) {
 	fs.StringVar(&o.ClusterName, "cluster-name", env.WithDefaultString("CLUSTER_NAME", ""), "[REQUIRED] The kubernetes cluster name for resource discovery.")
 	fs.StringVar(&o.ClusterEndpoint, "cluster-endpoint", env.WithDefaultString("CLUSTER_ENDPOINT", ""), "The external kubernetes cluster endpoint for new nodes to connect with. If not specified, will discover the cluster endpoint using DescribeCluster API.")
 	fs.BoolVarWithEnv(&o.IsolatedVPC, "isolated-vpc", "ISOLATED_VPC", false, "If true, then assume we can't reach AWS services which don't have a VPC endpoint. This also has the effect of disabling look-ups to the AWS on-demand pricing endpoint.")
-	fs.BoolVarWithEnv(&o.EKSControlPlane, "eks-control-plane", "EKS_CONTROL_PLANE", true, "Marking this true means that your cluster is running with an EKS control plane and Karpenter should attempt to discover cluster details from the DescribeCluster API ")
+	fs.BoolVarWithEnv(&o.EKSControlPlane, "eks-control-plane", "EKS_CONTROL_PLANE", false, "Marking this true means that your cluster is running with an EKS control plane and Karpenter should attempt to discover cluster details from the DescribeCluster API ")
 	fs.Float64Var(&o.VMMemoryOverheadPercent, "vm-memory-overhead-percent", utils.WithDefaultFloat64("VM_MEMORY_OVERHEAD_PERCENT", 0.075), "The VM memory overhead as a percent that will be subtracted from the total memory for all instance types when cached information is unavailable.")
 	fs.StringVar(&o.InterruptionQueue, "interruption-queue", env.WithDefaultString("INTERRUPTION_QUEUE", ""), "Interruption queue is the name of the SQS queue used for processing interruption events from EC2. Interruption handling is disabled if not specified. Enabling interruption handling may require additional permissions on the controller service account. Additional permissions are outlined in the docs.")
 	fs.IntVar(&o.ReservedENIs, "reserved-enis", env.WithDefaultInt("RESERVED_ENIS", 0), "Reserved ENIs are not included in the calculations for max-pods or kube-reserved. This is most often used in the VPC CNI custom networking setup https://docs.aws.amazon.com/eks/latest/userguide/cni-custom-network.html.")

pkg/providers/version/suite_test.go

@@ -80,19 +80,19 @@ var _ = Describe("Operator", func() {
 		It("should resolve Kubernetes Version via Describe Cluster with no errors", func() {
 			options.FromContext(ctx).EKSControlPlane = true
 			ExpectSingletonReconciled(ctx, versionController)
-			endpoint, err := awsEnv.VersionProvider.Get(ctx)
+			version, err := awsEnv.VersionProvider.Get(ctx)
 			Expect(err).ToNot(HaveOccurred())
-			Expect(endpoint).To(Equal("1.29"))
+			Expect(version).To(Equal("1.29"))
 		})
 	})
 
 	Context("with EKS_CONTROL_PLANE=false", func() {
 		It("should resolve Kubernetes Version via K8s API", func() {
 			options.FromContext(ctx).EKSControlPlane = false
 			ExpectSingletonReconciled(ctx, versionController)
-			endpoint, err := awsEnv.VersionProvider.Get(ctx)
+			version, err := awsEnv.VersionProvider.Get(ctx)
 			Expect(err).ToNot(HaveOccurred())
-			Expect(endpoint).To(Equal(testEnv.K8sVersion()))
+			Expect(version).To(Equal(testEnv.K8sVersion()))
 		})
 	})
 })

pkg/providers/version/version.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
+	"sync/atomic"
 
 	"github.com/patrickmn/go-cache"
 	"github.com/samber/lo"
@@ -30,7 +31,6 @@ import (
 
 	sdk "github.com/aws/karpenter-provider-aws/pkg/aws"
 
-	"github.com/aws/karpenter-provider-aws/pkg/fake"
 	"github.com/aws/karpenter-provider-aws/pkg/operator/options"
 
 	"sigs.k8s.io/karpenter/pkg/utils/pretty"
@@ -55,7 +55,7 @@ type DefaultProvider struct {
 	cm                  *pretty.ChangeMonitor
 	kubernetesInterface kubernetes.Interface
 	eksapi              sdk.EKSAPI
-	previousVersion     fake.AtomicPtr[string]
+	version             atomic.Pointer[string]
 }
 
 func NewDefaultProvider(kubernetesInterface kubernetes.Interface, cache *cache.Cache, eksapi sdk.EKSAPI) *DefaultProvider {
@@ -68,10 +68,7 @@ func NewDefaultProvider(kubernetesInterface kubernetes.Interface, cache *cache.C
 }
 
 func (p *DefaultProvider) Get(ctx context.Context) (string, error) {
-	if version := *p.previousVersion.Clone(); version != "" {
-		return version, nil
-	}
-	return "", fmt.Errorf("kubernetes version not yet cached")
+	return *p.version.Load(), nil
 }
 
 func (p *DefaultProvider) UpdateVersion(ctx context.Context) error {
@@ -80,24 +77,20 @@ func (p *DefaultProvider) UpdateVersion(ctx context.Context) error {
 
 	if options.FromContext(ctx).EKSControlPlane {
 		version, err = p.getEKSVersion(ctx)
-		versionSource = "EKS DescribeCluster"
+		if err != nil {
+			return fmt.Errorf("validating kubernetes version, %w", err)
+		}
 	} else {
 		version, err = p.getK8sVersion()
-		versionSource = "Kubernetes API"
-	}
-	if version == "" && err == nil {
-		version = *p.previousVersion.Clone()
-	}
-	if err != nil {
-		log.FromContext(ctx).Error(err, "failed to get kubernetes version")
-		return err
+		if err != nil {
+			return fmt.Errorf("validating kubernetes version, %w", err)
+		}
 	}
-	p.previousVersion.Set(&version)
+	p.version.Store(&version)
 	if p.cm.HasChanged("kubernetes-version", version) || p.cm.HasChanged("version-source", versionSource) {
-		log.FromContext(ctx).WithValues("version", version, "source", versionSource).V(1).Info("discovered kubernetes version")
+		log.FromContext(ctx).WithValues("version", version).V(1).Info("discovered kubernetes version")
 		if err := validateK8sVersion(version); err != nil {
-			log.FromContext(ctx).Error(err, "failed validating kubernetes version")
-			return err
+			return fmt.Errorf("validating kubernetes version, %w", err)
 		}
 	}
 	return nil