Move runtime config reconcilation post service setup

We need to wait for ovn cluster to be ready and that
relies on pods to be in ready state. And for multi replica
ovn cluster the pods needs to communicate with each other
using dns addresses and those are provided by the headless
service.

So moving the runtime config task post setting up the
services so cluster can be formed and pods move to the ready state.
This issue was catched in the backup/restore exercise [2].

Also adapted getPodIPInNetwork to set networkattachment condition
and accordingly the envtest.

[1] #540
[2] https://github.com/openstack-k8s-operators/dev-docs/blob/main/backup-restore/user-guide.md

Related-Issue: #OSPRH-27985
Signed-off-by: Yatin Karel <ykarel@redhat.com>

Author: karelyatin

internal/controller/ovndbcluster_controller.go

@@ -659,26 +659,31 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *
 		return ctrlResult, nil
 	}
 
+	// create Statefulset - end
+	// Handle service init
+	ctrlResult, err = r.reconcileServices(ctx, instance, helper, serviceLabels, serviceName)
+	if err != nil {
+		instance.Status.Conditions.Set(condition.FalseCondition(
+			condition.ExposeServiceReadyCondition,
+			condition.ErrorReason,
+			condition.SeverityWarning,
+			condition.ExposeServiceReadyErrorMessage,
+			err.Error()))
+		return ctrlResult, err
+	} else if (ctrlResult != ctrl.Result{}) {
+		instance.Status.Conditions.Set(condition.FalseCondition(
+			condition.ExposeServiceReadyCondition,
+			condition.RequestedReason,
+			condition.SeverityInfo,
+			condition.ExposeServiceReadyRunningMessage))
+		return ctrlResult, err
+	}
+
 	stateful := sfset.GetStatefulSet()
 	if stateful.Generation == stateful.Status.ObservedGeneration {
 		instance.Status.ReadyCount = stateful.Status.ReadyReplicas
 	}
 
-	// Only run runtime config when all pods are ready and no rolling update in progress
-	if instance.Status.ReadyCount == *instance.Spec.Replicas &&
-		stateful.Status.UpdatedReplicas == *instance.Spec.Replicas {
-		err = r.reconcileRuntimeConfig(ctx, helper, instance, serviceLabels, serviceName)
-		if err != nil {
-			instance.Status.Conditions.Set(condition.FalseCondition(
-				condition.ServiceConfigReadyCondition,
-				condition.ErrorReason,
-				condition.SeverityWarning,
-				condition.ServiceConfigReadyErrorMessage,
-				err.Error()))
-			return ctrl.Result{}, err
-		}
-	}
-
 	// verify if network attachment matches expectations
 	networkReady, networkAttachmentStatus, err := nad.VerifyNetworkStatusFromAnnotation(ctx, helper, networkAttachments, serviceLabels, instance.Status.ReadyCount)
 	if err != nil {
@@ -700,26 +705,6 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *
 		return ctrlResult, nil
 	}
 
-	// create Statefulset - end
-	// Handle service init
-	ctrlResult, err = r.reconcileServices(ctx, instance, helper, serviceLabels, serviceName)
-	if err != nil {
-		instance.Status.Conditions.Set(condition.FalseCondition(
-			condition.ExposeServiceReadyCondition,
-			condition.ErrorReason,
-			condition.SeverityWarning,
-			condition.ExposeServiceReadyErrorMessage,
-			err.Error()))
-		return ctrlResult, err
-	} else if (ctrlResult != ctrl.Result{}) {
-		instance.Status.Conditions.Set(condition.FalseCondition(
-			condition.ExposeServiceReadyCondition,
-			condition.RequestedReason,
-			condition.SeverityInfo,
-			condition.ExposeServiceReadyRunningMessage))
-		return ctrlResult, err
-	}
-
 	svcList, err := service.GetServicesListWithLabel(
 		ctx,
 		helper,
@@ -736,6 +721,21 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *
 		return ctrl.Result{}, err
 	}
 
+	// Only run runtime config when all pods are ready and no rolling update in progress
+	if instance.Status.ReadyCount == *instance.Spec.Replicas &&
+		stateful.Status.UpdatedReplicas == *instance.Spec.Replicas {
+		err = r.reconcileRuntimeConfig(ctx, helper, instance, serviceLabels, serviceName)
+		if err != nil {
+			instance.Status.Conditions.Set(condition.FalseCondition(
+				condition.ServiceConfigReadyCondition,
+				condition.ErrorReason,
+				condition.SeverityWarning,
+				condition.ServiceConfigReadyErrorMessage,
+				err.Error()))
+			return ctrl.Result{}, err
+		}
+	}
+
 	if statefulset.IsReady(stateful) && len(svcList.Items) > 0 {
 		instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage)
 		instance.Status.Conditions.MarkTrue(condition.ExposeServiceReadyCondition, condition.ExposeServiceReadyMessage)
@@ -797,7 +797,9 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *
 	return ctrl.Result{}, nil
 }
 
-func getPodIPInNetwork(ovnPod corev1.Pod, namespace string, networkAttachment string) (string, error) {
+func getPodIPInNetwork(ovnPod corev1.Pod, instance *ovnv1.OVNDBCluster) (string, error) {
+	namespace := instance.Namespace
+	networkAttachment := instance.Spec.NetworkAttachment
 	netStat, err := nad.GetNetworkStatusFromAnnotation(ovnPod.Annotations)
 	if err != nil {
 		err = fmt.Errorf("error while getting the Network Status for pod %s: %w", ovnPod.Name, err)
@@ -812,6 +814,12 @@ func getPodIPInNetwork(ovnPod corev1.Pod, namespace string, networkAttachment st
 	}
 	// If this is reached it means that no IP was found, construct error and return
 	err = fmt.Errorf("%w IP address from pod %s in network %s, IP is empty", util.ErrInvalidStatus, ovnPod.Name, networkAttachment)
+	instance.Status.Conditions.Set(condition.FalseCondition(
+		condition.NetworkAttachmentsReadyCondition,
+		condition.ErrorReason,
+		condition.SeverityWarning,
+		condition.NetworkAttachmentsReadyErrorMessage,
+		err.Error()))
 	return "", err
 }
 
@@ -1005,11 +1013,11 @@ func (r *OVNDBClusterReconciler) reconcileServices(
 				return ctrl.Result{}, err
 			}
 
-			dnsIP, err := getPodIPInNetwork(ovnPod, instance.Namespace, instance.Spec.NetworkAttachment)
-			dnsIPsList = append(dnsIPsList, dnsIP)
+			dnsIP, err := getPodIPInNetwork(ovnPod, instance)
 			if err != nil {
 				return ctrl.Result{}, err
 			}
+			dnsIPsList = append(dnsIPsList, dnsIP)
 		}
 		// DNSData info is called every reconcile loop to ensure that even if a pod gets
 		// restarted and it's IP has changed, the DNSData CR will have the correct info.

test/functional/ovndbcluster_controller_test.go

@@ -837,7 +837,7 @@ var _ = Describe("OVNDBCluster controller", func() {
 				corev1.ConditionFalse,
 				condition.ErrorReason,
 				"NetworkAttachments error occurred "+
-					"not all pods have interfaces with ips as configured in NetworkAttachments: internalapi",
+					"invalid status IP address from pod ovsdbserver-sb-0 in network internalapi, IP is empty",
 			)
 		})
 		It("reports that an IP is missing", func() {
@@ -877,7 +877,7 @@ var _ = Describe("OVNDBCluster controller", func() {
 				corev1.ConditionFalse,
 				condition.ErrorReason,
 				"NetworkAttachments error occurred "+
-					"not all pods have interfaces with ips as configured in NetworkAttachments: internalapi",
+					"invalid status IP address from pod ovsdbserver-sb-0 in network internalapi, IP is empty",
 			)
 		})
 		It("reports NetworkAttachmentsReady if the Pods got the proper annotations", func() {