Merge pull request #234 from fabriziopandini/initialize-configobjects-if-missing

✨ Initialize JoinConfiguration if missing

Author: k8s-ci-robot

controllers/kubeadmconfig_controller.go

@@ -189,6 +189,8 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
 
 		log.Info("Creating BootstrapData for the init control plane")
 
+		// Nb. in this case JoinConfiguration should not be defined by users, but in case of misconfigurations, CABPK simply ignore it
+
 		// get both of ClusterConfiguration and InitConfiguration strings to pass to the cloud init control plane generator
 		// kubeadm allows one of these values to be empty; CABPK replace missing values with an empty config, so the cloud init generation
 		// should not handle special cases.
@@ -255,13 +257,18 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
 	}
 
 	// Every other case it's a join scenario
-	// Nb. in this case ClusterConfiguration and JoinConfiguration should not be defined by users, but in case of misconfigurations, CABPK simply ignore them
+	// Nb. in this case ClusterConfiguration and InitConfiguration should not be defined by users, but in case of misconfigurations, CABPK simply ignore them
 
 	// Unlock any locks that might have been set during init process
 	r.KubeadmInitLock.Unlock(ctx, cluster)
 
+	// if the JoinConfiguration is missing, create a default one
 	if config.Spec.JoinConfiguration == nil {
-		return ctrl.Result{}, errors.New("Control plane already exists for the cluster, only KubeadmConfig objects with JoinConfiguration are allowed")
+		log.Info("Creating default JoinConfiguration")
+		config.Spec.JoinConfiguration = &kubeadmv1beta1.JoinConfiguration{}
+		if util.IsControlPlaneMachine(machine) {
+			config.Spec.JoinConfiguration.ControlPlane = &kubeadmv1beta1.JoinControlPlane{}
+		}
 	}
 
 	certificates := internalcluster.NewCertificates()
@@ -305,6 +312,8 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
 			return ctrl.Result{}, errors.New("Machine is a ControlPlane, but JoinConfiguration.ControlPlane is not set in the KubeadmConfig object")
 		}
 
+		log.Info("Creating BootstrapData for the join control plane")
+
 		cloudJoinData, err := cloudinit.NewJoinControlPlane(&cloudinit.ControlPlaneJoinInput{
 			JoinConfiguration: joindata,
 			Certificates:      certificates,
@@ -331,6 +340,8 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
 		return ctrl.Result{}, errors.New("Machine is a Worker, but JoinConfiguration.ControlPlane is set in the KubeadmConfig object")
 	}
 
+	log.Info("Creating BootstrapData for the worker node")
+
 	cloudJoinData, err := cloudinit.NewNode(&cloudinit.NodeInput{
 		BaseUserData: cloudinit.BaseUserData{
 			AdditionalFiles:     config.Spec.Files,

controllers/kubeadmconfig_controller_test.go

@@ -400,42 +400,6 @@ func TestKubeadmConfigReconciler_Reconcile_GenerateCloudConfigData(t *testing.T)
 	}
 }
 
-// Return an error if a worker has no JoinConfiguration defined
-// TODO: This logic should not error in this case. A JoinConfiguration should be autogenerated
-func TestKubeadmConfigReconciler_Reconcile_ErrorIfAWorkerHasNoJoinConfigurationAndTheControlPlaneIsInitialized(t *testing.T) {
-	cluster := newCluster("cluster")
-	cluster.Status.InfrastructureReady = true
-	cluster.Status.ControlPlaneInitialized = true
-
-	workerMachine := newWorkerMachine(cluster)
-	workerJoinConfig := newWorkerJoinKubeadmConfig(workerMachine)
-	workerJoinConfig.Spec.JoinConfiguration = nil // Makes workerJoinConfig invalid
-
-	objects := []runtime.Object{
-		cluster,
-		workerMachine,
-		workerJoinConfig,
-	}
-	myclient := fake.NewFakeClientWithScheme(setupScheme(), objects...)
-
-	k := &KubeadmConfigReconciler{
-		Log:             log.Log,
-		Client:          myclient,
-		KubeadmInitLock: &myInitLocker{},
-	}
-
-	request := ctrl.Request{
-		NamespacedName: types.NamespacedName{
-			Namespace: "default",
-			Name:      "worker-join-cfg",
-		},
-	}
-	_, err := k.Reconcile(request)
-	if err == nil {
-		t.Fatal("Expected error, got nil")
-	}
-}
-
 // If a controlplane has an invalid JoinConfiguration then user intervention is required.
 func TestKubeadmConfigReconciler_Reconcile_ErrorIfJoiningControlPlaneHasInvalidConfiguration(t *testing.T) {
 	// TODO: extract this kind of code into a setup function that puts the state of objects into an initialized controlplane (implies secrets exist)
@@ -526,99 +490,101 @@ func TestReconcileIfJoinNodesAndControlPlaneIsReady(t *testing.T) {
 	cluster.Status.InfrastructureReady = true
 	cluster.Status.ControlPlaneInitialized = true
 	cluster.Status.APIEndpoints = []clusterv1.APIEndpoint{{Host: "100.105.150.1", Port: 6443}}
-	controlPlaneInitMachine := newControlPlaneMachine(cluster, "control-plane-init-machine")
-	initConfig := newControlPlaneInitKubeadmConfig(controlPlaneInitMachine, "control-plane-init-config")
-
-	workerMachine := newWorkerMachine(cluster)
-	workerJoinConfig := newWorkerJoinKubeadmConfig(workerMachine)
 
-	controlPlaneJoinMachine := newControlPlaneMachine(cluster, "control-plane-join-machine")
-	controlPlaneJoinConfig := newControlPlaneJoinKubeadmConfig(controlPlaneJoinMachine, "control-plane-join-cfg")
-
-	objects := []runtime.Object{
-		cluster,
-		workerMachine,
-		workerJoinConfig,
-		controlPlaneJoinMachine,
-		controlPlaneJoinConfig,
-	}
-	objects = append(objects, createSecrets(t, cluster, initConfig)...)
-	myclient := fake.NewFakeClientWithScheme(setupScheme(), objects...)
-	k := &KubeadmConfigReconciler{
-		Log:                  log.Log,
-		Client:               myclient,
-		SecretsClientFactory: newFakeSecretFactory(),
-		KubeadmInitLock:      &myInitLocker{},
-	}
-
-	request := ctrl.Request{
-		NamespacedName: types.NamespacedName{
-			Namespace: "default",
-			Name:      "worker-join-cfg",
+	var useCases = []struct {
+		name          string
+		machine       *clusterv1.Machine
+		configName    string
+		configBuilder func(*clusterv1.Machine, string) *bootstrapv1.KubeadmConfig
+	}{
+		{
+			name:       "Join a worker node with a fully compiled kubeadm config object",
+			machine:    newWorkerMachine(cluster),
+			configName: "worker-join-cfg",
+			configBuilder: func(machine *clusterv1.Machine, name string) *bootstrapv1.KubeadmConfig {
+				return newWorkerJoinKubeadmConfig(machine)
+			},
+		},
+		{
+			name:          "Join a worker node  with an empty kubeadm config object (defaults apply)",
+			machine:       newWorkerMachine(cluster),
+			configName:    "worker-join-cfg",
+			configBuilder: newKubeadmConfig,
+		},
+		{
+			name:          "Join a control plane node with a fully compiled kubeadm config object",
+			machine:       newControlPlaneMachine(cluster, "control-plane-join-machine"),
+			configName:    "control-plane-join-cfg",
+			configBuilder: newControlPlaneJoinKubeadmConfig,
+		},
+		{
+			name:          "Join a control plane node with an empty kubeadm config object (defaults apply)",
+			machine:       newControlPlaneMachine(cluster, "control-plane-join-machine"),
+			configName:    "control-plane-join-cfg",
+			configBuilder: newKubeadmConfig,
 		},
-	}
-	result, err := k.Reconcile(request)
-	if err != nil {
-		t.Fatalf("Failed to reconcile:\n %+v", err)
-	}
-	if result.Requeue == true {
-		t.Fatal("did not expect to requeue")
-	}
-	if result.RequeueAfter != time.Duration(0) {
-		t.Fatal("did not expect to requeue after")
 	}
 
-	cfg, err := getKubeadmConfig(myclient, "worker-join-cfg")
-	if err != nil {
-		t.Fatalf("Failed to reconcile:\n %+v", err)
-	}
+	for _, rt := range useCases {
+		rt := rt // pin!
+		t.Run(rt.name, func(t *testing.T) {
+			config := rt.configBuilder(rt.machine, rt.configName)
 
-	if cfg.Status.Ready != true {
-		t.Fatal("Expected status ready")
-	}
+			objects := []runtime.Object{
+				cluster,
+				rt.machine,
+				config,
+			}
+			objects = append(objects, createSecrets(t, cluster, config)...)
+			myclient := fake.NewFakeClientWithScheme(setupScheme(), objects...)
+			k := &KubeadmConfigReconciler{
+				Log:                  log.Log,
+				Client:               myclient,
+				SecretsClientFactory: newFakeSecretFactory(),
+				KubeadmInitLock:      &myInitLocker{},
+			}
 
-	if cfg.Status.BootstrapData == nil {
-		t.Fatal("Expected status ready")
-	}
+			request := ctrl.Request{
+				NamespacedName: types.NamespacedName{
+					Namespace: config.GetNamespace(),
+					Name:      rt.configName,
+				},
+			}
+			result, err := k.Reconcile(request)
+			if err != nil {
+				t.Fatal(fmt.Sprintf("Failed to reconcile:\n %+v", err))
+			}
+			if result.Requeue == true {
+				t.Fatal("did not expected to requeue")
+			}
+			if result.RequeueAfter != time.Duration(0) {
+				t.Fatal("did not expected to requeue after")
+			}
 
-	request = ctrl.Request{
-		NamespacedName: types.NamespacedName{
-			Namespace: "default",
-			Name:      "control-plane-join-cfg",
-		},
-	}
-	result, err = k.Reconcile(request)
-	if err != nil {
-		t.Fatalf("Failed to reconcile:\n %+v", err)
-	}
-	if result.Requeue == true {
-		t.Fatal("did not expect to requeue")
-	}
-	if result.RequeueAfter != time.Duration(0) {
-		t.Fatal("did not expect to requeue after")
-	}
+			cfg, err := getKubeadmConfig(myclient, rt.configName)
+			if err != nil {
+				t.Fatal(fmt.Sprintf("Failed to reconcile:\n %+v", err))
+			}
 
-	cfg, err = getKubeadmConfig(myclient, "control-plane-join-cfg")
-	if err != nil {
-		t.Fatalf("Failed to reconcile:\n %+v", err)
-	}
+			if cfg.Status.Ready != true {
+				t.Fatal("Expected status ready")
+			}
 
-	if cfg.Status.Ready != true {
-		t.Fatal("Expected status ready")
-	}
+			if cfg.Status.BootstrapData == nil {
+				t.Fatal("Expected status ready")
+			}
 
-	if cfg.Status.BootstrapData == nil {
-		t.Fatal("Expected status ready")
-	}
+			myremoteclient, _ := k.SecretsClientFactory.NewSecretsClient(nil, nil)
+			l, err := myremoteclient.List(metav1.ListOptions{})
+			if err != nil {
+				t.Fatal(fmt.Sprintf("Failed to get secrets after reconcyle:\n %+v", err))
+			}
 
-	myremoteclient, _ := k.SecretsClientFactory.NewSecretsClient(nil, nil)
-	l, err := myremoteclient.List(metav1.ListOptions{})
-	if err != nil {
-		t.Fatalf("Failed to reconcile:\n %+v", err)
-	}
+			if len(l.Items) != 1 {
+				t.Fatal("Failed to get bootstrap token secret")
+			}
+		})
 
-	if len(l.Items) != 2 {
-		t.Fatalf("Failed to reconcile:\n %+v", err)
 	}
 }