refactor: kube-vip commands

charts/cluster-api-runtime-extensions-nutanix/defaultclusterclasses/nutanix-cluster-class.yaml

@@ -153,7 +153,6 @@ spec:
         - hostnamectl set-hostname "{{ ds.meta_data.hostname }}"
         - echo "::1         ipv6-localhost ipv6-loopback" >/etc/hosts
         - echo "127.0.0.1   localhost" >>/etc/hosts
-        - echo "127.0.0.1   kubernetes" >>/etc/hosts
         - echo "127.0.0.1   {{ ds.meta_data.hostname }}" >> /etc/hosts
         useExperimentalRetryJoin: true
         verbosity: 10

hack/examples/bases/nutanix/clusterclass/kustomization.yaml.tmpl

@@ -63,8 +63,12 @@ patches:
 - target:
     kind: KubeadmControlPlaneTemplate
   patch: |-
+    # deletes 'echo "127.0.0.1   kubernetes" >>/etc/hosts'
     - op: "remove"
-      path: "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands/6"
+      path: "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands/4"
+    # deletes 'sed -i 's#path: /etc/kubernetes/admin.conf#path: ...'
+    - op: "remove"
+      path: "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands/5"
     - op: "remove"
       path: "/spec/template/spec/kubeadmConfigSpec/postKubeadmCommands/1"
 

pkg/handlers/generic/mutation/controlplanevirtualip/inject.go

@@ -134,12 +134,13 @@ func (h *ControlPlaneVirtualIP) Mutate(
 		selectors.ControlPlane(),
 		log,
 		func(obj *controlplanev1.KubeadmControlPlaneTemplate) error {
-			virtualIPProviderFile, getFileErr := virtualIPProvider.GetFile(
+			files, preKubeadmCommands, postKubeadmCommands, generateErr := virtualIPProvider.GenerateFilesAndCommands(
 				ctx,
 				controlPlaneEndpointVar,
+				cluster,
 			)
-			if getFileErr != nil {
-				return getFileErr
+			if generateErr != nil {
+				return generateErr
 			}
 
 			log.WithValues(
@@ -151,16 +152,9 @@ func (h *ControlPlaneVirtualIP) Mutate(
 			))
 			obj.Spec.Template.Spec.KubeadmConfigSpec.Files = append(
 				obj.Spec.Template.Spec.KubeadmConfigSpec.Files,
-				*virtualIPProviderFile,
+				files...,
 			)
 
-			preKubeadmCommands, postKubeadmCommands, getCommandsErr := virtualIPProvider.GetCommands(
-				cluster,
-			)
-			if getCommandsErr != nil {
-				return getCommandsErr
-			}
-
 			if len(preKubeadmCommands) > 0 {
 				log.WithValues(
 					"patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(),

pkg/handlers/generic/mutation/controlplanevirtualip/inject_test.go

@@ -21,7 +21,6 @@ import (
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/handlers/mutation"
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/testutils/capitest"
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/testutils/capitest/request"
-	virtuialipproviders "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/controlplanevirtualip/providers"
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/options"
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/test/helpers"
 )
@@ -83,14 +82,14 @@ var _ = Describe("Generate ControlPlane virtual IP patches", func() {
 						Operation: "add",
 						Path:      "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands",
 						ValueMatcher: gomega.ContainElements(
-							virtuialipproviders.KubeVIPPreKubeadmCommands,
+							"/bin/bash /etc/caren/configure-for-kube-vip.sh set-host-aliases use-super-admin.conf",
 						),
 					},
 					{
 						Operation: "add",
 						Path:      "/spec/template/spec/kubeadmConfigSpec/postKubeadmCommands",
 						ValueMatcher: gomega.ContainElements(
-							virtuialipproviders.KubeVIPPostKubeadmCommands,
+							"/bin/bash /etc/caren/configure-for-kube-vip.sh use-admin.conf",
 						),
 					},
 				},
@@ -148,20 +147,28 @@ var _ = Describe("Generate ControlPlane virtual IP patches", func() {
 								),
 								gomega.HaveKey("permissions"),
 							),
+							gomega.SatisfyAll(
+								gomega.HaveKey("content"),
+								gomega.HaveKeyWithValue(
+									"path",
+									gomega.ContainSubstring("configure-for-kube-vip.sh"),
+								),
+								gomega.HaveKey("permissions"),
+							),
 						),
 					},
 					{
 						Operation: "add",
 						Path:      "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands",
 						ValueMatcher: gomega.ContainElements(
-							virtuialipproviders.KubeVIPPreKubeadmCommands,
+							"/bin/bash /etc/caren/configure-for-kube-vip.sh set-host-aliases use-super-admin.conf",
 						),
 					},
 					{
 						Operation: "add",
 						Path:      "/spec/template/spec/kubeadmConfigSpec/postKubeadmCommands",
 						ValueMatcher: gomega.ContainElements(
-							virtuialipproviders.KubeVIPPostKubeadmCommands,
+							"/bin/bash /etc/caren/configure-for-kube-vip.sh use-admin.conf",
 						),
 					},
 				},

pkg/handlers/generic/mutation/controlplanevirtualip/providers/kubevip.go

@@ -5,6 +5,7 @@ package providers
 
 import (
 	"context"
+	_ "embed"
 	"fmt"
 
 	"github.com/blang/semver/v4"
@@ -14,19 +15,30 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1"
+	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/common"
+)
+
+const (
+	kubeVIPFileOwner       = "root:root"
+	kubeVIPFilePath        = "/etc/kubernetes/manifests/kube-vip.yaml"
+	kubeVIPFilePermissions = "0600"
+
+	configureForKubeVIPScriptPermissions = "0700"
 )
 
 var (
-	//nolint:lll // for readability prefer to keep the long line
-	KubeVIPPreKubeadmCommands = []string{`if [ -f /run/kubeadm/kubeadm.yaml ]; then
-  sed -i 's#path: /etc/kubernetes/admin.conf#path: /etc/kubernetes/super-admin.conf#' /etc/kubernetes/manifests/kube-vip.yaml;
-fi`}
-	//nolint:lll // for readability prefer to keep the long line
-	KubeVIPPostKubeadmCommands = []string{`if [ -f /run/kubeadm/kubeadm.yaml ]; then
-  sed -i 's#path: /etc/kubernetes/super-admin.conf#path: /etc/kubernetes/admin.conf#' /etc/kubernetes/manifests/kube-vip.yaml;
-fi`}
+	configureForKubeVIPScriptOnRemote = common.ConfigFilePathOnRemote(
+		"configure-for-kube-vip.sh")
+
+	configureForKubeVIPScriptOnRemotePreKubeadmCommand = "/bin/bash " +
+		configureForKubeVIPScriptOnRemote + " set-host-aliases use-super-admin.conf"
+	configureForKubeVIPScriptOnRemotePostKubeadmCommand = "/bin/bash " +
+		configureForKubeVIPScriptOnRemote + " use-admin.conf"
 )
 
+//go:embed templates/configure-for-kube-vip.sh
+var configureForKubeVIPScript []byte
+
 type kubeVIPFromConfigMapProvider struct {
 	client client.Reader
 
@@ -50,35 +62,34 @@ func (p *kubeVIPFromConfigMapProvider) Name() string {
 	return "kube-vip"
 }
 
-// GetFile reads the kube-vip template from the ConfigMap
-// and returns the content a File, templating the required variables.
-func (p *kubeVIPFromConfigMapProvider) GetFile(
+// GenerateFilesAndCommands returns files and pre/post kubeadm commands for kube-vip.
+// It reads kube-vip template from a ConfigMap and returns the content a File, templating the required variables.
+// If required, it also returns a script file and pre/post kubeadm commands to change the kube-vip Pod to use the new
+// super-admin.conf file.
+func (p *kubeVIPFromConfigMapProvider) GenerateFilesAndCommands(
 	ctx context.Context,
 	spec v1alpha1.ControlPlaneEndpointSpec,
-) (*bootstrapv1.File, error) {
+	cluster *clusterv1.Cluster,
+) (files []bootstrapv1.File, preKubeadmCommands, postKubeadmCommands []string, err error) {
 	data, err := getTemplateFromConfigMap(ctx, p.client, p.configMapKey)
 	if err != nil {
-		return nil, fmt.Errorf("failed getting template data: %w", err)
+		return nil, nil, nil, fmt.Errorf("failed getting template data: %w", err)
 	}
 
 	kubeVIPStaticPod, err := templateValues(spec, data)
 	if err != nil {
-		return nil, fmt.Errorf("failed templating static Pod: %w", err)
+		return nil, nil, nil, fmt.Errorf("failed templating static Pod: %w", err)
 	}
 
-	return &bootstrapv1.File{
-		Content:     kubeVIPStaticPod,
-		Owner:       kubeVIPFileOwner,
-		Path:        kubeVIPFilePath,
-		Permissions: kubeVIPFilePermissions,
-	}, nil
-}
+	files = []bootstrapv1.File{
+		{
+			Content:     kubeVIPStaticPod,
+			Owner:       kubeVIPFileOwner,
+			Path:        kubeVIPFilePath,
+			Permissions: kubeVIPFilePermissions,
+		},
+	}
 
-//
-//nolint:gocritic // No need for named return values
-func (p *kubeVIPFromConfigMapProvider) GetCommands(
-	cluster *clusterv1.Cluster,
-) ([]string, []string, error) {
 	// The kube-vip static Pod uses admin.conf on the host to connect to the API server.
 	// But, starting with Kubernetes 1.29, admin.conf first gets created with no RBAC permissions.
 	// At the same time, 'kubeadm init' command waits for the API server to be reachable on the kube-vip IP.
@@ -89,15 +100,40 @@ func (p *kubeVIPFromConfigMapProvider) GetCommands(
 	// after kubeadm has assigned it the necessary RBAC permissions.
 	//
 	// See https://github.com/kube-vip/kube-vip/issues/684
+	//
+	// There is also another issue introduced in Kubernetes 1.29.
+	// If a cloud provider did not yet initialise the node's .status.addresses,
+	// the code for creating the /etc/hosts file including the hostAliases does not get run.
+	// The kube-vip static Pod runs before the cloud provider and will not be able to resolve the kubernetes DNS name.
+	// To work around this:
+	// 1. return a preKubeadmCommand to add kubernetes DNS name to /etc/hosts.
+	//
+	// See https://github.com/kube-vip/kube-vip/issues/692
+	// See https://github.com/kubernetes/kubernetes/issues/122420#issuecomment-1864609518
 	needCommands, err := needHackCommands(cluster)
 	if err != nil {
-		return nil, nil, fmt.Errorf("failed to determine if kube-vip commands are needed: %w", err)
+		return nil, nil, nil, fmt.Errorf(
+			"failed to determine if kube-vip commands are needed: %w",
+			err,
+		)
 	}
 	if !needCommands {
-		return nil, nil, nil
+		return files, nil, nil, nil
 	}
 
-	return KubeVIPPreKubeadmCommands, KubeVIPPostKubeadmCommands, nil
+	files = append(
+		files,
+		bootstrapv1.File{
+			Content:     string(configureForKubeVIPScript),
+			Path:        configureForKubeVIPScriptOnRemote,
+			Permissions: configureForKubeVIPScriptPermissions,
+		},
+	)
+
+	preKubeadmCommands = []string{configureForKubeVIPScriptOnRemotePreKubeadmCommand}
+	postKubeadmCommands = []string{configureForKubeVIPScriptOnRemotePostKubeadmCommand}
+
+	return files, preKubeadmCommands, postKubeadmCommands, nil
 }
 
 type multipleKeysError struct {

pkg/handlers/generic/mutation/controlplanevirtualip/providers/kubevip_test.go

@@ -11,22 +11,69 @@ import (
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1"
 )
 
-func Test_GetFile(t *testing.T) {
+func Test_GenerateFilesAndCommands(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
-		name                     string
-		controlPlaneEndpointSpec v1alpha1.ControlPlaneEndpointSpec
-		configMap                *corev1.ConfigMap
-		expectedContent          string
-		expectedErr              error
+		name                        string
+		controlPlaneEndpointSpec    v1alpha1.ControlPlaneEndpointSpec
+		cluster                     *clusterv1.Cluster
+		configMap                   *corev1.ConfigMap
+		expectedFiles               []bootstrapv1.File
+		expectedPreKubeadmCommands  []string
+		expectedPostKubeadmCommands []string
+		expectedErr                 error
 	}{
+		{
+			name: "should return templated data with both host and port and pre/post kubeadm hack commands",
+			controlPlaneEndpointSpec: v1alpha1.ControlPlaneEndpointSpec{
+				Host: "10.20.100.10",
+				Port: 6443,
+			},
+			configMap: &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "default-kube-vip-template",
+					Namespace: "default",
+				},
+				Data: map[string]string{
+					"data": validKubeVIPTemplate,
+				},
+			},
+			cluster: &clusterv1.Cluster{
+				Spec: clusterv1.ClusterSpec{
+					Topology: &clusterv1.Topology{
+						Version: "v1.29.0",
+					},
+				},
+			},
+			expectedFiles: []bootstrapv1.File{
+				{
+					Content:     expectedKubeVIPPod,
+					Owner:       kubeVIPFileOwner,
+					Path:        kubeVIPFilePath,
+					Permissions: kubeVIPFilePermissions,
+				},
+				{
+					Content:     string(configureForKubeVIPScript),
+					Path:        configureForKubeVIPScriptOnRemote,
+					Permissions: configureForKubeVIPScriptPermissions,
+				},
+			},
+			expectedPreKubeadmCommands: []string{
+				configureForKubeVIPScriptOnRemotePreKubeadmCommand,
+			},
+			expectedPostKubeadmCommands: []string{
+				configureForKubeVIPScriptOnRemotePostKubeadmCommand,
+			},
+		},
 		{
 			name: "should return templated data with both host and port",
 			controlPlaneEndpointSpec: v1alpha1.ControlPlaneEndpointSpec{
@@ -42,7 +89,21 @@ func Test_GetFile(t *testing.T) {
 					"data": validKubeVIPTemplate,
 				},
 			},
-			expectedContent: expectedKubeVIPPod,
+			cluster: &clusterv1.Cluster{
+				Spec: clusterv1.ClusterSpec{
+					Topology: &clusterv1.Topology{
+						Version: "v1.28.0",
+					},
+				},
+			},
+			expectedFiles: []bootstrapv1.File{
+				{
+					Content:     expectedKubeVIPPod,
+					Owner:       kubeVIPFileOwner,
+					Path:        kubeVIPFilePath,
+					Permissions: kubeVIPFilePermissions,
+				},
+			},
 		},
 	}
 
@@ -57,12 +118,15 @@ func Test_GetFile(t *testing.T) {
 				configMapKey: client.ObjectKeyFromObject(tt.configMap),
 			}
 
-			file, err := provider.GetFile(context.TODO(), tt.controlPlaneEndpointSpec)
+			files, preKubeadmCommands, postKubeadmCommands, err := provider.GenerateFilesAndCommands(
+				context.TODO(),
+				tt.controlPlaneEndpointSpec,
+				tt.cluster,
+			)
 			require.Equal(t, tt.expectedErr, err)
-			assert.Equal(t, tt.expectedContent, file.Content)
-			assert.NotEmpty(t, file.Path)
-			assert.NotEmpty(t, file.Owner)
-			assert.NotEmpty(t, file.Permissions)
+			assert.Equal(t, tt.expectedFiles, files)
+			assert.Equal(t, tt.expectedPreKubeadmCommands, preKubeadmCommands)
+			assert.Equal(t, tt.expectedPostKubeadmCommands, postKubeadmCommands)
 		})
 	}
 }