Merge pull request #5824 from killianmuldoon/fix/unlock-kubebootstrap

🐛  Add unlock mechanism to the kubeadm bootstrap provider

bootstrap/kubeadm/internal/locking/control_plane_init_mutex.go

@@ -63,16 +63,27 @@ func (c *ControlPlaneInitMutex) Lock(ctx context.Context, cluster *clusterv1.Clu
 	case err != nil:
 		log.Error(err, "Failed to acquire lock")
 		return false
-	default: // successfully found an existing config map
+	default: // Successfully found an existing config map.
 		info, err := sema.information()
 		if err != nil {
 			log.Error(err, "Failed to get information about the existing lock")
 			return false
 		}
-		// the machine requesting the lock is the machine that created the lock, therefore the lock is acquired
+		// The machine requesting the lock is the machine that created the lock, therefore the lock is acquired.
 		if info.MachineName == machine.Name {
 			return true
 		}
+
+		// If the machine that created the lock can not be found unlock the mutex.
+		if err := c.client.Get(ctx, client.ObjectKey{
+			Namespace: cluster.Namespace,
+			Name:      info.MachineName,
+		}, &clusterv1.Machine{}); err != nil {
+			log.Error(err, "Failed to get machine holding ControlPlane lock")
+			if apierrors.IsNotFound(err) {
+				c.Unlock(ctx, cluster)
+			}
+		}
 		log.Info("Waiting on another machine to initialize", "init-machine", info.MachineName)
 		return false
 	}

bootstrap/kubeadm/internal/locking/control_plane_init_mutex_test.go

@@ -56,7 +56,6 @@ func TestControlPlaneInitMutex_Lock(t *testing.T) {
 	g.Expect(corev1.AddToScheme(scheme)).To(Succeed())
 
 	uid := types.UID("test-uid")
-
 	tests := []struct {
 		name          string
 		client        client.Client
@@ -129,6 +128,95 @@ func TestControlPlaneInitMutex_Lock(t *testing.T) {
 		})
 	}
 }
+
+func TestControlPlaneInitMutex_LockWithMachineDeletion(t *testing.T) {
+	g := NewWithT(t)
+
+	scheme := runtime.NewScheme()
+	g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed())
+	g.Expect(corev1.AddToScheme(scheme)).To(Succeed())
+
+	newMachineName := "new-machine"
+	tests := []struct {
+		name                string
+		client              client.Client
+		expectedMachineName string
+	}{
+		{
+			name: "should not give the lock to new machine if the machine that created it does exist",
+			client: &fakeClient{
+				Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(
+					&corev1.ConfigMap{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      configMapName(clusterName),
+							Namespace: clusterNamespace},
+						Data: map[string]string{
+							"lock-information": "{\"machineName\":\"existent-machine\"}",
+						}},
+					&clusterv1.Machine{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "existent-machine",
+							Namespace: clusterNamespace,
+						},
+					},
+				).Build(),
+			},
+			expectedMachineName: "existent-machine",
+		},
+		{
+			name: "should give the lock to new machine if the machine that created it does not exist",
+			client: &fakeClient{
+				Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(
+					&corev1.ConfigMap{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      configMapName(clusterName),
+							Namespace: clusterNamespace},
+						Data: map[string]string{
+							"lock-information": "{\"machineName\":\"non-existent-machine\"}",
+						}},
+				).Build(),
+			},
+			expectedMachineName: newMachineName,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			l := &ControlPlaneInitMutex{
+				log:    log.Log,
+				client: tc.client,
+			}
+
+			cluster := &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: clusterNamespace,
+					Name:      clusterName,
+				},
+			}
+			machine := &clusterv1.Machine{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: newMachineName,
+				},
+			}
+
+			g.Eventually(func(g Gomega) error {
+				l.Lock(ctx, cluster, machine)
+
+				cm := &corev1.ConfigMap{}
+				g.Expect(tc.client.Get(ctx, client.ObjectKey{
+					Name:      configMapName(clusterName),
+					Namespace: cluster.Namespace,
+				}, cm)).To(Succeed())
+
+				info, err := semaphore{cm}.information()
+				g.Expect(err).To(BeNil())
+
+				g.Expect(info.MachineName).To(Equal(tc.expectedMachineName))
+				return nil
+			}, "20s").Should(Succeed())
+		})
+	}
+}
+
 func TestControlPlaneInitMutex_UnLock(t *testing.T) {
 	uid := types.UID("test-uid")
 	configMap := &corev1.ConfigMap{
@@ -217,7 +305,8 @@ func TestInfoLines_Lock(t *testing.T) {
 	}
 
 	logtester := &logtests{
-		InfoLog: make([]line, 0),
+		InfoLog:  make([]line, 0),
+		ErrorLog: make([]line, 0),
 	}
 	l := &ControlPlaneInitMutex{
 		log:    logr.New(logtester),
@@ -281,7 +370,8 @@ func (fc *fakeClient) Delete(ctx context.Context, obj client.Object, opts ...cli
 
 type logtests struct {
 	logr.Logger
-	InfoLog []line
+	InfoLog  []line
+	ErrorLog []line
 }
 
 type line struct {
@@ -306,6 +396,18 @@ func (l *logtests) Info(level int, msg string, keysAndValues ...interface{}) {
 		data: data,
 	})
 }
+
+func (l *logtests) Error(err error, msg string, keysAndValues ...interface{}) {
+	data := make(map[string]interface{})
+	for i := 0; i < len(keysAndValues); i += 2 {
+		data[keysAndValues[i].(string)] = keysAndValues[i+1]
+	}
+	l.ErrorLog = append(l.ErrorLog, line{
+		line: msg + err.Error(),
+		data: data,
+	})
+}
+
 func (l *logtests) WithValues(keysAndValues ...interface{}) logr.LogSink {
 	return l
 }