Rearrange method and remove unnecessary code

Also fixes a bind configuration syntax bug.
openstack-k8s-operators · Jan 21, 2025 · 58412a6 · 58412a6
1 parent bce3dab
commit 58412a6
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 178 deletions.
diff --git a/controllers/designate_controller.go b/controllers/designate_controller.go
@@ -760,6 +760,8 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des
 		return ctrl.Result{}, err
 	}
 
+	// While nsRecordsConfigMap is not used until much later in this function, it's valuable to check this early
+	// as it is an important precondition.
 	nsRecordsLabels := labels.GetLabels(instance, labels.GetGroupLabel(instance.ObjectMeta.Name), map[string]string{})
 	nsRecordsConfigMap, err := r.handleConfigMap(ctx, helper, instance, designate.NsRecordsConfigMap, nsRecordsLabels)
 	if err != nil {
@@ -827,76 +829,6 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des
 		return ctrl.Result{}, err
 	}
 
-	if err != nil {
-		return ctrl.Result{}, err
-	}
-	if len(nsRecordsConfigMap.Data) > 0 {
-		poolsYamlConfigMap := &corev1.ConfigMap{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      designate.PoolsYamlConfigMap,
-				Namespace: instance.GetNamespace(),
-				Labels:    bindLabels,
-			},
-			Data: make(map[string]string),
-		}
-		poolsYaml, err := designate.GeneratePoolsYamlData(bindConfigMap.Data, mdnsConfigMap.Data, nsRecordsConfigMap.Data)
-		if err != nil {
-			return ctrl.Result{}, err
-		}
-		Log.Info(fmt.Sprintf("pools.yaml content is\n%v", poolsYaml))
-		updatedPoolsYaml := make(map[string]string)
-		updatedPoolsYaml[designate.PoolsYamlContent] = poolsYaml
-
-		_, err = controllerutil.CreateOrPatch(ctx, helper.GetClient(), poolsYamlConfigMap, func() error {
-			poolsYamlConfigMap.Labels = util.MergeStringMaps(poolsYamlConfigMap.Labels, bindLabels)
-			poolsYamlConfigMap.Data = updatedPoolsYaml
-			return controllerutil.SetControllerReference(instance, poolsYamlConfigMap, helper.GetScheme())
-		})
-		if err != nil {
-			Log.Info("Unable to create config map for pools.yaml file")
-			return ctrl.Result{}, err
-		}
-		configMaps := []interface{}{
-			poolsYamlConfigMap.Data,
-		}
-
-		poolsYamlsEnvVars := make(map[string]env.Setter)
-		_, changed, err := r.createHashOfInputHashes(ctx, instance, designate.PoolsYamlHash, poolsYamlsEnvVars, configMaps)
-		if err != nil {
-			return ctrl.Result{}, err
-		}
-		if changed {
-			Log.Info("PoolsYamlHash has changed, creating a pool update job")
-
-			var poolUpdateHash string
-			var ok bool
-			if poolUpdateHash, ok = instance.Status.Hash[designatev1beta1.PoolUpdateHash]; !ok {
-				instance.Status.Hash[designatev1beta1.PoolUpdateHash] = ""
-				poolUpdateHash = ""
-			}
-			jobDef := designate.PoolUpdateJob(instance, serviceLabels, serviceAnnotations)
-
-			Log.Info("Initializing pool update job")
-			poolUpdatejob := job.NewJob(
-				jobDef,
-				designatev1beta1.PoolUpdateHash,
-				instance.Spec.PreserveJobs,
-				time.Duration(15)*time.Second,
-				poolUpdateHash,
-			)
-			_, err = poolUpdatejob.DoJob(ctx, helper)
-			if err != nil {
-				return ctrl.Result{}, err
-			}
-			instance.Status.Hash[designatev1beta1.PoolUpdateHash] = poolUpdatejob.GetHash()
-			err = r.Client.Status().Update(ctx, instance)
-			if err != nil {
-				return ctrl.Result{}, err
-			}
-			Log.Info("Pool update job completed successfully")
-		}
-	}
-
 	// deploy designate-central
 	designateCentral, op, err := r.centralDeploymentCreateOrUpdate(ctx, instance)
 	if err != nil {
@@ -1143,6 +1075,76 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des
 	}
 	Log.Info("Deployment Unbound task reconciled")
 
+	if len(nsRecordsConfigMap.Data) > 0 {
+		poolsYamlConfigMap := &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      designate.PoolsYamlConfigMap,
+				Namespace: instance.GetNamespace(),
+				Labels:    bindLabels,
+			},
+			Data: make(map[string]string),
+		}
+		poolsYaml, err := designate.GeneratePoolsYamlData(bindConfigMap.Data, mdnsConfigMap.Data, nsRecordsConfigMap.Data)
+		if err != nil {
+			Log.Error(err, "Pool data creation failed")
+			return ctrl.Result{}, err
+		}
+		Log.Info(fmt.Sprintf("pools.yaml content is\n%v", poolsYaml))
+		updatedPoolsYaml := make(map[string]string)
+		updatedPoolsYaml[designate.PoolsYamlContent] = poolsYaml
+
+		_, err = controllerutil.CreateOrPatch(ctx, helper.GetClient(), poolsYamlConfigMap, func() error {
+			poolsYamlConfigMap.Labels = util.MergeStringMaps(poolsYamlConfigMap.Labels, bindLabels)
+			poolsYamlConfigMap.Data = updatedPoolsYaml
+			return controllerutil.SetControllerReference(instance, poolsYamlConfigMap, helper.GetScheme())
+		})
+		if err != nil {
+			Log.Info("Unable to create config map for pools.yaml file")
+			return ctrl.Result{}, err
+		}
+		configMaps := []interface{}{
+			poolsYamlConfigMap.Data,
+		}
+
+		if designateCentral.Status.ReadyCount == *designateCentral.Spec.Replicas {
+			poolsYamlsEnvVars := make(map[string]env.Setter)
+			_, changed, err := r.createHashOfInputHashes(ctx, instance, designate.PoolsYamlHash, poolsYamlsEnvVars, configMaps)
+			if err != nil {
+				return ctrl.Result{}, err
+			}
+			if changed {
+				Log.Info("PoolsYamlHash has changed, creating a pool update job")
+
+				var poolUpdateHash string
+				var ok bool
+				if poolUpdateHash, ok = instance.Status.Hash[designatev1beta1.PoolUpdateHash]; !ok {
+					instance.Status.Hash[designatev1beta1.PoolUpdateHash] = ""
+					poolUpdateHash = ""
+				}
+				jobDef := designate.PoolUpdateJob(instance, serviceLabels, serviceAnnotations)
+
+				Log.Info("Initializing pool update job")
+				poolUpdatejob := job.NewJob(
+					jobDef,
+					designatev1beta1.PoolUpdateHash,
+					instance.Spec.PreserveJobs,
+					time.Duration(15)*time.Second,
+					poolUpdateHash,
+				)
+				_, err = poolUpdatejob.DoJob(ctx, helper)
+				if err != nil {
+					return ctrl.Result{}, err
+				}
+				instance.Status.Hash[designatev1beta1.PoolUpdateHash] = poolUpdatejob.GetHash()
+				err = r.Client.Status().Update(ctx, instance)
+				if err != nil {
+					return ctrl.Result{}, err
+				}
+				Log.Info("Pool update job completed successfully")
+			}
+		}
+	}
+
 	// remove finalizers from unused MariaDBAccount records
 	err = mariadbv1.DeleteUnusedMariaDBAccountFinalizers(ctx, helper, designate.DatabaseCRName, instance.Spec.DatabaseAccount, instance.Namespace)
 	if err != nil {
@@ -1154,6 +1156,9 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des
 	if instance.Status.Conditions.AllSubConditionIsTrue() {
 		instance.Status.Conditions.MarkTrue(
 			condition.ReadyCondition, condition.ReadyMessage)
+	} else {
+		Log.Info("Conditions for readiness not all true")
+		return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
 	}
 	Log.Info("Reconciled Service successfully")
 	return ctrl.Result{}, nil

diff --git a/controllers/designatebackendbind9_controller.go b/controllers/designatebackendbind9_controller.go
@@ -281,15 +281,6 @@ func (r *DesignateBackendbind9Reconciler) reconcileDelete(instance *designatev1b
 	return ctrl.Result{}, nil
 }
 
-func (r *DesignateBackendbind9Reconciler) reconcileInit(
-	instance *designatev1beta1.DesignateBackendbind9,
-) (ctrl.Result, error) {
-	r.Log.Info(fmt.Sprintf("Reconciling Service '%s' init", instance.Name))
-
-	r.Log.Info(fmt.Sprintf("Reconciled Service '%s' init successfully", instance.Name))
-	return ctrl.Result{}, nil
-}
-
 func (r *DesignateBackendbind9Reconciler) reconcileNormal(ctx context.Context, instance *designatev1beta1.DesignateBackendbind9, helper *helper.Helper) (ctrl.Result, error) {
 	r.Log.Info("Reconciling Service")
 
@@ -305,15 +296,6 @@ func (r *DesignateBackendbind9Reconciler) reconcileNormal(ctx context.Context, i
 	}
 	// run check OpenStack secret - end
 
-	//
-	// check for required TransportURL secret holding transport URL string
-	//
-	ctrlResult, err = r.getSecret(ctx, helper, instance, instance.Spec.TransportURLSecret, &configMapVars, "secret-")
-	if err != nil {
-		return ctrlResult, err
-	}
-	// run check TransportURL secret - end
-
 	//
 	// check for required service secrets
 	//
@@ -325,24 +307,6 @@ func (r *DesignateBackendbind9Reconciler) reconcileNormal(ctx context.Context, i
 	}
 	// run check service secrets - end
 
-	//
-	// check for required Designate config maps that should have been created by parent Designate CR
-	//
-
-	parentDesignateName := designate.GetOwningDesignateName(instance)
-	r.Log.Info(fmt.Sprintf("Reconciling Service '%s' init: parent name: %s", instance.Name, parentDesignateName))
-
-	ctrlResult, err = r.getSecret(ctx, helper, instance, fmt.Sprintf("%s-scripts", parentDesignateName), &configMapVars, "")
-	if err != nil {
-		return ctrlResult, err
-	}
-	ctrlResult, err = r.getSecret(ctx, helper, instance, fmt.Sprintf("%s-config-data", parentDesignateName), &configMapVars, "")
-	// note r.getSecret adds Conditions with condition.InputReadyWaitingMessage
-	// when secret is not found
-	if err != nil {
-		return ctrlResult, err
-	}
-
 	instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage)
 	// run check parent Designate CR config maps - end
 
@@ -444,14 +408,6 @@ func (r *DesignateBackendbind9Reconciler) reconcileNormal(ctx context.Context, i
 			instance.Spec.NetworkAttachments, err)
 	}
 
-	// Handle service init
-	ctrlResult, err = r.reconcileInit(instance)
-	if err != nil {
-		return ctrlResult, err
-	} else if (ctrlResult != ctrl.Result{}) {
-		return ctrlResult, nil
-	}
-
 	// Handle service update
 	ctrlResult, err = r.reconcileUpdate(instance)
 	if err != nil {

diff --git a/hack/run_with_local_webhook.sh b/hack/run_with_local_webhook.sh
@@ -16,8 +16,8 @@ SKIP_CERT=${SKIP_CERT:-false}
 CRC_IP=${CRC_IP:-$(/sbin/ip -o -4 addr list crc | awk '{print $4}' | cut -d/ -f1)}
 
 #Open 9443
-sudo firewall-cmd --zone=libvirt --add-port=9443/tcp
-sudo firewall-cmd --runtime-to-permanent
+#sudo firewall-cmd --zone=libvirt --add-port=9443/tcp
+#sudo firewall-cmd --runtime-to-permanent
 
 # Generate the certs and the ca bundle
 if [ "$SKIP_CERT" = false ] ; then

diff --git a/pkg/designatecentral/deployment.go b/pkg/designatecentral/deployment.go
@@ -27,7 +27,6 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	// "k8s.io/apimachinery/pkg/util/intstr"
 )
 
 // Deployment func
@@ -37,15 +36,12 @@ func Deployment(
 	labels map[string]string,
 	annotations map[string]string,
 ) *appsv1.Deployment {
+	// TODO(beagles): running as root should not be necessary here.
 	rootUser := int64(0)
-	// Designate's uid and gid magic numbers come from the 'designate-user' in
-	// https://github.com/openstack/kolla/blob/master/kolla/common/users.py
-	// designateUser := int64(42411)
-	// designateGroup := int64(42411)
 
 	serviceName := fmt.Sprintf("%s-central", designate.ServiceName)
-	volumes := getServicePodVolumes(serviceName)
-	volumeMounts := getServicePodVolumeMounts(serviceName)
+	volumes := designate.GetVolumes(serviceName)
+	volumeMounts := designate.GetVolumeMounts(serviceName)
 
 	livenessProbe := &corev1.Probe{
 		// TODO might need tuning

diff --git a/pkg/designatecentral/volumes.go b/pkg/designatecentral/volumes.go
diff --git a/templates/designatebackendbind9/config-named/options.conf b/templates/designatebackendbind9/config-named/options.conf
@@ -9,10 +9,10 @@ options {
 
         # TODO: The '*'s need to be replaced by actual addresses.
 {{ if eq .IPVersion "4" }}
-        listen-on port 53 { *; };
+        listen-on port 53 { any; };
         listen-on-v6 { none; };
 {{ else if eq .IPVersion "6" }}
-        listen-on-v6 port 53 { *; };
+        listen-on-v6 port 53 { any; };
         listen-on { none; };
 {{ end }}