Merge pull request #16975 from justinsb/version_skew_more

Fixup kubelet and controlPlaneKubelet config building

nodeup/pkg/model/context.go

@@ -72,7 +72,10 @@ type NodeupModelContext struct {
 	// usesNoneDNS is true if the cluster runs with dns=none (which uses fixed IPs, for example a load balancer, instead of DNS)
 	usesNoneDNS bool
 
-	kubernetesVersion   *kopsmodel.KubernetesVersion
+	// Deprecated: This should be renamed to controlPlaneVersion / nodeVersion;
+	// controlPlaneVersion should probably/ideally only be populated on control plane nodes.
+	kubernetesVersion *kopsmodel.KubernetesVersion
+
 	bootstrapCerts      map[string]*nodetasks.BootstrapCert
 	bootstrapKeypairIDs map[string]string
 

pkg/apis/kops/model/kubernetes_version.go

@@ -50,7 +50,7 @@ func MustParseKubernetesVersion(versionString string) *KubernetesVersion {
 	return kubernetesVersion
 }
 
-func (v *KubernetesVersion) String() string {
+func (v KubernetesVersion) String() string {
 	return v.versionString
 }
 
@@ -62,13 +62,13 @@ func IsBaseURL(kubernetesVersion string) bool {
 
 // IsBaseURL checks if the version string is a URL, rather than a version identifier.
 // URLs are typically used for CI builds and during development.
-func (v *KubernetesVersion) IsBaseURL() bool {
+func (v KubernetesVersion) IsBaseURL() bool {
 	return IsBaseURL(v.versionString)
 }
 
 // IsGTE checks if the version is greater than or equal (>=) to the specified version.
 // It panics if the kubernetes version in the cluster is invalid, or if the version is invalid.
-func (v *KubernetesVersion) IsGTE(version string) bool {
+func (v KubernetesVersion) IsGTE(version string) bool {
 	parsedVersion, err := util.ParseKubernetesVersion(version)
 	if err != nil || parsedVersion == nil {
 		panic(fmt.Sprintf("error parsing version %q: %v", version, err))
@@ -84,6 +84,11 @@ func (v *KubernetesVersion) IsGTE(version string) bool {
 
 // IsLT checks if the version is strictly less (<) than the specified version.
 // It panics if the kubernetes version in the cluster is invalid, or if the version is invalid.
-func (v *KubernetesVersion) IsLT(version string) bool {
+func (v KubernetesVersion) IsLT(version string) bool {
 	return !v.IsGTE(version)
 }
+
+// Minor returns the minor version of the Kubernetes version.
+func (v KubernetesVersion) Minor() int {
+	return int(v.version.Minor)
+}

pkg/model/components/apiserver.go

@@ -185,11 +185,11 @@ func (b *KubeAPIServerOptionsBuilder) BuildOptions(cluster *kops.Cluster) error
 
 	if clusterSpec.CloudProvider.AWS != nil {
 
-		if _, found := c.FeatureGates["InTreePluginAWSUnregister"]; !found && b.IsKubernetesLT("1.31") {
+		if _, found := c.FeatureGates["InTreePluginAWSUnregister"]; !found && b.ControlPlaneKubernetesVersion().IsLT("1.31") {
 			c.FeatureGates["InTreePluginAWSUnregister"] = "true"
 		}
 
-		if _, found := c.FeatureGates["CSIMigrationAWS"]; !found && b.IsKubernetesLT("1.27") {
+		if _, found := c.FeatureGates["CSIMigrationAWS"]; !found && b.ControlPlaneKubernetesVersion().IsLT("1.27") {
 			c.FeatureGates["CSIMigrationAWS"] = "true"
 		}
 	}

pkg/model/components/awscloudcontrollermanager.go

@@ -74,7 +74,7 @@ func (b *AWSCloudControllerManagerOptionsBuilder) BuildOptions(cluster *kops.Clu
 
 	if eccm.Image == "" {
 		// See https://us.gcr.io/k8s-artifacts-prod/provider-aws/cloud-controller-manager
-		switch b.KubernetesVersion.Minor {
+		switch b.ControlPlaneKubernetesVersion().Minor() {
 		case 25:
 			eccm.Image = "registry.k8s.io/provider-aws/cloud-controller-manager:v1.25.15"
 		case 26:
@@ -94,7 +94,7 @@ func (b *AWSCloudControllerManagerOptionsBuilder) BuildOptions(cluster *kops.Clu
 		}
 	}
 
-	if b.IsKubernetesLT("1.25") {
+	if b.ControlPlaneKubernetesVersion().IsLT("1.25") {
 		eccm.EnableLeaderMigration = fi.PtrTo(true)
 	}
 

pkg/model/components/containerd_test.go

@@ -20,7 +20,6 @@ import (
 	"testing"
 
 	kopsapi "k8s.io/kops/pkg/apis/kops"
-	"k8s.io/kops/pkg/apis/kops/util"
 	"k8s.io/kops/pkg/assets"
 	"k8s.io/kops/util/pkg/vfs"
 )
@@ -47,20 +46,15 @@ func Test_Build_Containerd_Supported_Version(t *testing.T) {
 		c := buildContainerdCluster(v)
 		b := assets.NewAssetBuilder(vfs.Context, c.Spec.Assets, false)
 
-		version, err := util.ParseKubernetesVersion(v)
+		optionsContext, err := NewOptionsContext(c, b, b.KubeletSupportedVersion)
 		if err != nil {
-			t.Fatalf("unexpected error from ParseKubernetesVersion %s: %v", v, err)
+			t.Fatalf("unexpected error from NewOptionsContext: %v", err)
 		}
-
 		ob := &ContainerdOptionsBuilder{
-			&OptionsContext{
-				AssetBuilder:      b,
-				KubernetesVersion: *version,
-			},
+			OptionsContext: optionsContext,
 		}
 
-		err = ob.BuildOptions(c)
-		if err != nil {
+		if err := ob.BuildOptions(c); err != nil {
 			t.Fatalf("unexpected error from BuildOptions: %v", err)
 		}
 

pkg/model/components/context.go

@@ -39,15 +39,59 @@ import (
 type OptionsContext struct {
 	ClusterName string
 
+	// Deprecated: Prefer using NodeKubernetesVersion() and ControlPlaneKubernetesVersion()
 	KubernetesVersion semver.Version
 
 	AssetBuilder *assets.AssetBuilder
+
+	nodeKubernetesVersion         kopsmodel.KubernetesVersion
+	controlPlaneKubernetesVersion kopsmodel.KubernetesVersion
+}
+
+func NewOptionsContext(cluster *kops.Cluster, assetBuilder *assets.AssetBuilder, maxKubeletSupportedVersion string) (*OptionsContext, error) {
+	optionsContext := &OptionsContext{
+		ClusterName:  cluster.ObjectMeta.Name,
+		AssetBuilder: assetBuilder,
+	}
+
+	sv, err := util.ParseKubernetesVersion(cluster.Spec.KubernetesVersion)
+	if err != nil {
+		return nil, fmt.Errorf("unable to determine kubernetes version from %q", cluster.Spec.KubernetesVersion)
+	}
+	optionsContext.KubernetesVersion = *sv
+
+	controlPlaneKubernetesVersion, err := kopsmodel.ParseKubernetesVersion(cluster.Spec.KubernetesVersion)
+	if err != nil {
+		return nil, fmt.Errorf("unable to determine kubernetes version from %q: %w", cluster.Spec.KubernetesVersion, err)
+	}
+	nodeKubernetesVersion := controlPlaneKubernetesVersion
+	if maxKubeletSupportedVersion != "" {
+		nodeKubernetesVersion, err = kopsmodel.ParseKubernetesVersion(maxKubeletSupportedVersion)
+		if err != nil {
+			return nil, fmt.Errorf("unable to determine kubernetes version from %q: %w", maxKubeletSupportedVersion, err)
+		}
+	}
+
+	optionsContext.nodeKubernetesVersion = *nodeKubernetesVersion
+	optionsContext.controlPlaneKubernetesVersion = *controlPlaneKubernetesVersion
+
+	return optionsContext, nil
+}
+
+func (c *OptionsContext) NodeKubernetesVersion() kopsmodel.KubernetesVersion {
+	return c.nodeKubernetesVersion
+}
+
+func (c *OptionsContext) ControlPlaneKubernetesVersion() kopsmodel.KubernetesVersion {
+	return c.controlPlaneKubernetesVersion
 }
 
+// Deprecated: prefer using NodeKubernetesVersion() and ControlPlaneKubernetesVersion()
 func (c *OptionsContext) IsKubernetesGTE(version string) bool {
 	return util.IsKubernetesGTE(version, c.KubernetesVersion)
 }
 
+// Deprecated: prefer using NodeKubernetesVersion() and ControlPlaneKubernetesVersion()
 func (c *OptionsContext) IsKubernetesLT(version string) bool {
 	return !c.IsKubernetesGTE(version)
 }

pkg/model/components/gcpcloudcontrollermanager.go

@@ -73,13 +73,13 @@ func (b *GCPCloudControllerManagerOptionsBuilder) BuildOptions(cluster *kops.Clu
 
 	if ccmConfig.Image == "" {
 		// TODO: Implement CCM image publishing
-		switch b.KubernetesVersion.Minor {
+		switch b.ControlPlaneKubernetesVersion().Minor() {
 		default:
 			ccmConfig.Image = "gcr.io/k8s-staging-cloud-provider-gcp/cloud-controller-manager:master"
 		}
 	}
 
-	if b.IsKubernetesLT("1.25") {
+	if b.ControlPlaneKubernetesVersion().IsLT("1.25") {
 		ccmConfig.EnableLeaderMigration = fi.PtrTo(true)
 	}
 

pkg/model/components/kubecontrollermanager.go

@@ -53,7 +53,7 @@ func (b *KubeControllerManagerOptionsBuilder) BuildOptions(o *kops.Cluster) erro
 	// TLDR; set this too low, and have a few EBS Volumes, and you will spam AWS api
 
 	{
-		klog.V(4).Infof("Kubernetes version %q supports AttachDetachReconcileSyncPeriod; will configure", b.KubernetesVersion)
+		klog.V(4).Infof("Kubernetes version %q supports AttachDetachReconcileSyncPeriod; will configure", b.ControlPlaneKubernetesVersion().String())
 		// If not set ... or set to 0s ... which is stupid
 		if kcm.AttachDetachReconcileSyncPeriod == nil ||
 			kcm.AttachDetachReconcileSyncPeriod.Duration.String() == "0s" {
@@ -149,11 +149,11 @@ func (b *KubeControllerManagerOptionsBuilder) BuildOptions(o *kops.Cluster) erro
 			kcm.FeatureGates = make(map[string]string)
 		}
 
-		if _, found := kcm.FeatureGates["InTreePluginAWSUnregister"]; !found && b.IsKubernetesLT("1.31") {
+		if _, found := kcm.FeatureGates["InTreePluginAWSUnregister"]; !found && b.ControlPlaneKubernetesVersion().IsLT("1.31") {
 			kcm.FeatureGates["InTreePluginAWSUnregister"] = "true"
 		}
 
-		if _, found := kcm.FeatureGates["CSIMigrationAWS"]; !found && b.IsKubernetesLT("1.27") {
+		if _, found := kcm.FeatureGates["CSIMigrationAWS"]; !found && b.ControlPlaneKubernetesVersion().IsLT("1.27") {
 			kcm.FeatureGates["CSIMigrationAWS"] = "true"
 		}
 	}