From 8e00d592df68911571e74277fa3aadad69ec024c Mon Sep 17 00:00:00 2001
From: Martin Schuppert <mschuppert@redhat.com>
Date: Mon, 15 Jan 2024 10:40:36 +0100
Subject: [PATCH] Fix osconfigenerator for Redis/OVNDBs vips

Special Redis on OVNDBs roles are not considered in the
configgenerator when rendering the template. As a result the
required hiera redis_vip was not set

Function lookup() did not find a value for the name 'redis_vip'

This updates the configgenerator to consider Redis and OVNDBs
and have the entries like the following in the rendered
parameter file:

~~~
  OVNDBsVirtualFixedIPs:
    - ip_address: 172.17.0.12
      use_neutron: false
  RedisVirtualFixedIPs:
    - ip_address: 172.17.0.11
      use_neutron: false
~~~
---
 .../openstackconfiggenerator_controller.go        |  7 ++++---
 pkg/openstackconfiggenerator/configmap.go         | 15 +++++++++++----
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/controllers/openstackconfiggenerator_controller.go b/controllers/openstackconfiggenerator_controller.go
index a91bf02f..d1c0a7ce 100644
--- a/controllers/openstackconfiggenerator_controller.go
+++ b/controllers/openstackconfiggenerator_controller.go
@@ -423,7 +423,7 @@ func (r *OpenStackConfigGeneratorReconciler) Reconcile(ctx context.Context, req
 		}
 
 		// check if osvmset and osbms is in status provisioned
-		msg, deployed, err := r.verifyNodeResourceStatus(ctx, instance)
+		msg, deployed, err := r.verifyNodeResourceStatus(ctx, instance, OSPVersion)
 		if err != nil {
 			return ctrl.Result{}, err
 		}
@@ -655,6 +655,7 @@ func (r *OpenStackConfigGeneratorReconciler) SetupWithManager(mgr ctrl.Manager)
 func (r *OpenStackConfigGeneratorReconciler) verifyNodeResourceStatus(
 	ctx context.Context,
 	instance *ospdirectorv1beta1.OpenStackConfigGenerator,
+	ospVersion shared.OSPVersion,
 ) (string, bool, error) {
 
 	msg := ""
@@ -669,7 +670,7 @@ func (r *OpenStackConfigGeneratorReconciler) verifyNodeResourceStatus(
 	}
 
 	for _, vmset := range vmsetList.Items {
-		if openstackconfiggenerator.IsRoleIncluded(vmset.Spec.RoleName, instance) {
+		if openstackconfiggenerator.IsRoleIncluded(vmset.Spec.RoleName, instance, ospVersion) {
 			if vmset.Status.ProvisioningStatus.State != shared.ProvisioningState(shared.VMSetCondTypeProvisioned) {
 				msg := fmt.Sprintf("Waiting on OpenStackVMset %s to be provisioned...", vmset.Name)
 				return msg, false, nil
@@ -690,7 +691,7 @@ func (r *OpenStackConfigGeneratorReconciler) verifyNodeResourceStatus(
 		//
 		// wait for all BMS be provisioned if baremetalhosts for the bms are requested
 		//
-		if openstackconfiggenerator.IsRoleIncluded(bmset.Spec.RoleName, instance) {
+		if openstackconfiggenerator.IsRoleIncluded(bmset.Spec.RoleName, instance, ospVersion) {
 			if bmset.Status.ProvisioningStatus.State != shared.ProvisioningState(shared.BaremetalSetCondTypeProvisioned) &&
 				bmset.Status.ProvisioningStatus.State != shared.ProvisioningState(shared.BaremetalSetCondTypeEmpty) {
 				msg := fmt.Sprintf("Waiting on OpenStackBaremetalSet %s to be provisioned...", bmset.Name)
diff --git a/pkg/openstackconfiggenerator/configmap.go b/pkg/openstackconfiggenerator/configmap.go
index 5365de55..a086574f 100644
--- a/pkg/openstackconfiggenerator/configmap.go
+++ b/pkg/openstackconfiggenerator/configmap.go
@@ -357,9 +357,16 @@ func createNetworksMap(
 }
 
 // IsRoleIncluded - checks if the role exists in the ConfigGenerator Roles set
-func IsRoleIncluded(roleName string, instance *ospdirectorv1beta1.OpenStackConfigGenerator) bool {
-
-	if len(instance.Spec.Roles) == 0 || roleName == controlplane.Role {
+func IsRoleIncluded(roleName string, instance *ospdirectorv1beta1.OpenStackConfigGenerator, ospVersion shared.OSPVersion) bool {
+
+	// return for special conditions:
+	// - if there is no specific role specified in the spec
+	// - the roleName is one of the special roles for allocated VIPs in ipam for
+	//   - ControlPlane
+	//   - Redis (OSP17.1)
+	//   - OVNDPs (OSP17.1)
+	if len(instance.Spec.Roles) == 0 || roleName == controlplane.Role ||
+		(ospVersion == shared.OSPVersion(shared.TemplateVersion17_1) && (roleName == "Redis" || roleName == "OVNDBs")) {
 		return true
 	}
 	for _, r := range instance.Spec.Roles {
@@ -397,7 +404,7 @@ func createRolesMap(
 				roleOveride, roleOverrideFound = instance.Spec.TripleoRoleOverride[roleName]
 			}
 
-			if IsRoleIncluded(roleName, instance) || (roleOverrideFound && IsRoleIncluded(roleOveride.RoleName, instance)) {
+			if IsRoleIncluded(roleName, instance, ospVersion) || (roleOverrideFound && IsRoleIncluded(roleOveride.RoleName, instance, ospVersion)) {
 				//
 				// check if role is VM
 				//