fix: use provided NodeRegistrationOptions as base (#823)

* fix: use provided NodeRegistrationOptions as base * test: add case for SetNodeRegistrationOptions with non-empty base
kubernetes-sigs · Jun 11, 2019 · b1d5924 · b1d5924
1 parent 77da568
commit b1d5924
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 11 deletions.
diff --git a/pkg/cloud/aws/services/ec2/instances.go b/pkg/cloud/aws/services/ec2/instances.go
@@ -221,7 +221,8 @@ func (s *Service) createInstance(machine *actuators.MachineScope, bootstrapToken
 					),
 				),
 				kubeadm.WithJoinNodeRegistrationOptions(
-					kubeadm.NewNodeRegistration(
+					kubeadm.SetNodeRegistrationOptions(
+						&machine.MachineConfig.KubeadmConfiguration.Join.NodeRegistration,
 						kubeadm.WithTaints(machine.GetMachine().Spec.Taints),
 						kubeadm.WithNodeRegistrationName(hostnameLookup),
 						kubeadm.WithCRISocket(containerdSocket),
@@ -275,14 +276,16 @@ func (s *Service) createInstance(machine *actuators.MachineScope, bootstrapToken
 			kubeadm.SetInitConfigurationOptions(
 				&machine.MachineConfig.KubeadmConfiguration.Init,
 				kubeadm.WithNodeRegistrationOptions(
-					kubeadm.NewNodeRegistration(
+					kubeadm.SetNodeRegistrationOptions(
+						&machine.MachineConfig.KubeadmConfiguration.Init.NodeRegistration,
 						kubeadm.WithTaints(machine.GetMachine().Spec.Taints),
 						kubeadm.WithNodeRegistrationName(hostnameLookup),
 						kubeadm.WithCRISocket(containerdSocket),
 						kubeadm.WithKubeletExtraArgs(map[string]string{"cloud-provider": cloudProvider}),
 					),
 				),
 			)
+
 			initConfigYAML, err := kubeadm.ConfigurationToYAML(&machine.MachineConfig.KubeadmConfiguration.Init)
 			if err != nil {
 				return nil, err
@@ -320,7 +323,8 @@ func (s *Service) createInstance(machine *actuators.MachineScope, bootstrapToken
 				),
 			),
 			kubeadm.WithJoinNodeRegistrationOptions(
-				kubeadm.NewNodeRegistration(
+				kubeadm.SetNodeRegistrationOptions(
+					&machine.MachineConfig.KubeadmConfiguration.Join.NodeRegistration,
 					kubeadm.WithNodeRegistrationName(hostnameLookup),
 					kubeadm.WithCRISocket(containerdSocket),
 					kubeadm.WithKubeletExtraArgs(map[string]string{"cloud-provider": cloudProvider}),

diff --git a/pkg/cloud/aws/services/kubeadm/configs_test.go b/pkg/cloud/aws/services/kubeadm/configs_test.go
@@ -39,7 +39,7 @@ func TestInitConfiguration(t *testing.T) {
 			},
 			options: []kubeadm.InitConfigurationOption{
 				kubeadm.WithNodeRegistrationOptions(
-					kubeadm.NewNodeRegistration(),
+					kubeadm.SetNodeRegistrationOptions(&kubeadmv1beta1.NodeRegistrationOptions{}),
 				),
 			},
 		},

diff --git a/pkg/cloud/aws/services/kubeadm/noderegistration.go b/pkg/cloud/aws/services/kubeadm/noderegistration.go
@@ -26,13 +26,12 @@ import (
 // NodeRegistrationOption is a function that sets a value on a Kubeadm NodeRegistrationOptions.
 type NodeRegistrationOption func(*kubeadmv1beta1.NodeRegistrationOptions)
 
-// NewNodeRegistration will create a kubeadmNodeRegistrationOptions with some defaults and the ability to provide customizations.
-func NewNodeRegistration(opts ...NodeRegistrationOption) kubeadmv1beta1.NodeRegistrationOptions {
-	nro := &kubeadmv1beta1.NodeRegistrationOptions{}
+// SetNodeRegistrationOptions will take a kubeadmNodeRegistrationOptions and apply customizations.
+func SetNodeRegistrationOptions(base *kubeadmv1beta1.NodeRegistrationOptions, opts ...NodeRegistrationOption) kubeadmv1beta1.NodeRegistrationOptions {
 	for _, opt := range opts {
-		opt(nro)
+		opt(base)
 	}
-	return *nro
+	return *base
 }
 
 // WithTaints will set the taints on the NodeRegistration.

diff --git a/pkg/cloud/aws/services/kubeadm/noderegistration_test.go b/pkg/cloud/aws/services/kubeadm/noderegistration_test.go
@@ -48,7 +48,8 @@ func TestNewNodeRegistration(t *testing.T) {
 					},
 				},
 			},
-			actual: kubeadm.NewNodeRegistration(
+			actual: kubeadm.SetNodeRegistrationOptions(
+				&kubeadmv1beta1.NodeRegistrationOptions{},
 				kubeadm.WithNodeRegistrationName("test name"),
 				kubeadm.WithCRISocket("/test/path/to/socket.sock"),
 				kubeadm.WithTaints([]corev1.Taint{
@@ -68,11 +69,31 @@ func TestNewNodeRegistration(t *testing.T) {
 					"node-labels": "test value one,test value two",
 				},
 			},
-			actual: kubeadm.NewNodeRegistration(
+			actual: kubeadm.SetNodeRegistrationOptions(
+				&kubeadmv1beta1.NodeRegistrationOptions{},
 				kubeadm.WithKubeletExtraArgs(map[string]string{"node-labels": "test value one"}),
 				kubeadm.WithKubeletExtraArgs(map[string]string{"node-labels": "test value two"}),
 			),
 		},
+		{
+			name: "test starting with non-empty base",
+			expected: kubeadmv1beta1.NodeRegistrationOptions{
+				CRISocket: "/test/path/to/socket.sock",
+				KubeletExtraArgs: map[string]string{
+					"cni-bin-dir":  "/opt/cni/bin",
+					"cni-conf-dir": "/etc/cni/net.d",
+				},
+			},
+			actual: kubeadm.SetNodeRegistrationOptions(
+				&kubeadmv1beta1.NodeRegistrationOptions{
+					KubeletExtraArgs: map[string]string{
+						"cni-bin-dir":  "/opt/cni/bin",
+						"cni-conf-dir": "/etc/cni/net.d",
+					},
+				},
+				kubeadm.WithCRISocket("/test/path/to/socket.sock"),
+			),
+		},
 	}
 
 	for _, tc := range testcases {