Skip to content

Commit

Permalink
Replace deprecated kubelet flags
Browse files Browse the repository at this point in the history
  • Loading branch information
rfranzke committed Jan 25, 2022
1 parent 76256de commit 9892434
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 93 deletions.
16 changes: 12 additions & 4 deletions pkg/webhook/controlplane/ensurer.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@ import (
"github.com/gardener/gardener/extensions/pkg/webhook/controlplane/genericmutator"
v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
"github.com/gardener/gardener/pkg/utils/version"
versionutils "github.com/gardener/gardener/pkg/utils/version"
"github.com/go-logr/logr"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"
)

Expand Down Expand Up @@ -401,7 +403,7 @@ func (e *ensurer) EnsureKubeletServiceUnitOptions(ctx context.Context, gctx gcon

if opt := extensionswebhook.UnitOptionWithSectionAndName(new, "Service", "ExecStart"); opt != nil {
command := extensionswebhook.DeserializeCommandLine(opt.Value)
command = ensureKubeletCommandLineArgs(command, csiEnabled)
command = ensureKubeletCommandLineArgs(command, csiEnabled, kubeletVersion)
opt.Value = extensionswebhook.SerializeCommandLine(command, 1, " \\\n ")
}

Expand All @@ -414,10 +416,12 @@ func (e *ensurer) EnsureKubeletServiceUnitOptions(ctx context.Context, gctx gcon
return new, nil
}

func ensureKubeletCommandLineArgs(command []string, csiEnabled bool) []string {
func ensureKubeletCommandLineArgs(command []string, csiEnabled bool, kubeletVersion *semver.Version) []string {
if csiEnabled {
command = extensionswebhook.EnsureStringWithPrefix(command, "--cloud-provider=", "external")
command = extensionswebhook.EnsureStringWithPrefix(command, "--enable-controller-attach-detach=", "true")
if !version.ConstraintK8sGreaterEqual123.Check(kubeletVersion) {
command = extensionswebhook.EnsureStringWithPrefix(command, "--cloud-provider=", "external")
command = extensionswebhook.EnsureStringWithPrefix(command, "--enable-controller-attach-detach=", "true")
}
} else {
command = extensionswebhook.EnsureStringWithPrefix(command, "--cloud-provider=", "aws")
}
Expand Down Expand Up @@ -449,6 +453,10 @@ func (e *ensurer) EnsureKubeletConfiguration(ctx context.Context, gctx gcontext.
new.FeatureGates["CSIMigrationAWS"] = true
// kubelets of new worker nodes can directly be started with the the <csiMigrationCompleteFeatureGate> feature gate
new.FeatureGates[csiMigrationCompleteFeatureGate] = true

if version.ConstraintK8sGreaterEqual123.Check(kubeletVersion) {
new.EnableControllerAttachDetach = pointer.Bool(true)
}
}

return nil
Expand Down
142 changes: 53 additions & 89 deletions pkg/webhook/controlplane/ensurer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ import (
"context"
"testing"

"github.com/Masterminds/semver"
"github.com/gardener/gardener-extension-provider-aws/pkg/aws"

"github.com/Masterminds/semver"
"github.com/coreos/go-systemd/v22/unit"
extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller"
"github.com/gardener/gardener/extensions/pkg/controller/csimigration"
Expand All @@ -35,6 +35,7 @@ import (
"github.com/gardener/gardener/pkg/utils/version"
"github.com/golang/mock/gomock"
. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -618,57 +619,38 @@ done
}
})

It("should modify existing elements of kubelet.service unit options (k8s < 1.17)", func() {
newUnitOptions := []*unit.UnitOption{
{
Section: "Service",
Name: "ExecStart",
Value: `/opt/bin/hyperkube kubelet \
--config=/var/lib/kubelet/config/kubelet \
--cloud-provider=aws`,
},
hostnamectlUnitOption,
}

opts, err := ensurer.EnsureKubeletServiceUnitOptions(ctx, eContextK8s116, semver.MustParse("1.16.0"), oldUnitOptions, nil)
Expect(err).To(Not(HaveOccurred()))
Expect(opts).To(Equal(newUnitOptions))
})
DescribeTable("should modify existing elements of kubelet.service unit options",
func(gctx gcontext.GardenContext, kubeletVersion *semver.Version, cloudProvider string, withControllerAttachDetachFlag bool) {
newUnitOptions := []*unit.UnitOption{
{
Section: "Service",
Name: "ExecStart",
Value: `/opt/bin/hyperkube kubelet \
--config=/var/lib/kubelet/config/kubelet`,
},
hostnamectlUnitOption,
}

It("should modify existing elements of kubelet.service unit options (k8s >= 1.17)", func() {
newUnitOptions := []*unit.UnitOption{
{
Section: "Service",
Name: "ExecStart",
Value: `/opt/bin/hyperkube kubelet \
--config=/var/lib/kubelet/config/kubelet \
--cloud-provider=aws`,
},
hostnamectlUnitOption,
}
if cloudProvider != "" {
newUnitOptions[0].Value += ` \
--cloud-provider=` + cloudProvider
}

opts, err := ensurer.EnsureKubeletServiceUnitOptions(ctx, eContextK8s117, semver.MustParse("1.17.0"), oldUnitOptions, nil)
Expect(err).To(Not(HaveOccurred()))
Expect(opts).To(Equal(newUnitOptions))
})
if withControllerAttachDetachFlag {
newUnitOptions[0].Value += ` \
--enable-controller-attach-detach=true`
}

It("should modify existing elements of kubelet.service unit options (k8s >= 1.18)", func() {
newUnitOptions := []*unit.UnitOption{
{
Section: "Service",
Name: "ExecStart",
Value: `/opt/bin/hyperkube kubelet \
--config=/var/lib/kubelet/config/kubelet \
--cloud-provider=external \
--enable-controller-attach-detach=true`,
},
hostnamectlUnitOption,
}
opts, err := ensurer.EnsureKubeletServiceUnitOptions(ctx, gctx, kubeletVersion, oldUnitOptions, nil)
Expect(err).To(Not(HaveOccurred()))
Expect(opts).To(Equal(newUnitOptions))
},

opts, err := ensurer.EnsureKubeletServiceUnitOptions(ctx, eContextK8s118, semver.MustParse("1.18.0"), oldUnitOptions, nil)
Expect(err).To(Not(HaveOccurred()))
Expect(opts).To(Equal(newUnitOptions))
})
Entry("kubelet version < 1.17", eContextK8s116, semver.MustParse("1.16.0"), "aws", false),
Entry("1.17 <= kubelet version < 1.18", eContextK8s117, semver.MustParse("1.17.0"), "aws", false),
Entry("1.18 <= kubelet version < 1.23", eContextK8s118, semver.MustParse("1.18.0"), "external", true),
Entry("kubelet version >= 1.23", eContextK8s118, semver.MustParse("1.23.0"), "", false),
)
})

Describe("#EnsureKubeletConfiguration", func() {
Expand All @@ -686,50 +668,32 @@ done
}
})

It("should modify existing elements of kubelet configuration (< 1.17)", func() {
newKubeletConfig := &kubeletconfigv1beta1.KubeletConfiguration{
FeatureGates: map[string]bool{
"Foo": true,
},
}
kubeletConfig := *oldKubeletConfig

err := ensurer.EnsureKubeletConfiguration(ctx, eContextK8s117, semver.MustParse("1.17.0"), &kubeletConfig, nil)
Expect(err).To(Not(HaveOccurred()))
Expect(&kubeletConfig).To(Equal(newKubeletConfig))
})

It("should modify existing elements of kubelet configuration (>= 1.18)", func() {
newKubeletConfig := &kubeletconfigv1beta1.KubeletConfiguration{
FeatureGates: map[string]bool{
"Foo": true,
"CSIMigration": true,
"CSIMigrationAWS": true,
"CSIMigrationAWSComplete": true,
},
}
kubeletConfig := *oldKubeletConfig
DescribeTable("should modify existing elements of kubelet configuration",
func(gctx gcontext.GardenContext, kubeletVersion *semver.Version, withCSIFeatureGates bool, csiMigrationCompleteFeatureGate string, enableControllerAttachDetach *bool) {
newKubeletConfig := &kubeletconfigv1beta1.KubeletConfiguration{
FeatureGates: map[string]bool{
"Foo": true,
},
EnableControllerAttachDetach: enableControllerAttachDetach,
}
kubeletConfig := *oldKubeletConfig

err := ensurer.EnsureKubeletConfiguration(ctx, eContextK8s118, semver.MustParse("1.18.0"), &kubeletConfig, nil)
Expect(err).To(Not(HaveOccurred()))
Expect(&kubeletConfig).To(Equal(newKubeletConfig))
})
if withCSIFeatureGates {
newKubeletConfig.FeatureGates["CSIMigration"] = true
newKubeletConfig.FeatureGates["CSIMigrationAWS"] = true
newKubeletConfig.FeatureGates[csiMigrationCompleteFeatureGate] = true
}

It("should modify existing elements of kubelet configuration (>= 1.21)", func() {
newKubeletConfig := &kubeletconfigv1beta1.KubeletConfiguration{
FeatureGates: map[string]bool{
"Foo": true,
"CSIMigration": true,
"CSIMigrationAWS": true,
"InTreePluginAWSUnregister": true,
},
}
kubeletConfig := *oldKubeletConfig
err := ensurer.EnsureKubeletConfiguration(ctx, gctx, kubeletVersion, &kubeletConfig, nil)
Expect(err).To(Not(HaveOccurred()))
Expect(&kubeletConfig).To(Equal(newKubeletConfig))
},

err := ensurer.EnsureKubeletConfiguration(ctx, eContextK8s121, semver.MustParse("1.21.0"), &kubeletConfig, nil)
Expect(err).To(Not(HaveOccurred()))
Expect(&kubeletConfig).To(Equal(newKubeletConfig))
})
Entry("kubelet < 1.18", eContextK8s117, semver.MustParse("1.17.0"), false, "", nil),
Entry("1.18 <= kubelet < 1.21", eContextK8s118, semver.MustParse("1.18.0"), true, "CSIMigrationAWSComplete", nil),
Entry("1.21 <= kubelet < 1.23", eContextK8s121, semver.MustParse("1.21.0"), true, "InTreePluginAWSUnregister", nil),
Entry("kubelet >= 1.23", eContextK8s121, semver.MustParse("1.23.0"), true, "InTreePluginAWSUnregister", pointer.Bool(true)),
)
})

Describe("#EnsureKubernetesGeneralConfiguration", func() {
Expand Down

0 comments on commit 9892434

Please sign in to comment.