Skip to content

Commit 3a8a13f

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #16975 from justinsb/version_skew_more
Fixup kubelet and controlPlaneKubelet config building
2 parents 905f9cf + 0f13759 commit 3a8a13f

File tree

15 files changed

+172
-129
lines changed

15 files changed

+172
-129
lines changed

‎nodeup/pkg/model/context.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,10 @@ type NodeupModelContext struct {
7272
// usesNoneDNS is true if the cluster runs with dns=none (which uses fixed IPs, for example a load balancer, instead of DNS)
7373
usesNoneDNS bool
7474

75-
kubernetesVersion *kopsmodel.KubernetesVersion
75+
// Deprecated: This should be renamed to controlPlaneVersion / nodeVersion;
76+
// controlPlaneVersion should probably/ideally only be populated on control plane nodes.
77+
kubernetesVersion *kopsmodel.KubernetesVersion
78+
7679
bootstrapCerts map[string]*nodetasks.BootstrapCert
7780
bootstrapKeypairIDs map[string]string
7881

‎pkg/apis/kops/model/kubernetes_version.go‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func MustParseKubernetesVersion(versionString string) *KubernetesVersion {
5050
return kubernetesVersion
5151
}
5252

53-
func (v *KubernetesVersion) String() string {
53+
func (v KubernetesVersion) String() string {
5454
return v.versionString
5555
}
5656

@@ -62,13 +62,13 @@ func IsBaseURL(kubernetesVersion string) bool {
6262

6363
// IsBaseURL checks if the version string is a URL, rather than a version identifier.
6464
// URLs are typically used for CI builds and during development.
65-
func (v *KubernetesVersion) IsBaseURL() bool {
65+
func (v KubernetesVersion) IsBaseURL() bool {
6666
return IsBaseURL(v.versionString)
6767
}
6868

6969
// IsGTE checks if the version is greater than or equal (>=) to the specified version.
7070
// It panics if the kubernetes version in the cluster is invalid, or if the version is invalid.
71-
func (v *KubernetesVersion) IsGTE(version string) bool {
71+
func (v KubernetesVersion) IsGTE(version string) bool {
7272
parsedVersion, err := util.ParseKubernetesVersion(version)
7373
if err != nil || parsedVersion == nil {
7474
panic(fmt.Sprintf("error parsing version %q: %v", version, err))
@@ -84,6 +84,11 @@ func (v *KubernetesVersion) IsGTE(version string) bool {
8484

8585
// IsLT checks if the version is strictly less (<) than the specified version.
8686
// It panics if the kubernetes version in the cluster is invalid, or if the version is invalid.
87-
func (v *KubernetesVersion) IsLT(version string) bool {
87+
func (v KubernetesVersion) IsLT(version string) bool {
8888
return !v.IsGTE(version)
8989
}
90+
91+
// Minor returns the minor version of the Kubernetes version.
92+
func (v KubernetesVersion) Minor() int {
93+
return int(v.version.Minor)
94+
}

‎pkg/model/components/apiserver.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,11 @@ func (b *KubeAPIServerOptionsBuilder) BuildOptions(cluster *kops.Cluster) error
185185

186186
if clusterSpec.CloudProvider.AWS != nil {
187187

188-
if _, found := c.FeatureGates["InTreePluginAWSUnregister"]; !found && b.IsKubernetesLT("1.31") {
188+
if _, found := c.FeatureGates["InTreePluginAWSUnregister"]; !found && b.ControlPlaneKubernetesVersion().IsLT("1.31") {
189189
c.FeatureGates["InTreePluginAWSUnregister"] = "true"
190190
}
191191

192-
if _, found := c.FeatureGates["CSIMigrationAWS"]; !found && b.IsKubernetesLT("1.27") {
192+
if _, found := c.FeatureGates["CSIMigrationAWS"]; !found && b.ControlPlaneKubernetesVersion().IsLT("1.27") {
193193
c.FeatureGates["CSIMigrationAWS"] = "true"
194194
}
195195
}

‎pkg/model/components/awscloudcontrollermanager.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func (b *AWSCloudControllerManagerOptionsBuilder) BuildOptions(cluster *kops.Clu
7474

7575
if eccm.Image == "" {
7676
// See https://us.gcr.io/k8s-artifacts-prod/provider-aws/cloud-controller-manager
77-
switch b.KubernetesVersion.Minor {
77+
switch b.ControlPlaneKubernetesVersion().Minor() {
7878
case 25:
7979
eccm.Image = "registry.k8s.io/provider-aws/cloud-controller-manager:v1.25.15"
8080
case 26:
@@ -94,7 +94,7 @@ func (b *AWSCloudControllerManagerOptionsBuilder) BuildOptions(cluster *kops.Clu
9494
}
9595
}
9696

97-
if b.IsKubernetesLT("1.25") {
97+
if b.ControlPlaneKubernetesVersion().IsLT("1.25") {
9898
eccm.EnableLeaderMigration = fi.PtrTo(true)
9999
}
100100

‎pkg/model/components/containerd_test.go‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"testing"
2121

2222
kopsapi "k8s.io/kops/pkg/apis/kops"
23-
"k8s.io/kops/pkg/apis/kops/util"
2423
"k8s.io/kops/pkg/assets"
2524
"k8s.io/kops/util/pkg/vfs"
2625
)
@@ -47,20 +46,15 @@ func Test_Build_Containerd_Supported_Version(t *testing.T) {
4746
c := buildContainerdCluster(v)
4847
b := assets.NewAssetBuilder(vfs.Context, c.Spec.Assets, false)
4948

50-
version, err := util.ParseKubernetesVersion(v)
49+
optionsContext, err := NewOptionsContext(c, b, b.KubeletSupportedVersion)
5150
if err != nil {
52-
t.Fatalf("unexpected error from ParseKubernetesVersion %s: %v", v, err)
51+
t.Fatalf("unexpected error from NewOptionsContext: %v", err)
5352
}
54-
5553
ob := &ContainerdOptionsBuilder{
56-
&OptionsContext{
57-
AssetBuilder: b,
58-
KubernetesVersion: *version,
59-
},
54+
OptionsContext: optionsContext,
6055
}
6156

62-
err = ob.BuildOptions(c)
63-
if err != nil {
57+
if err := ob.BuildOptions(c); err != nil {
6458
t.Fatalf("unexpected error from BuildOptions: %v", err)
6559
}
6660

‎pkg/model/components/context.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,59 @@ import (
3939
type OptionsContext struct {
4040
ClusterName string
4141

42+
// Deprecated: Prefer using NodeKubernetesVersion() and ControlPlaneKubernetesVersion()
4243
KubernetesVersion semver.Version
4344

4445
AssetBuilder *assets.AssetBuilder
46+
47+
nodeKubernetesVersion kopsmodel.KubernetesVersion
48+
controlPlaneKubernetesVersion kopsmodel.KubernetesVersion
49+
}
50+
51+
func NewOptionsContext(cluster *kops.Cluster, assetBuilder *assets.AssetBuilder, maxKubeletSupportedVersion string) (*OptionsContext, error) {
52+
optionsContext := &OptionsContext{
53+
ClusterName: cluster.ObjectMeta.Name,
54+
AssetBuilder: assetBuilder,
55+
}
56+
57+
sv, err := util.ParseKubernetesVersion(cluster.Spec.KubernetesVersion)
58+
if err != nil {
59+
return nil, fmt.Errorf("unable to determine kubernetes version from %q", cluster.Spec.KubernetesVersion)
60+
}
61+
optionsContext.KubernetesVersion = *sv
62+
63+
controlPlaneKubernetesVersion, err := kopsmodel.ParseKubernetesVersion(cluster.Spec.KubernetesVersion)
64+
if err != nil {
65+
return nil, fmt.Errorf("unable to determine kubernetes version from %q: %w", cluster.Spec.KubernetesVersion, err)
66+
}
67+
nodeKubernetesVersion := controlPlaneKubernetesVersion
68+
if maxKubeletSupportedVersion != "" {
69+
nodeKubernetesVersion, err = kopsmodel.ParseKubernetesVersion(maxKubeletSupportedVersion)
70+
if err != nil {
71+
return nil, fmt.Errorf("unable to determine kubernetes version from %q: %w", maxKubeletSupportedVersion, err)
72+
}
73+
}
74+
75+
optionsContext.nodeKubernetesVersion = *nodeKubernetesVersion
76+
optionsContext.controlPlaneKubernetesVersion = *controlPlaneKubernetesVersion
77+
78+
return optionsContext, nil
79+
}
80+
81+
func (c *OptionsContext) NodeKubernetesVersion() kopsmodel.KubernetesVersion {
82+
return c.nodeKubernetesVersion
83+
}
84+
85+
func (c *OptionsContext) ControlPlaneKubernetesVersion() kopsmodel.KubernetesVersion {
86+
return c.controlPlaneKubernetesVersion
4587
}
4688

89+
// Deprecated: prefer using NodeKubernetesVersion() and ControlPlaneKubernetesVersion()
4790
func (c *OptionsContext) IsKubernetesGTE(version string) bool {
4891
return util.IsKubernetesGTE(version, c.KubernetesVersion)
4992
}
5093

94+
// Deprecated: prefer using NodeKubernetesVersion() and ControlPlaneKubernetesVersion()
5195
func (c *OptionsContext) IsKubernetesLT(version string) bool {
5296
return !c.IsKubernetesGTE(version)
5397
}

‎pkg/model/components/gcpcloudcontrollermanager.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,13 @@ func (b *GCPCloudControllerManagerOptionsBuilder) BuildOptions(cluster *kops.Clu
7373

7474
if ccmConfig.Image == "" {
7575
// TODO: Implement CCM image publishing
76-
switch b.KubernetesVersion.Minor {
76+
switch b.ControlPlaneKubernetesVersion().Minor() {
7777
default:
7878
ccmConfig.Image = "gcr.io/k8s-staging-cloud-provider-gcp/cloud-controller-manager:master"
7979
}
8080
}
8181

82-
if b.IsKubernetesLT("1.25") {
82+
if b.ControlPlaneKubernetesVersion().IsLT("1.25") {
8383
ccmConfig.EnableLeaderMigration = fi.PtrTo(true)
8484
}
8585

‎pkg/model/components/kubecontrollermanager.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func (b *KubeControllerManagerOptionsBuilder) BuildOptions(o *kops.Cluster) erro
5353
// TLDR; set this too low, and have a few EBS Volumes, and you will spam AWS api
5454

5555
{
56-
klog.V(4).Infof("Kubernetes version %q supports AttachDetachReconcileSyncPeriod; will configure", b.KubernetesVersion)
56+
klog.V(4).Infof("Kubernetes version %q supports AttachDetachReconcileSyncPeriod; will configure", b.ControlPlaneKubernetesVersion().String())
5757
// If not set ... or set to 0s ... which is stupid
5858
if kcm.AttachDetachReconcileSyncPeriod == nil ||
5959
kcm.AttachDetachReconcileSyncPeriod.Duration.String() == "0s" {
@@ -149,11 +149,11 @@ func (b *KubeControllerManagerOptionsBuilder) BuildOptions(o *kops.Cluster) erro
149149
kcm.FeatureGates = make(map[string]string)
150150
}
151151

152-
if _, found := kcm.FeatureGates["InTreePluginAWSUnregister"]; !found && b.IsKubernetesLT("1.31") {
152+
if _, found := kcm.FeatureGates["InTreePluginAWSUnregister"]; !found && b.ControlPlaneKubernetesVersion().IsLT("1.31") {
153153
kcm.FeatureGates["InTreePluginAWSUnregister"] = "true"
154154
}
155155

156-
if _, found := kcm.FeatureGates["CSIMigrationAWS"]; !found && b.IsKubernetesLT("1.27") {
156+
if _, found := kcm.FeatureGates["CSIMigrationAWS"]; !found && b.ControlPlaneKubernetesVersion().IsLT("1.27") {
157157
kcm.FeatureGates["CSIMigrationAWS"] = "true"
158158
}
159159
}

0 commit comments

Comments
 (0)