Skip to content

Commit e8f5fd1

Browse files
dkoshkindkoshkin
authored
refactor: kube-vip commands (#699)
**What problem does this PR solve?**: This PR refactors the kube-vip handler to: 1. Use a script file when running preKubeadmCommands/postKubeadmCommands. IMO this is simpler to maintain and change in the future instead of looking for complex multi-line strings in the slices. 2. Move `echo "127.0.0.1 kubernetes" >>/etc/hosts` out of the template and into the handler since this command is not unique to Nutanix and would be needed for other infra providers using kube-vip. **Which issue(s) this PR fixes**: Fixes # **How Has This Been Tested?**: <!-- Please describe the tests that you ran to verify your changes. Provide output from the tests and any manual steps needed to replicate the tests. --> **Special notes for your reviewer**: <!-- Use this to provide any additional information to the reviewers. This may include: - Best way to review the PR. - Where the author wants the most review attention on. - etc. -->
1 parent 6f6d921 commit e8f5fd1

File tree

9 files changed

+232
-71
lines changed

9 files changed

+232
-71
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,6 @@ spec:
153153
- hostnamectl set-hostname "{{ ds.meta_data.hostname }}"
154154
- echo "::1 ipv6-localhost ipv6-loopback" >/etc/hosts
155155
- echo "127.0.0.1 localhost" >>/etc/hosts
156-
- echo "127.0.0.1 kubernetes" >>/etc/hosts
157156
- echo "127.0.0.1 {{ ds.meta_data.hostname }}" >> /etc/hosts
158157
useExperimentalRetryJoin: true
159158
verbosity: 10

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,12 @@ patches:
6363
- target:
6464
kind: KubeadmControlPlaneTemplate
6565
patch: |-
66+
# deletes 'echo "127.0.0.1 kubernetes" >>/etc/hosts'
6667
- op: "remove"
67-
path: "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands/6"
68+
path: "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands/4"
69+
# deletes 'sed -i 's#path: /etc/kubernetes/admin.conf#path: ...'
70+
- op: "remove"
71+
path: "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands/5"
6872
- op: "remove"
6973
path: "/spec/template/spec/kubeadmConfigSpec/postKubeadmCommands/1"
7074

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

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -134,12 +134,13 @@ func (h *ControlPlaneVirtualIP) Mutate(
134134
selectors.ControlPlane(),
135135
log,
136136
func(obj *controlplanev1.KubeadmControlPlaneTemplate) error {
137-
virtualIPProviderFile, getFileErr := virtualIPProvider.GetFile(
137+
files, preKubeadmCommands, postKubeadmCommands, generateErr := virtualIPProvider.GenerateFilesAndCommands(
138138
ctx,
139139
controlPlaneEndpointVar,
140+
cluster,
140141
)
141-
if getFileErr != nil {
142-
return getFileErr
142+
if generateErr != nil {
143+
return generateErr
143144
}
144145

145146
log.WithValues(
@@ -151,16 +152,9 @@ func (h *ControlPlaneVirtualIP) Mutate(
151152
))
152153
obj.Spec.Template.Spec.KubeadmConfigSpec.Files = append(
153154
obj.Spec.Template.Spec.KubeadmConfigSpec.Files,
154-
*virtualIPProviderFile,
155+
files...,
155156
)
156157

157-
preKubeadmCommands, postKubeadmCommands, getCommandsErr := virtualIPProvider.GetCommands(
158-
cluster,
159-
)
160-
if getCommandsErr != nil {
161-
return getCommandsErr
162-
}
163-
164158
if len(preKubeadmCommands) > 0 {
165159
log.WithValues(
166160
"patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(),

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/handlers/mutation"
2222
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/testutils/capitest"
2323
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/testutils/capitest/request"
24-
virtuialipproviders "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/controlplanevirtualip/providers"
2524
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/options"
2625
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/test/helpers"
2726
)
@@ -83,14 +82,14 @@ var _ = Describe("Generate ControlPlane virtual IP patches", func() {
8382
Operation: "add",
8483
Path: "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands",
8584
ValueMatcher: gomega.ContainElements(
86-
virtuialipproviders.KubeVIPPreKubeadmCommands,
85+
"/bin/bash /etc/caren/configure-for-kube-vip.sh set-host-aliases use-super-admin.conf",
8786
),
8887
},
8988
{
9089
Operation: "add",
9190
Path: "/spec/template/spec/kubeadmConfigSpec/postKubeadmCommands",
9291
ValueMatcher: gomega.ContainElements(
93-
virtuialipproviders.KubeVIPPostKubeadmCommands,
92+
"/bin/bash /etc/caren/configure-for-kube-vip.sh use-admin.conf",
9493
),
9594
},
9695
},
@@ -148,20 +147,28 @@ var _ = Describe("Generate ControlPlane virtual IP patches", func() {
148147
),
149148
gomega.HaveKey("permissions"),
150149
),
150+
gomega.SatisfyAll(
151+
gomega.HaveKey("content"),
152+
gomega.HaveKeyWithValue(
153+
"path",
154+
gomega.ContainSubstring("configure-for-kube-vip.sh"),
155+
),
156+
gomega.HaveKey("permissions"),
157+
),
151158
),
152159
},
153160
{
154161
Operation: "add",
155162
Path: "/spec/template/spec/kubeadmConfigSpec/preKubeadmCommands",
156163
ValueMatcher: gomega.ContainElements(
157-
virtuialipproviders.KubeVIPPreKubeadmCommands,
164+
"/bin/bash /etc/caren/configure-for-kube-vip.sh set-host-aliases use-super-admin.conf",
158165
),
159166
},
160167
{
161168
Operation: "add",
162169
Path: "/spec/template/spec/kubeadmConfigSpec/postKubeadmCommands",
163170
ValueMatcher: gomega.ContainElements(
164-
virtuialipproviders.KubeVIPPostKubeadmCommands,
171+
"/bin/bash /etc/caren/configure-for-kube-vip.sh use-admin.conf",
165172
),
166173
},
167174
},

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

Lines changed: 65 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package providers
55

66
import (
77
"context"
8+
_ "embed"
89
"fmt"
910

1011
"github.com/blang/semver/v4"
@@ -14,19 +15,30 @@ import (
1415
"sigs.k8s.io/controller-runtime/pkg/client"
1516

1617
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1"
18+
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/common"
19+
)
20+
21+
const (
22+
kubeVIPFileOwner = "root:root"
23+
kubeVIPFilePath = "/etc/kubernetes/manifests/kube-vip.yaml"
24+
kubeVIPFilePermissions = "0600"
25+
26+
configureForKubeVIPScriptPermissions = "0700"
1727
)
1828

1929
var (
20-
//nolint:lll // for readability prefer to keep the long line
21-
KubeVIPPreKubeadmCommands = []string{`if [ -f /run/kubeadm/kubeadm.yaml ]; then
22-
sed -i 's#path: /etc/kubernetes/admin.conf#path: /etc/kubernetes/super-admin.conf#' /etc/kubernetes/manifests/kube-vip.yaml;
23-
fi`}
24-
//nolint:lll // for readability prefer to keep the long line
25-
KubeVIPPostKubeadmCommands = []string{`if [ -f /run/kubeadm/kubeadm.yaml ]; then
26-
sed -i 's#path: /etc/kubernetes/super-admin.conf#path: /etc/kubernetes/admin.conf#' /etc/kubernetes/manifests/kube-vip.yaml;
27-
fi`}
30+
configureForKubeVIPScriptOnRemote = common.ConfigFilePathOnRemote(
31+
"configure-for-kube-vip.sh")
32+
33+
configureForKubeVIPScriptOnRemotePreKubeadmCommand = "/bin/bash " +
34+
configureForKubeVIPScriptOnRemote + " set-host-aliases use-super-admin.conf"
35+
configureForKubeVIPScriptOnRemotePostKubeadmCommand = "/bin/bash " +
36+
configureForKubeVIPScriptOnRemote + " use-admin.conf"
2837
)
2938

39+
//go:embed templates/configure-for-kube-vip.sh
40+
var configureForKubeVIPScript []byte
41+
3042
type kubeVIPFromConfigMapProvider struct {
3143
client client.Reader
3244

@@ -50,35 +62,34 @@ func (p *kubeVIPFromConfigMapProvider) Name() string {
5062
return "kube-vip"
5163
}
5264

53-
// GetFile reads the kube-vip template from the ConfigMap
54-
// and returns the content a File, templating the required variables.
55-
func (p *kubeVIPFromConfigMapProvider) GetFile(
65+
// GenerateFilesAndCommands returns files and pre/post kubeadm commands for kube-vip.
66+
// It reads kube-vip template from a ConfigMap and returns the content a File, templating the required variables.
67+
// If required, it also returns a script file and pre/post kubeadm commands to change the kube-vip Pod to use the new
68+
// super-admin.conf file.
69+
func (p *kubeVIPFromConfigMapProvider) GenerateFilesAndCommands(
5670
ctx context.Context,
5771
spec v1alpha1.ControlPlaneEndpointSpec,
58-
) (*bootstrapv1.File, error) {
72+
cluster *clusterv1.Cluster,
73+
) (files []bootstrapv1.File, preKubeadmCommands, postKubeadmCommands []string, err error) {
5974
data, err := getTemplateFromConfigMap(ctx, p.client, p.configMapKey)
6075
if err != nil {
61-
return nil, fmt.Errorf("failed getting template data: %w", err)
76+
return nil, nil, nil, fmt.Errorf("failed getting template data: %w", err)
6277
}
6378

6479
kubeVIPStaticPod, err := templateValues(spec, data)
6580
if err != nil {
66-
return nil, fmt.Errorf("failed templating static Pod: %w", err)
81+
return nil, nil, nil, fmt.Errorf("failed templating static Pod: %w", err)
6782
}
6883

69-
return &bootstrapv1.File{
70-
Content: kubeVIPStaticPod,
71-
Owner: kubeVIPFileOwner,
72-
Path: kubeVIPFilePath,
73-
Permissions: kubeVIPFilePermissions,
74-
}, nil
75-
}
84+
files = []bootstrapv1.File{
85+
{
86+
Content: kubeVIPStaticPod,
87+
Owner: kubeVIPFileOwner,
88+
Path: kubeVIPFilePath,
89+
Permissions: kubeVIPFilePermissions,
90+
},
91+
}
7692

77-
//
78-
//nolint:gocritic // No need for named return values
79-
func (p *kubeVIPFromConfigMapProvider) GetCommands(
80-
cluster *clusterv1.Cluster,
81-
) ([]string, []string, error) {
8293
// The kube-vip static Pod uses admin.conf on the host to connect to the API server.
8394
// But, starting with Kubernetes 1.29, admin.conf first gets created with no RBAC permissions.
8495
// 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(
89100
// after kubeadm has assigned it the necessary RBAC permissions.
90101
//
91102
// See https://github.com/kube-vip/kube-vip/issues/684
103+
//
104+
// There is also another issue introduced in Kubernetes 1.29.
105+
// If a cloud provider did not yet initialise the node's .status.addresses,
106+
// the code for creating the /etc/hosts file including the hostAliases does not get run.
107+
// The kube-vip static Pod runs before the cloud provider and will not be able to resolve the kubernetes DNS name.
108+
// To work around this:
109+
// 1. return a preKubeadmCommand to add kubernetes DNS name to /etc/hosts.
110+
//
111+
// See https://github.com/kube-vip/kube-vip/issues/692
112+
// See https://github.com/kubernetes/kubernetes/issues/122420#issuecomment-1864609518
92113
needCommands, err := needHackCommands(cluster)
93114
if err != nil {
94-
return nil, nil, fmt.Errorf("failed to determine if kube-vip commands are needed: %w", err)
115+
return nil, nil, nil, fmt.Errorf(
116+
"failed to determine if kube-vip commands are needed: %w",
117+
err,
118+
)
95119
}
96120
if !needCommands {
97-
return nil, nil, nil
121+
return files, nil, nil, nil
98122
}
99123

100-
return KubeVIPPreKubeadmCommands, KubeVIPPostKubeadmCommands, nil
124+
files = append(
125+
files,
126+
bootstrapv1.File{
127+
Content: string(configureForKubeVIPScript),
128+
Path: configureForKubeVIPScriptOnRemote,
129+
Permissions: configureForKubeVIPScriptPermissions,
130+
},
131+
)
132+
133+
preKubeadmCommands = []string{configureForKubeVIPScriptOnRemotePreKubeadmCommand}
134+
postKubeadmCommands = []string{configureForKubeVIPScriptOnRemotePostKubeadmCommand}
135+
136+
return files, preKubeadmCommands, postKubeadmCommands, nil
101137
}
102138

103139
type multipleKeysError struct {

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

Lines changed: 76 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,22 +11,69 @@ import (
1111
"github.com/stretchr/testify/require"
1212
corev1 "k8s.io/api/core/v1"
1313
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
14+
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
15+
bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
1416
"sigs.k8s.io/controller-runtime/pkg/client"
1517
"sigs.k8s.io/controller-runtime/pkg/client/fake"
1618

1719
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1"
1820
)
1921

20-
func Test_GetFile(t *testing.T) {
22+
func Test_GenerateFilesAndCommands(t *testing.T) {
2123
t.Parallel()
2224

2325
tests := []struct {
24-
name string
25-
controlPlaneEndpointSpec v1alpha1.ControlPlaneEndpointSpec
26-
configMap *corev1.ConfigMap
27-
expectedContent string
28-
expectedErr error
26+
name string
27+
controlPlaneEndpointSpec v1alpha1.ControlPlaneEndpointSpec
28+
cluster *clusterv1.Cluster
29+
configMap *corev1.ConfigMap
30+
expectedFiles []bootstrapv1.File
31+
expectedPreKubeadmCommands []string
32+
expectedPostKubeadmCommands []string
33+
expectedErr error
2934
}{
35+
{
36+
name: "should return templated data with both host and port and pre/post kubeadm hack commands",
37+
controlPlaneEndpointSpec: v1alpha1.ControlPlaneEndpointSpec{
38+
Host: "10.20.100.10",
39+
Port: 6443,
40+
},
41+
configMap: &corev1.ConfigMap{
42+
ObjectMeta: metav1.ObjectMeta{
43+
Name: "default-kube-vip-template",
44+
Namespace: "default",
45+
},
46+
Data: map[string]string{
47+
"data": validKubeVIPTemplate,
48+
},
49+
},
50+
cluster: &clusterv1.Cluster{
51+
Spec: clusterv1.ClusterSpec{
52+
Topology: &clusterv1.Topology{
53+
Version: "v1.29.0",
54+
},
55+
},
56+
},
57+
expectedFiles: []bootstrapv1.File{
58+
{
59+
Content: expectedKubeVIPPod,
60+
Owner: kubeVIPFileOwner,
61+
Path: kubeVIPFilePath,
62+
Permissions: kubeVIPFilePermissions,
63+
},
64+
{
65+
Content: string(configureForKubeVIPScript),
66+
Path: configureForKubeVIPScriptOnRemote,
67+
Permissions: configureForKubeVIPScriptPermissions,
68+
},
69+
},
70+
expectedPreKubeadmCommands: []string{
71+
configureForKubeVIPScriptOnRemotePreKubeadmCommand,
72+
},
73+
expectedPostKubeadmCommands: []string{
74+
configureForKubeVIPScriptOnRemotePostKubeadmCommand,
75+
},
76+
},
3077
{
3178
name: "should return templated data with both host and port",
3279
controlPlaneEndpointSpec: v1alpha1.ControlPlaneEndpointSpec{
@@ -42,7 +89,21 @@ func Test_GetFile(t *testing.T) {
4289
"data": validKubeVIPTemplate,
4390
},
4491
},
45-
expectedContent: expectedKubeVIPPod,
92+
cluster: &clusterv1.Cluster{
93+
Spec: clusterv1.ClusterSpec{
94+
Topology: &clusterv1.Topology{
95+
Version: "v1.28.0",
96+
},
97+
},
98+
},
99+
expectedFiles: []bootstrapv1.File{
100+
{
101+
Content: expectedKubeVIPPod,
102+
Owner: kubeVIPFileOwner,
103+
Path: kubeVIPFilePath,
104+
Permissions: kubeVIPFilePermissions,
105+
},
106+
},
46107
},
47108
}
48109

@@ -57,12 +118,15 @@ func Test_GetFile(t *testing.T) {
57118
configMapKey: client.ObjectKeyFromObject(tt.configMap),
58119
}
59120

60-
file, err := provider.GetFile(context.TODO(), tt.controlPlaneEndpointSpec)
121+
files, preKubeadmCommands, postKubeadmCommands, err := provider.GenerateFilesAndCommands(
122+
context.TODO(),
123+
tt.controlPlaneEndpointSpec,
124+
tt.cluster,
125+
)
61126
require.Equal(t, tt.expectedErr, err)
62-
assert.Equal(t, tt.expectedContent, file.Content)
63-
assert.NotEmpty(t, file.Path)
64-
assert.NotEmpty(t, file.Owner)
65-
assert.NotEmpty(t, file.Permissions)
127+
assert.Equal(t, tt.expectedFiles, files)
128+
assert.Equal(t, tt.expectedPreKubeadmCommands, preKubeadmCommands)
129+
assert.Equal(t, tt.expectedPostKubeadmCommands, postKubeadmCommands)
66130
})
67131
}
68132
}

0 commit comments

Comments
 (0)