fix: removing --keep-terminated-pod-volumes from kubelet flags and ad…

…ding k8s 1.31 to ci-test (#455)

* fix: removing --keep-terminated-pod-volumes from kubelet flags

* fix: test updating case to properly get the --keep-terminated-pod-volumes on older k8s versions

* fix: removing print statement

* test: upgrading controller-runtime/tools/envtest to leverage the new envtest 1.31 binaries

.github/workflows/ci-test.yml

@@ -14,7 +14,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        k8sVersion: ["1.25.x", "1.26.x", "1.27.x", "1.28.x", "1.29.x", "1.30.x"]
+        k8sVersion: ["1.25.x", "1.26.x", "1.27.x", "1.28.x", "1.29.x", "1.30.x", "1.31.x"]
     env:
       K8S_VERSION: ${{ matrix.k8sVersion }}
     steps:

hack/toolchain.sh

@@ -15,7 +15,7 @@ tools() {
     go install github.com/google/[email protected]
     go install github.com/mikefarah/yq/[email protected]
     go install github.com/norwoodj/helm-docs/cmd/[email protected]
-    go install sigs.k8s.io/controller-runtime/tools/setup-envtest@v0.0.0-20240409134613-20f3f4bed925
+    go install sigs.k8s.io/controller-runtime/tools/setup-envtest@0c7827e417acc15f29e7c4bfccede809d372676a 
     go install sigs.k8s.io/controller-tools/cmd/[email protected]
     go install github.com/sigstore/cosign/v2/cmd/[email protected]
 #   go install -tags extended github.com/gohugoio/[email protected]

pkg/providers/imagefamily/bootstrap/aksbootstrap.go

@@ -241,6 +241,7 @@ var (
 	// removed --image-pull-progress-deadline=30m  (not in 1.24?)
 	// removed --network-plugin=cni (not in 1.24?)
 	// removed --azure-container-registry-config (not in 1.30)
+	// removed --keep-terminated-pod-volumes (not in 1.31)
 	kubeletFlagsBase = map[string]string{
 		"--address":                           "0.0.0.0",
 		"--anonymous-auth":                    "false",
@@ -257,7 +258,6 @@ var (
 		"--eviction-hard":                     "memory.available<750Mi,nodefs.available<10%,nodefs.inodesFree<5%",
 		"--image-gc-high-threshold":           "85",
 		"--image-gc-low-threshold":            "80",
-		"--keep-terminated-pod-volumes":       "false",
 		"--kubeconfig":                        "/var/lib/kubelet/kubeconfig",
 		"--max-pods":                          "110",
 		"--node-status-update-frequency":      "10s",
@@ -486,6 +486,11 @@ func (a AKS) applyOptions(nbv *NodeBootstrapVariables) {
 	}), ",")
 
 	// Assign Per K8s version kubelet flags
+	minorVersion := semver.MustParse(a.KubernetesVersion).Minor
+	if minorVersion < 31 {
+		kubeletFlagsBase["--keep-terminated-pod-volumes"] = "false"
+	}
+
 	credentialProviderURL := CredentialProviderURL(a.KubernetesVersion, a.Arch)
 	if credentialProviderURL != "" { // use OOT credential provider
 		nbv.CredentialProviderDownloadURL = credentialProviderURL

pkg/providers/instancetype/suite_test.go

@@ -27,6 +27,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/blang/semver/v4"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/samber/lo"
@@ -1092,7 +1093,13 @@ var _ = Describe("InstanceType Provider", func() {
 	})
 
 	Context("Bootstrap", func() {
-		It("should gate kubelet flags that are dependent on kubelet version", func() {
+		var (
+			kubeletFlags          string
+			decodedString         string
+			minorVersion          uint64
+			credentialProviderURL string
+		)
+		BeforeEach(func() {
 			ExpectApplied(ctx, env.Client, nodePool, nodeClass)
 			pod := coretest.UnschedulablePod()
 			ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, coreProvisioner, pod)
@@ -1104,28 +1111,40 @@ var _ = Describe("InstanceType Provider", func() {
 			Expect(customData).ToNot(BeNil())
 			decodedBytes, err := base64.StdEncoding.DecodeString(customData)
 			Expect(err).To(Succeed())
-			decodedString := string(decodedBytes[:])
-			Expect(decodedString).To(ContainSubstring("CREDENTIAL_PROVIDER_DOWNLOAD_URL"))
-			kubeletFlags := decodedString[strings.Index(decodedString, "KUBELET_FLAGS=")+len("KUBELET_FLAGS="):]
+			decodedString = string(decodedBytes[:])
+			kubeletFlags = decodedString[strings.Index(decodedString, "KUBELET_FLAGS=")+len("KUBELET_FLAGS="):]
 
-			// TODO: (bsoghigian) leverage the helpers from the azure cni pr once they get in instead for testing kubelet flags
-			// NOTE: env.Version may differ from the version we get for the apiserver
 			k8sVersion, err := azureEnv.ImageProvider.KubeServerVersion(ctx)
 			Expect(err).To(BeNil())
-			crendetialProviderURL := bootstrap.CredentialProviderURL(k8sVersion, "amd64")
-			if crendetialProviderURL != "" {
+			minorVersion = semver.MustParse(k8sVersion).Minor
+			credentialProviderURL = bootstrap.CredentialProviderURL(k8sVersion, "amd64")
+		})
+
+		It("should include or exclude --keep-terminated-pod-volumes based on kubelet version", func() {
+			if minorVersion < 31 {
+				Expect(kubeletFlags).To(ContainSubstring("--keep-terminated-pod-volumes"))
+			} else {
+				Expect(kubeletFlags).ToNot(ContainSubstring("--keep-terminated-pod-volumes"))
+			}
+		})
+
+		It("should include correct flags and credential provider URL when CredentialProviderURL is not empty", func() {
+			if credentialProviderURL != "" {
 				Expect(kubeletFlags).ToNot(ContainSubstring("--azure-container-registry-config"))
 				Expect(kubeletFlags).To(ContainSubstring("--image-credential-provider-config=/var/lib/kubelet/credential-provider-config.yaml"))
 				Expect(kubeletFlags).To(ContainSubstring("--image-credential-provider-bin-dir=/var/lib/kubelet/credential-provider"))
-				Expect(decodedString).To(ContainSubstring(crendetialProviderURL))
-			} else {
+				Expect(decodedString).To(ContainSubstring(credentialProviderURL))
+			}
+		})
+
+		It("should include correct flags when CredentialProviderURL is empty", func() {
+			if credentialProviderURL == "" {
 				Expect(kubeletFlags).To(ContainSubstring("--azure-container-registry-config"))
 				Expect(kubeletFlags).ToNot(ContainSubstring("--image-credential-provider-config"))
 				Expect(kubeletFlags).ToNot(ContainSubstring("--image-credential-provider-bin-dir"))
 			}
 		})
 	})
-
 	Context("LoadBalancer", func() {
 		resourceGroup := "test-resourceGroup"