Merge pull request #2480 from k8s-infra-cherrypick-robot/cherry-pick-2478-to-release-0.12

[release-0.12] Don't set OSMachine Ready until all config is complete

Author: k8s-ci-robot

controllers/openstackmachine_controller.go

@@ -365,11 +365,6 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope
 		return ctrl.Result{}, err
 	}
 
-	result := r.reconcileMachineState(scope, openStackMachine, machine, machineServer)
-	if result != nil {
-		return *result, nil
-	}
-
 	computeService, err := compute.NewService(scope)
 	if err != nil {
 		return ctrl.Result{}, err
@@ -383,9 +378,9 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope
 	}
 
 	if instanceStatus == nil {
-		msg := "server has been unexpectedly deleted"
-		conditions.MarkFalse(openStackMachine, infrav1.InstanceReadyCondition, infrav1.InstanceDeletedReason, clusterv1.ConditionSeverityError, msg)
-		openStackMachine.SetFailure(capoerrors.DeprecatedCAPIUpdateMachineError, errors.New(msg))
+		conditions.MarkFalse(openStackMachine, infrav1.InstanceReadyCondition, infrav1.InstanceDeletedReason, clusterv1.ConditionSeverityError, infrav1.ServerUnexpectedDeletedMessage)
+		openStackMachine.SetFailure(capoerrors.DeprecatedCAPIUpdateMachineError, errors.New(infrav1.ServerUnexpectedDeletedMessage)) //nolint:stylecheck // This error is not used as an error
+		return ctrl.Result{}, nil
 	}
 
 	instanceNS, err := instanceStatus.NetworkStatus()
@@ -413,6 +408,11 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope
 		conditions.MarkTrue(openStackMachine, infrav1.APIServerIngressReadyCondition)
 	}
 
+	result := r.reconcileMachineState(scope, openStackMachine, machine, machineServer)
+	if result != nil {
+		return *result, nil
+	}
+
 	scope.Logger().Info("Reconciled Machine create successfully")
 	return ctrl.Result{}, nil
 }

test/e2e/suites/e2e/e2e_test.go

@@ -56,6 +56,8 @@ import (
 	crclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
+	"sigs.k8s.io/cluster-api-provider-openstack/internal/util/ssa"
+	"sigs.k8s.io/cluster-api-provider-openstack/pkg/generated/applyconfiguration/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-openstack/test/e2e/shared"
 )
 
@@ -1014,20 +1016,48 @@ var _ = Describe("e2e tests [PR-Blocking]", func() {
 			serverToDelete := allServers[controlPlaneMachines[0].Spec.InfrastructureRef.Name]
 			err = shared.DeleteOpenStackServer(ctx, e2eCtx, serverToDelete.ID)
 			Expect(err).NotTo(HaveOccurred())
+
 			shared.Logf("Waiting for the OpenStackMachine to have a condition that the server has been unexpectedly deleted")
-			Eventually(func() bool {
+			retries := 0
+			Eventually(func() (clusterv1.Condition, error) {
+				k8sClient := e2eCtx.Environment.BootstrapClusterProxy.GetClient()
+
 				openStackMachine := &infrav1.OpenStackMachine{}
-				err := e2eCtx.Environment.BootstrapClusterProxy.GetClient().Get(ctx, crclient.ObjectKey{Name: controlPlaneMachines[0].Name, Namespace: controlPlaneMachines[0].Namespace}, openStackMachine)
+				err := k8sClient.Get(ctx, crclient.ObjectKey{Name: controlPlaneMachines[0].Name, Namespace: controlPlaneMachines[0].Namespace}, openStackMachine)
 				if err != nil {
-					return false
+					return clusterv1.Condition{}, err
 				}
 				for _, condition := range openStackMachine.Status.Conditions {
-					if condition.Type == infrav1.InstanceReadyCondition && condition.Status == corev1.ConditionFalse && condition.Reason == infrav1.InstanceDeletedReason && condition.Message == "server has been unexpectedly deleted" {
-						return true
+					if condition.Type == infrav1.InstanceReadyCondition {
+						return condition, nil
 					}
 				}
-				return false
-			}, e2eCtx.E2EConfig.GetIntervals(specName, "wait-delete-machine")...).Should(BeTrue())
+
+				// Make some non-functional change to the object which will
+				// cause CAPO to reconcile it, otherwise we won't notice the
+				// server is gone until the configured controller-runtime
+				// resync.
+				retries++
+				applyConfig := v1beta1.OpenStackMachine(openStackMachine.Name, openStackMachine.Namespace).
+					WithAnnotations(map[string]string{
+						"e2e-test-retries": fmt.Sprintf("%d", retries),
+					})
+				err = k8sClient.Patch(ctx, openStackMachine, ssa.ApplyConfigPatch(applyConfig), crclient.ForceOwnership, crclient.FieldOwner("capo-e2e"))
+				if err != nil {
+					return clusterv1.Condition{}, err
+				}
+
+				return clusterv1.Condition{}, errors.New("condition InstanceReadyCondition not found")
+			}, time.Minute*3, time.Second*10).Should(MatchFields(
+				IgnoreExtras,
+				Fields{
+					"Type":     Equal(infrav1.InstanceReadyCondition),
+					"Status":   Equal(corev1.ConditionFalse),
+					"Reason":   Equal(infrav1.InstanceDeletedReason),
+					"Message":  Equal(infrav1.ServerUnexpectedDeletedMessage),
+					"Severity": Equal(clusterv1.ConditionSeverityError),
+				},
+			), "OpenStackMachine should be marked not ready with InstanceDeletedReason")
 		})
 	})
 })