Merge pull request #11838 from chrischdi/pr-fix-node-drain-volumes

🌱 e2e: ensure node-drain with real volume detachments can get deleted without race conditions

Author: k8s-ci-robot

test/e2e/node_drain.go

@@ -93,8 +93,9 @@ type NodeDrainTimeoutSpecInput struct {
 // * Verify Node drains for control plane and MachineDeployment Machines are blocked by WaitCompleted Pods
 // * Force deleting the WaitCompleted Pods
 // * Verify Node drains for control plane and MachineDeployment Machines are blocked by PDBs
-// * Set NodeDrainTimeout to 1s to unblock Node drain
+// * Delete the unevictable pod PDBs
 // * Verify machine deletion is blocked by waiting for volume detachment (only if VerifyNodeVolumeDetach is enabled)
+// * Set NodeDrainTimeout to 1s to unblock Node drain
 // * Unblocks waiting for volume detachment (only if VerifyNodeVolumeDetach is enabled)
 // * Verify scale down succeeded because Node drains were unblocked.
 func NodeDrainTimeoutSpec(ctx context.Context, inputGetter func() NodeDrainTimeoutSpecInput) {
@@ -526,26 +527,19 @@ func NodeDrainTimeoutSpec(ctx context.Context, inputGetter func() NodeDrainTimeo
 			}, input.E2EConfig.GetIntervals(specName, "wait-machine-deleted")...).Should(Succeed())
 		}
 
-		By("Set NodeDrainTimeout to 1s to unblock Node drain")
-		// Note: This also verifies that KCP & MachineDeployments are still propagating changes to NodeDrainTimeout down to
-		// Machines that already have a deletionTimestamp.
-		drainTimeout := &metav1.Duration{Duration: time.Duration(1) * time.Second}
-		modifyControlPlaneViaClusterAndWait(ctx, modifyControlPlaneViaClusterAndWaitInput{
-			ClusterProxy: input.BootstrapClusterProxy,
-			Cluster:      cluster,
-			ModifyControlPlaneTopology: func(topology *clusterv1.ControlPlaneTopology) {
-				topology.NodeDrainTimeout = drainTimeout
-			},
-			WaitForControlPlane: input.E2EConfig.GetIntervals(specName, "wait-control-plane"),
-		})
-		modifyMachineDeploymentViaClusterAndWait(ctx, modifyMachineDeploymentViaClusterAndWaitInput{
-			ClusterProxy: input.BootstrapClusterProxy,
-			Cluster:      cluster,
-			ModifyMachineDeploymentTopology: func(topology *clusterv1.MachineDeploymentTopology) {
-				topology.NodeDrainTimeout = drainTimeout
-			},
-			WaitForMachineDeployments: input.E2EConfig.GetIntervals(specName, "wait-worker-nodes"),
+		By("Delete PDB for all unevictable pods to let drain succeed")
+		framework.DeletePodDisruptionBudget(ctx, framework.DeletePodDisruptionBudgetInput{
+			ClientSet: workloadClusterProxy.GetClientSet(),
+			Budget:    cpDeploymentWithPDBName(),
+			Namespace: "unevictable-workload",
 		})
+		for _, md := range machineDeployments {
+			framework.DeletePodDisruptionBudget(ctx, framework.DeletePodDisruptionBudgetInput{
+				ClientSet: workloadClusterProxy.GetClientSet(),
+				Budget:    mdDeploymentWithPDBName(md.Name),
+				Namespace: "unevictable-workload",
+			})
+		}
 
 		if input.VerifyNodeVolumeDetach {
 			By("Verify Node removal for control plane and MachineDeployment Machines are blocked (only by volume detachments)")
@@ -575,6 +569,21 @@ func NodeDrainTimeoutSpec(ctx context.Context, inputGetter func() NodeDrainTimeo
 			input.UnblockNodeVolumeDetachment(ctx, input.BootstrapClusterProxy, cluster)
 		}
 
+		// Set NodeDrainTimeout and NodeVolumeDetachTimeout to let the second ControlPlane Node get deleted without requiring manual intervention.
+		By("Set NodeDrainTimeout and NodeVolumeDetachTimeout for ControlPlanes to 1s to unblock Node drain")
+		// Note: This also verifies that KCP & MachineDeployments are still propagating changes to NodeDrainTimeout down to
+		// Machines that already have a deletionTimestamp.
+		drainTimeout := &metav1.Duration{Duration: time.Duration(1) * time.Second}
+		modifyControlPlaneViaClusterAndWait(ctx, modifyControlPlaneViaClusterAndWaitInput{
+			ClusterProxy: input.BootstrapClusterProxy,
+			Cluster:      cluster,
+			ModifyControlPlaneTopology: func(topology *clusterv1.ControlPlaneTopology) {
+				topology.NodeDrainTimeout = drainTimeout
+				topology.NodeVolumeDetachTimeout = drainTimeout
+			},
+			WaitForControlPlane: input.E2EConfig.GetIntervals(specName, "wait-control-plane"),
+		})
+
 		By("Verify scale down succeeded because Node drains and Volume detachments were unblocked")
 		// When we scale down the KCP, controlplane machines are deleted one by one, so it requires more time
 		// MD Machine deletion is done in parallel and will be faster.

test/framework/deployment_helpers.go

@@ -725,3 +725,19 @@ func AddPodDisruptionBudget(ctx context.Context, input AddPodDisruptionBudgetInp
 		return fmt.Errorf("podDisruptionBudget needs to be successfully deployed: %v", err)
 	}, retryableOperationTimeout, retryableOperationInterval).Should(Succeed(), "podDisruptionBudget needs to be successfully deployed")
 }
+
+type DeletePodDisruptionBudgetInput struct {
+	ClientSet *kubernetes.Clientset
+	Budget    string
+	Namespace string
+}
+
+func DeletePodDisruptionBudget(ctx context.Context, input DeletePodDisruptionBudgetInput) {
+	Eventually(func() error {
+		err := input.ClientSet.PolicyV1().PodDisruptionBudgets(input.Namespace).Delete(ctx, input.Budget, metav1.DeleteOptions{})
+		if apierrors.IsNotFound(err) || err == nil {
+			return nil
+		}
+		return fmt.Errorf("podDisruptionBudget needs to be deleted: %v", err)
+	}, retryableOperationTimeout, retryableOperationInterval).Should(Succeed(), "podDisruptionBudget needs to be deleted")
+}