Some refactor notes.

Author: beagles

controllers/designate_controller.go

@@ -67,6 +67,14 @@ func getOrDefault(source string, def string) string {
 	return source
 }
 
+// NOTE(beagles): refactoring this is a little tricky because it has multiple
+// side-effects. It sets a parameter in conditions, sets a hash in the envVars,
+// and returns results. The function name is a little misleading as well because
+// it doesn't actually get the secret. It's more that it checks for it and gets
+// the hash. At the very least a rename to"ensureSecret" is warranted. It might
+// also be better to move into a separate file along with other helper
+// functions.
+
 // helper function for retrieving a secret.
 func getSecret(
 	ctx context.Context,
@@ -701,6 +709,9 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des
 		return ctrlResult, nil
 	}
 
+	// NOTE(beagles): consider moving the API reconcile until after the pools
+	// yaml can be created. This avoid the "no servers" error that occurs when
+	// there aren't any configured DNS servers.
 	//
 	// normal reconcile tasks
 	//
@@ -748,6 +759,8 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des
 	}
 	Log.Info("Deployment API task reconciled")
 
+	// NOTE(beagles): Consider moving IP Map construction into a separate function.
+
 	// Handle Mdns predictable IPs configmap
 	nad, err := nad.GetNADWithName(ctx, helper, instance.Spec.DesignateNetworkAttachment, instance.Namespace)
 	if err != nil {
@@ -850,6 +863,8 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des
 		return ctrl.Result{}, err
 	}
 
+	// NOTE(beagles): Consider moving pools yaml generation into a separate function.
+
 	Log.Info("Bind configmap was created successfully")
 	if len(nsRecords) > 0 && instance.Status.DesignateCentralReadyCount > 0 {
 		Log.Info("NS records data found")
@@ -908,7 +923,10 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des
 		}
 	}
 
-	// deploy designate-central
+	// NOTE(beagles): Kind of makes you wish for macros. These are all the same
+	// pattern with just different names.
+	//
+	//  deploy designate-central
 	designateCentral, op, err := r.centralDeploymentCreateOrUpdate(ctx, instance)
 	if err != nil {
 		instance.Status.Conditions.Set(condition.FalseCondition(

pkg/designate/generate_bind9_pools_yaml.go

@@ -91,10 +91,15 @@ func GeneratePoolsYamlDataAndHash(BindMap, MdnsMap map[string]string, nsRecords
 		return nsRecords[i].Priority < nsRecords[j].Priority
 	})
 
+	// Create an ordered list of IPs for the BIND's predictable IPs.
 	bindIPs := make([]string, len(BindMap))
 	keyTmpl := "bind_address_%d"
-	for i := 0; i < len(BindMap); i++ {
-		bindIPs[i] = BindMap[fmt.Sprintf(keyTmpl, i)]
+	for i := range len(BindMap) {
+		if ip, ok := BindMap[fmt.Sprintf(keyTmpl, i)]; ok {
+			bindIPs[i] = ip
+		} else {
+			return "", "", fmt.Errorf("bind IP not found for index %d", i)
+		}
 	}
 
 	masterHosts := make([]string, 0, len(MdnsMap))