Merge pull request #297 from averdagu/fix/osprh-6798

[OVNDBCluster] Check bound error on reconcileService

controllers/ovndbcluster_controller.go

@@ -719,7 +719,9 @@ func (r *OVNDBClusterReconciler) reconcileServices(
 	// cluster member so it can be resolved from outside cluster (edpm nodes)
 	if instance.Spec.NetworkAttachment != "" {
 		var dnsIPsList []string
-		for _, ovnPod := range podList.Items[:*(instance.Spec.Replicas)] {
+		// TODO(averdagu): use built in Min once go1.21 is used
+		minLen := ovn_common.Min(len(podList.Items), int(*(instance.Spec.Replicas)))
+		for _, ovnPod := range podList.Items[:minLen] {
 			svc, err = service.GetServiceWithName(
 				ctx,
 				helper,
@@ -737,7 +739,9 @@ func (r *OVNDBClusterReconciler) reconcileServices(
 			}
 
 		}
-		// Create DNSData CR
+		// 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.
+		// If nothing changed this won't modify the current dnsmasq pod.
 		err = ovndbcluster.DNSData(
 			ctx,
 			helper,
@@ -749,6 +753,14 @@ func (r *OVNDBClusterReconciler) reconcileServices(
 		if err != nil {
 			return ctrl.Result{}, err
 		}
+		// It can be possible that not all pods are ready, so DNSData won't
+		// have complete information, return error to retrigger reconcile loop
+		// Returning here instead of at the beggining of the for is done to
+		// expose the already created pods to other services/dataplane nodes
+		if len(podList.Items) < int(*(instance.Spec.Replicas)) {
+			err = fmt.Errorf("not all pods are yet created, number of expected pods: %v, current pods: %v", *(instance.Spec.Replicas), len(podList.Items))
+			return ctrl.Result{RequeueAfter: 1 * time.Second}, err
+		}
 	}
 	// dbAddress will contain ovsdbserver-(nb|sb).openstack.svc or empty
 	scheme := "tcp"

pkg/common/utils.go

@@ -0,0 +1,22 @@
+/*
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+    http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package common
+
+// Helper function while can't use buildin min
+// TOOD(averdagu) remove when using go1.21.0
+func Min(x, y int) int {
+	if x < y {
+		return x
+	}
+	return y
+}

tests/functional/ovndbcluster_controller_test.go

@@ -212,6 +212,47 @@ var _ = Describe("OVNDBCluster controller", func() {
 		})
 	})
 
+	When("OVNDBClusters are created with networkAttachments", func() {
+		It("does not break if pods are not created yet", func() {
+			// Create OVNDBCluster with 1 replica
+			spec := GetDefaultOVNDBClusterSpec()
+			spec.NetworkAttachment = "internalapi"
+			internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
+			nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
+			DeferCleanup(th.DeleteInstance, nad)
+			dbs := CreateOVNDBClusters(namespace, map[string][]string{namespace + "/internalapi": {"10.0.0.1"}}, 1)
+
+			// Increase replica to 3
+			Eventually(func(g Gomega) {
+				c := GetOVNDBCluster(dbs[0])
+				*c.Spec.Replicas = 3
+				g.Expect(k8sClient.Update(ctx, c)).Should(Succeed())
+			}).Should(Succeed())
+
+			//Check that error occurs
+			Eventually(func(g Gomega) {
+				conditions := GetOVNDBCluster(dbs[0]).Status.Conditions
+				cond := conditions.Get(condition.ExposeServiceReadyCondition)
+				g.Expect(cond.Status).To(Equal(corev1.ConditionFalse))
+			}).Should(Succeed())
+
+			// Decrease replicas back to 1
+			Eventually(func(g Gomega) {
+				c := GetOVNDBCluster(dbs[0])
+				*c.Spec.Replicas = 1
+				g.Expect(k8sClient.Update(ctx, c)).Should(Succeed())
+			}).Should(Succeed())
+
+			//Check that error doesn't happen and instance is ready
+			Eventually(func(g Gomega) {
+				conditions := GetOVNDBCluster(dbs[0]).Status.Conditions
+				cond := conditions.Get(condition.DeploymentReadyCondition)
+				g.Expect(cond.Status).To(Equal(corev1.ConditionTrue))
+			}).Should(Succeed())
+
+		})
+	})
+
 	When("OVNDBCluster is created with networkAttachments", func() {
 		var OVNDBClusterName types.NamespacedName
 		BeforeEach(func() {