Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixup kubelet and controlPlaneKubelet config building #16975

Merged
merged 1 commit into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion nodeup/pkg/model/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
13 changes: 9 additions & 4 deletions pkg/apis/kops/model/kubernetes_version.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func MustParseKubernetesVersion(versionString string) *KubernetesVersion {
return kubernetesVersion
}

func (v *KubernetesVersion) String() string {
func (v KubernetesVersion) String() string {
return v.versionString
}

Expand All @@ -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))
Expand All @@ -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)
}
4 changes: 2 additions & 2 deletions pkg/model/components/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/model/components/awscloudcontrollermanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
}

Expand Down
14 changes: 4 additions & 10 deletions pkg/model/components/containerd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -47,20 +46,15 @@ func Test_Build_Containerd_Supported_Version(t *testing.T) {
c := buildContainerdCluster(v)
b := assets.NewAssetBuilder(vfs.Context, c.Spec.Assets, c.Spec.KubernetesVersion, 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)
}

Expand Down
44 changes: 44 additions & 0 deletions pkg/model/components/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/model/components/gcpcloudcontrollermanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/model/components/kubecontrollermanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down Expand Up @@ -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"
}
}
Expand Down
Loading
Loading