Merge pull request #352 from averdagu/fix/OSPRH-10372

Move external config map generation to OVNDBCluster

api/v1beta1/client.go

@@ -52,6 +52,26 @@ func getDBClusters(
 	return ovnDBList, nil
 }
 
+func GetOVNController(
+	ctx context.Context,
+	h *helper.Helper,
+	namespace string,
+) (*OVNController, error) {
+	ovnControllerList := &OVNControllerList{}
+	listOpts := []client.ListOption{
+		client.InNamespace(namespace),
+	}
+	err := h.GetClient().List(ctx, ovnControllerList, listOpts...)
+	if err != nil {
+		return nil, err
+	}
+	if len(ovnControllerList.Items) > 0 {
+		return &ovnControllerList.Items[0], nil
+	}
+
+	return nil, nil
+}
+
 // GetDBClusterByType - return OVNDBCluster for the given dbType
 func GetDBClusterByType(
 	ctx context.Context,
@@ -87,8 +107,8 @@ func getItems(list client.ObjectList) []client.Object {
 	return items
 }
 
-// OVNDBClusterNamespaceMapFunc - DBCluster Watch Function
-func OVNDBClusterNamespaceMapFunc(crs client.ObjectList, reader client.Reader) handler.MapFunc {
+// OVNCRNamespaceMapFunc // Generic function to watch any OVN CR
+func OVNCRNamespaceMapFunc(crs client.ObjectList, reader client.Reader) handler.MapFunc {
 	return func(ctx context.Context, obj client.Object) []reconcile.Request {
 		log := log.FromContext(ctx)
 		result := []reconcile.Request{}

config/rbac/role.yaml

@@ -143,6 +143,12 @@ rules:
   - patch
   - update
   - watch
+- apiGroups:
+  - ovn.openstack.org
+  resources:
+  - ovncontroller
+  verbs:
+  - watch
 - apiGroups:
   - ovn.openstack.org
   resources:

controllers/ovncontroller_controller.go

@@ -25,7 +25,6 @@ import (
 	"github.com/go-logr/logr"
 	netattdefv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
 	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -227,7 +226,7 @@ func (r *OVNControllerReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Owns(&corev1.ServiceAccount{}).
 		Owns(&rbacv1.Role{}).
 		Owns(&rbacv1.RoleBinding{}).
-		Watches(&ovnv1.OVNDBCluster{}, handler.EnqueueRequestsFromMapFunc(ovnv1.OVNDBClusterNamespaceMapFunc(crs, mgr.GetClient()))).
+		Watches(&ovnv1.OVNDBCluster{}, handler.EnqueueRequestsFromMapFunc(ovnv1.OVNCRNamespaceMapFunc(crs, mgr.GetClient()))).
 		Watches(
 			&corev1.Secret{},
 			handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
@@ -601,35 +600,10 @@ func (r *OVNControllerReconciler) reconcileNormal(ctx context.Context, instance
 
 	sbCluster, err := ovnv1.GetDBClusterByType(ctx, helper, instance.Namespace, map[string]string{}, ovnv1.SBDBType)
 	if err != nil {
-		Log.Info("No SB OVNDBCluster defined, deleting external ConfigMap")
-		cleanupConfigMapErr := r.deleteExternalConfigMaps(ctx, helper, instance)
-		if cleanupConfigMapErr != nil {
-			Log.Error(cleanupConfigMapErr, "Failed to delete external ConfigMap")
-			return ctrl.Result{}, cleanupConfigMapErr
-		}
+		Log.Info("No SB OVNDBCluster defined. Exiting reconcile.")
 		return ctrl.Result{}, nil
 	}
 
-	ep, err := sbCluster.GetExternalEndpoint()
-	if err != nil || ep == "" {
-		Log.Info("No external endpoint defined for SB OVNDBCluster, deleting external ConfigMap")
-		cleanupConfigMapErr := r.deleteExternalConfigMaps(ctx, helper, instance)
-		if cleanupConfigMapErr != nil {
-			Log.Error(cleanupConfigMapErr, "Failed to delete external ConfigMap")
-			return ctrl.Result{}, cleanupConfigMapErr
-		}
-	}
-
-	if sbCluster.Spec.NetworkAttachment != "" {
-		// Create ConfigMap for external dataplane consumption
-		// TODO(ihar) - is there any hashing mechanism for EDP config? do we trigger deploy somehow?
-		err = r.generateExternalConfigMaps(ctx, helper, instance, sbCluster, &configMapVars)
-		if err != nil {
-			Log.Error(err, "Failed to generate external ConfigMap")
-			return ctrl.Result{}, err
-		}
-	}
-
 	// create OVN Config Job - start
 	// Waits for OVS pods to run the configJob which basically will set config into OVS database
 	if instance.Status.OVSNumberReady != instance.Status.DesiredNumberScheduled {
@@ -719,61 +693,6 @@ func (r *OVNControllerReconciler) generateServiceConfigMaps(
 	return configmap.EnsureConfigMaps(ctx, h, instance, cms, envVars)
 }
 
-// generateExternalConfigMaps - create configmaps for external dataplane consumption
-func (r *OVNControllerReconciler) generateExternalConfigMaps(
-	ctx context.Context,
-	h *helper.Helper,
-	instance *ovnv1.OVNController,
-	sbCluster *ovnv1.OVNDBCluster,
-	envVars *map[string]env.Setter,
-) error {
-	// Create/update configmaps from templates
-	cmLabels := labels.GetLabels(instance, labels.GetGroupLabel(ovnv1.ServiceNameOVNController), map[string]string{})
-
-	externalEndpoint, err := sbCluster.GetExternalEndpoint()
-	if err != nil {
-		return err
-	}
-
-	externalTemplateParameters := make(map[string]interface{})
-	externalTemplateParameters["OVNRemote"] = externalEndpoint
-	externalTemplateParameters["OVNEncapType"] = instance.Spec.ExternalIDS.OvnEncapType
-
-	cms := []util.Template{
-		// EDP ConfigMap
-		{
-			Name:          fmt.Sprintf("%s-config", instance.Name),
-			Namespace:     instance.Namespace,
-			Type:          util.TemplateTypeConfig,
-			InstanceType:  instance.Kind,
-			Labels:        cmLabels,
-			ConfigOptions: externalTemplateParameters,
-		},
-	}
-	return configmap.EnsureConfigMaps(ctx, h, instance, cms, envVars)
-}
-
-// TODO(ihar) this function could live in lib-common
-// deleteExternalConfigMaps - delete obsolete configmaps for external dataplane consumption
-func (r *OVNControllerReconciler) deleteExternalConfigMaps(
-	ctx context.Context,
-	h *helper.Helper,
-	instance *ovnv1.OVNController,
-) error {
-	cm := &corev1.ConfigMap{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      fmt.Sprintf("%s-config", instance.Name),
-			Namespace: instance.Namespace,
-		},
-	}
-
-	err := h.GetClient().Delete(ctx, cm)
-	if err != nil && !k8s_errors.IsNotFound(err) {
-		return fmt.Errorf("error deleting external config map %s: %w", cm.Name, err)
-	}
-	return nil
-}
-
 // createHashOfInputHashes - creates a hash of hashes which gets added to the resources which requires a restart
 // if any of the input resources change, like configs, passwords, ...
 //

controllers/ovndbcluster_controller.go

@@ -23,6 +23,7 @@ import (
 	"time"
 
 	"github.com/go-logr/logr"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -86,6 +87,7 @@ func (r *OVNDBClusterReconciler) GetLogger(ctx context.Context) logr.Logger {
 }
 
 //+kubebuilder:rbac:groups=ovn.openstack.org,resources=ovndbclusters,verbs=get;list;watch;create;update;patch;delete
+//+kubebuilder:rbac:groups=ovn.openstack.org,resources=ovncontroller,verbs=watch;
 //+kubebuilder:rbac:groups=ovn.openstack.org,resources=ovndbclusters/status,verbs=get;update;patch
 //+kubebuilder:rbac:groups=ovn.openstack.org,resources=ovndbclusters/finalizers,verbs=update;patch
 //+kubebuilder:rbac:groups=core,resources=configmaps,verbs=get;list;watch;create;update;patch;delete;
@@ -205,6 +207,7 @@ func (r *OVNDBClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request
 
 // SetupWithManager sets up the controller with the Manager.
 func (r *OVNDBClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	crs := &ovnv1.OVNDBClusterList{}
 	// index caBundleSecretNameField
 	if err := mgr.GetFieldIndexer().IndexField(context.Background(), &ovnv1.OVNDBCluster{}, caBundleSecretNameField, func(rawObj client.Object) []string {
 		// Extract the secret name from the spec, if one is provided
@@ -238,6 +241,7 @@ func (r *OVNDBClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Owns(&rbacv1.Role{}).
 		Owns(&rbacv1.RoleBinding{}).
 		Owns(&infranetworkv1.DNSData{}).
+		Watches(&ovnv1.OVNController{}, handler.EnqueueRequestsFromMapFunc(ovnv1.OVNCRNamespaceMapFunc(crs, mgr.GetClient()))).
 		Watches(
 			&corev1.Secret{},
 			handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
@@ -374,6 +378,15 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *
 				err.Error()))
 			return ctrl.Result{}, err
 		}
+	} else if instance.Spec.DBType == ovnv1.SBDBType {
+		// This config map was created by the SB and it only needs to be deleted once
+		// since this reconcile loop can be done by the SB and the NB, filtering so only
+		// one deletes it.
+		Log.Info("NetworkAttachment is empty, deleting external config map")
+		err = r.deleteExternalConfigMaps(ctx, helper, instance.Namespace)
+		if err != nil {
+			return ctrl.Result{}, err
+		}
 	}
 
 	serviceAnnotations, err := nad.CreateNetworksAnnotation(instance.Namespace, networkAttachments)
@@ -612,6 +625,16 @@ func (r *OVNDBClusterReconciler) reconcileNormal(ctx context.Context, instance *
 
 		// Set DB Address
 		instance.Status.InternalDBAddress = strings.Join(internalDbAddress, ",")
+		if instance.Spec.DBType == ovnv1.SBDBType && instance.Spec.NetworkAttachment != "" {
+			// This config map will populate the sb db address to edpm, can't use the nb
+			// If there's no networkAttachments the configMap is not needed
+			configMapVars := make(map[string]env.Setter)
+			err = r.generateExternalConfigMaps(ctx, helper, instance, serviceName, &configMapVars)
+			if err != nil {
+				Log.Info(fmt.Sprintf("Error while generating external config map: %v", err))
+			}
+		}
+
 	}
 	Log.Info("Reconciled Service successfully")
 	return ctrl.Result{}, nil
@@ -786,6 +809,68 @@ func (r *OVNDBClusterReconciler) reconcileServices(
 	return ctrl.Result{}, nil
 }
 
+// generateServiceConfigMaps - create create configmaps which hold service configuration
+func (r *OVNDBClusterReconciler) generateExternalConfigMaps(
+	ctx context.Context,
+	h *helper.Helper,
+	instance *ovnv1.OVNDBCluster,
+	serviceName string,
+	envVars *map[string]env.Setter,
+) error {
+	// Create/update configmaps from templates
+	cmLabels := labels.GetLabels(instance, labels.GetGroupLabel(serviceName), map[string]string{})
+	log := r.GetLogger(ctx)
+
+	externalEndpoint, err := instance.GetExternalEndpoint()
+	if err != nil {
+		return err
+	}
+
+	externalTemplateParameters := make(map[string]interface{})
+	externalTemplateParameters["OVNRemote"] = externalEndpoint
+
+	ovnController, err := ovnv1.GetOVNController(ctx, h, instance.Namespace)
+	if err != nil {
+		log.Info(fmt.Sprintf("Error on getting OVNController: %v", err))
+		return err
+	}
+	if ovnController != nil {
+		externalTemplateParameters["OVNEncapType"] = ovnController.Spec.ExternalIDS.OvnEncapType
+	}
+
+	cms := []util.Template{
+		// EDP ConfigMap
+		{
+			Name:          "ovncontroller-config",
+			Namespace:     instance.Namespace,
+			Type:          util.TemplateTypeConfig,
+			InstanceType:  instance.Kind,
+			Labels:        cmLabels,
+			ConfigOptions: externalTemplateParameters,
+		},
+	}
+	return configmap.EnsureConfigMaps(ctx, h, instance, cms, envVars)
+}
+
+func (r *OVNDBClusterReconciler) deleteExternalConfigMaps(
+	ctx context.Context,
+	h *helper.Helper,
+	namespace string,
+) error {
+	cm := &corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "ovncontroller-config",
+			Namespace: namespace,
+		},
+	}
+
+	err := h.GetClient().Delete(ctx, cm)
+	if err != nil && !k8s_errors.IsNotFound(err) {
+		return fmt.Errorf("error deleting external config map %s: %w", cm.Name, err)
+	}
+	return nil
+}
+
 // generateServiceConfigMaps - create create configmaps which hold service configuration
 func (r *OVNDBClusterReconciler) generateServiceConfigMaps(
 	ctx context.Context,

controllers/ovnnorthd_controller.go

@@ -212,7 +212,7 @@ func (r *OVNNorthdReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Owns(&corev1.ServiceAccount{}).
 		Owns(&rbacv1.Role{}).
 		Owns(&rbacv1.RoleBinding{}).
-		Watches(&ovnv1.OVNDBCluster{}, handler.EnqueueRequestsFromMapFunc(ovnv1.OVNDBClusterNamespaceMapFunc(crs, mgr.GetClient()))).
+		Watches(&ovnv1.OVNDBCluster{}, handler.EnqueueRequestsFromMapFunc(ovnv1.OVNCRNamespaceMapFunc(crs, mgr.GetClient()))).
 		Watches(
 			&corev1.Secret{},
 			handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),

templates/ovncontroller/config/ovsdb-config → templates/ovndbcluster/config/ovsdb-config

@@ -1,2 +1,4 @@
 ovn-remote: {{ .OVNRemote }}
+{{if (index . "OVNEncapType")}}
 ovn-encap-type: {{ .OVNEncapType }}
+{{end}}

tests/functional/base_test.go

@@ -235,6 +235,10 @@ func CreateOVNController(namespace string, spec ovnv1.OVNControllerSpec) client.
 	return ovn.GetOVNController(name)
 }
 
+func DeleteOVNController(instance types.NamespacedName) {
+	th.DeleteInstance(ovn.GetOVNController(instance))
+}
+
 func GetOVNController(name types.NamespacedName) *ovnv1.OVNController {
 	return ovn.GetOVNController(name)
 }