Add unlock if bootstrap machine holding lock does not exist

Signed-off-by: killianmuldoon <[email protected]>
kubernetes-sigs · Dec 8, 2021 · 2cb1836 · 2cb1836
1 parent 891f585
commit 2cb1836
Show file tree

Hide file tree

Showing 2 changed files with 115 additions and 3 deletions.
diff --git a/bootstrap/kubeadm/internal/locking/control_plane_init_mutex.go b/bootstrap/kubeadm/internal/locking/control_plane_init_mutex.go
@@ -62,16 +62,25 @@ 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.
+		err = c.client.Get(ctx, client.ObjectKey{
+			Namespace: cluster.Namespace,
+			Name:      info.MachineName,
+		}, &clusterv1.Machine{})
+		if apierrors.IsNotFound(err) {
+			c.Unlock(ctx, cluster)
+		}
 		log.Info("Waiting on another machine to initialize", "init-machine", info.MachineName)
 		return false
 	}

diff --git a/bootstrap/kubeadm/internal/locking/control_plane_init_mutex_test.go b/bootstrap/kubeadm/internal/locking/control_plane_init_mutex_test.go
@@ -55,7 +55,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
@@ -99,6 +98,21 @@ func TestControlPlaneInitMutex_Lock(t *testing.T) {
 			},
 			shouldAcquire: false,
 		},
+		{
+			name: "should release the lock if the machine the 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(),
+			},
+			shouldAcquire: false,
+		},
 	}
 
 	for _, tc := range tests {
@@ -128,6 +142,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{