Fix ConfigMap names (#33)

Also improvements to log messages
nephio-project · Jun 15, 2023 · 53c51fc · 53c51fc
1 parent 5c30ddd
commit 53c51fc
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 50 deletions.
diff --git a/controllers/amf/reconciler.go b/controllers/amf/reconciler.go
@@ -74,7 +74,7 @@ func (r *AMFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	err := r.Client.Get(ctx, req.NamespacedName, amfDeployment)
 	if err != nil {
 		if k8serrors.IsNotFound(err) {
-			log.Info("AMFDeployment resource not found. Ignoring since object must be deleted")
+			log.Info("AMFDeployment resource not found, ignoring sibecausence object must be deleted")
 			return reconcile.Result{}, nil
 		}
 		log.Error(err, "Failed to get AMFDeployment")
@@ -84,7 +84,7 @@ func (r *AMFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	namespace := amfDeployment.Namespace
 
 	configMapFound := false
-	configMapName := amfDeployment.Name + "-amf-configmap"
+	configMapName := amfDeployment.Name
 	var configMapVersion string
 	currentConfigMap := new(apiv1.ConfigMap)
 	if err := r.Client.Get(ctx, types.NamespacedName{Name: configMapName, Namespace: namespace}, currentConfigMap); err == nil {
@@ -113,17 +113,17 @@ func (r *AMFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		// On the subsequent runs it gets undelying depoyment Conditions and use the last one to decide if status has to be updated.
 		if deployment.DeletionTimestamp == nil {
 			if err := r.syncStatus(ctx, deployment, amfDeployment); err != nil {
-				log.Error(err, "Failed to update AMFDeployment status", "AMFDeployment.namespace", namespace, "AMFDeployment.name", amfDeployment.Name)
+				log.Error(err, "Failed to update status")
 				return reconcile.Result{}, err
 			}
 		}
 
 		if currentDeployment.Spec.Template.Annotations[controllers.ConfigMapVersionAnnotation] != configMapVersion {
-			log.Info("ConfigMap has been updated. Rolling Deployment pods.", "AMFDeployment.namespace", namespace, "AMFDeployment.name", amfDeployment.Name)
+			log.Info("ConfigMap has been updated, rolling Deployment pods", "Deployment.namespace", currentDeployment.Namespace, "Deployment.name", currentDeployment.Name)
 			currentDeployment.Spec.Template.Annotations[controllers.ConfigMapVersionAnnotation] = configMapVersion
 
 			if err := r.Update(ctx, currentDeployment); err != nil {
-				log.Error(err, "Failed to update Deployment", "AMFDeployment.namespace", currentDeployment.Namespace, "AMFDeployment.name", currentDeployment.Name)
+				log.Error(err, "Failed to update Deployment", "Deployment.namespace", currentDeployment.Namespace, "Deployment.name", currentDeployment.Name)
 				return reconcile.Result{}, err
 			}
 
@@ -133,37 +133,37 @@ func (r *AMFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 
 	if configMap, err := createConfigMap(log, amfDeployment); err == nil {
 		if !configMapFound {
-			log.Info("Creating AMFDeployment configmap", "AMFDeployment.namespace", namespace, "ConfigMap.name", configMap.Name)
+			log.Info("Creating ConfigMap", "ConfigMap.namespace", configMap.Namespace, "ConfigMap.name", configMap.Name)
 
 			// Set the controller reference, specifying that AMFDeployment controling underlying deployment
 			if err := ctrl.SetControllerReference(amfDeployment, configMap, r.Scheme); err != nil {
-				log.Error(err, "Got error while setting Owner reference on configmap.", "AMFDeployment.namespace", namespace)
+				log.Error(err, "Got error while setting Owner reference on configmap.", "ConfigMap.namespace", configMap.Namespace, "ConfigMap.name", configMap.Name)
 			}
 
 			if err := r.Client.Create(ctx, configMap); err != nil {
-				log.Error(err, fmt.Sprintf("Failed to create ConfigMap %s\n", err.Error()))
+				log.Error(err, "Failed to create ConfigMap", "ConfigMap.namespace", configMap.Namespace, "ConfigMap.name", configMap.Name)
 				return reconcile.Result{}, err
 			}
 
 			configMapVersion = configMap.ResourceVersion
 		}
 	} else {
-		log.Error(err, fmt.Sprintf("Failed to create ConfigMap %s\n", err.Error()))
+		log.Error(err, "Failed to create ConfigMap")
 		return reconcile.Result{}, err
 	}
 
 	if !serviceFound {
 		service := createService(amfDeployment)
 
-		log.Info("Creating AMFDeployment service", "AMFDeployment.namespace", namespace, "Service.name", service.Name)
+		log.Info("Creating AMFDeployment service", "Service.namespace", service.Namespace, "Service.name", service.Name)
 
 		// Set the controller reference, specifying that AMFDeployment controling underlying deployment
 		if err := ctrl.SetControllerReference(amfDeployment, service, r.Scheme); err != nil {
-			log.Error(err, "Got error while setting Owner reference on AMF service.", "AMFDeployment.namespace", namespace)
+			log.Error(err, "Got error while setting Owner reference on AMF service", "Service.namespace", service.Namespace, "Service.name", service.Name)
 		}
 
 		if err := r.Client.Create(ctx, service); err != nil {
-			log.Error(err, fmt.Sprintf("Failed to create Service %s\n", err.Error()))
+			log.Error(err, "Failed to create Service", "Service.namespace", service.Namespace, "Service.name", service.Name)
 			return reconcile.Result{}, err
 		}
 	}
@@ -174,18 +174,18 @@ func (r *AMFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 			if ok := controllers.ValidateNetworkAttachmentDefinitions(ctx, r.Client, log, amfDeployment.Kind, deployment); ok {
 				// Set the controller reference, specifying that AMFDeployment controls the underlying Deployment
 				if err := ctrl.SetControllerReference(amfDeployment, deployment, r.Scheme); err != nil {
-					log.Error(err, "Got error while setting Owner reference on deployment.", "AMFDeployment.namespace", namespace)
+					log.Error(err, "Got error while setting Owner reference on deployment", "Deployment.namespace", deployment.Namespace, "Deployment.name", deployment.Name)
 				}
 
-				log.Info("Creating AMFDeployment", "AMFDeployment.namespace", namespace, "AMFDeployment.name", amfDeployment.Name)
+				log.Info("Creating Deployment", "Deployment.namespace", deployment.Namespace, "Deployment.name", deployment.Name)
 				if err := r.Client.Create(ctx, deployment); err != nil {
-					log.Error(err, "Failed to create new Deployment", "AMFDeployment.namespace", namespace, "AMFDeployment.name", amfDeployment.Name)
+					log.Error(err, "Failed to create new Deployment", "Deployment.namespace", deployment.Namespace, "Deployment.name", deployment.Name)
 				}
 
 				// TODO(tliron): explain why we need requeueing (do we?)
 				return reconcile.Result{RequeueAfter: time.Duration(30) * time.Second}, nil
 			} else {
-				log.Info("Not all NetworkAttachDefinitions available in current namespace. Requeue in 10 sec.", "AMFDeployment.namespace", namespace)
+				log.Info("Not all NetworkAttachDefinitions available in current namespace, requeuing")
 				return reconcile.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
 			}
 		}

diff --git a/controllers/smf/reconciler.go b/controllers/smf/reconciler.go
@@ -18,7 +18,6 @@ package smf
 
 import (
 	"context"
-	"fmt"
 	"time"
 
 	nephiov1alpha1 "github.com/nephio-project/api/nf_deployments/v1alpha1"
@@ -74,7 +73,7 @@ func (r *SMFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	err := r.Client.Get(ctx, req.NamespacedName, smfDeployment)
 	if err != nil {
 		if k8serrors.IsNotFound(err) {
-			log.Info("SMFDeployment resource not found. Ignoring since object must be deleted")
+			log.Info("SMFDeployment resource not found, ignoring because object must be deleted")
 			return reconcile.Result{}, nil
 		}
 		log.Error(err, "Failed to get SMFDeployment")
@@ -84,7 +83,7 @@ func (r *SMFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	namespace := smfDeployment.Namespace
 
 	configMapFound := false
-	configMapName := smfDeployment.Name + "-smf-configmap"
+	configMapName := smfDeployment.Name
 	var configMapVersion string
 	currentConfigMap := new(apiv1.ConfigMap)
 	if err := r.Client.Get(ctx, types.NamespacedName{Name: configMapName, Namespace: namespace}, currentConfigMap); err == nil {
@@ -93,7 +92,7 @@ func (r *SMFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	}
 
 	serviceFound := false
-	serviceName := "smf-nsmf"
+	serviceName := smfDeployment.Name
 	currentService := new(apiv1.Service)
 	if err := r.Client.Get(ctx, types.NamespacedName{Name: serviceName, Namespace: namespace}, currentService); err == nil {
 		serviceFound = true
@@ -111,16 +110,16 @@ func (r *SMFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 
 		if deployment.DeletionTimestamp == nil {
 			if err := r.syncStatus(ctx, deployment, smfDeployment); err != nil {
-				log.Error(err, "Failed to update SMFDeployment status", "SMFDeployment.namespace", namespace, "SMFDeployment.name", smfDeployment.Name)
+				log.Error(err, "Failed to update status")
 				return reconcile.Result{}, err
 			}
 		}
 
 		if currentDeployment.Spec.Template.Annotations[controllers.ConfigMapVersionAnnotation] != configMapVersion {
-			log.Info("ConfigMap has been updated. Rolling Deployment pods.", "SMFDeployment.namespace", namespace, "SMFDeployment.name", smfDeployment.Name)
+			log.Info("ConfigMap has been updated, rolling Deployment pods", "Deployment.namespace", currentDeployment.Namespace, "Deployment.name", currentDeployment.Name)
 			currentDeployment.Spec.Template.Annotations[controllers.ConfigMapVersionAnnotation] = configMapVersion
 			if err := r.Update(ctx, currentDeployment); err != nil {
-				log.Error(err, "Failed to update Deployment", "SMFDeployment.namespace", currentDeployment.Namespace, "SMFDeployment.name", currentDeployment.Name)
+				log.Error(err, "Failed to update Deployment", "Deployment.namespace", currentDeployment.Namespace, "Deployment.name", currentDeployment.Name)
 				return reconcile.Result{}, err
 			}
 			return reconcile.Result{Requeue: true}, nil
@@ -129,37 +128,37 @@ func (r *SMFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 
 	if configMap, err := createConfigMap(log, smfDeployment); err == nil {
 		if !configMapFound {
-			log.Info("Creating SMFDeployment configmap", "SMFDeployment.namespace", namespace, "ConfigMap.name", configMap.Name)
+			log.Info("Creating ConfigMap", "ConfigMap.namespace", configMap.Namespace, "ConfigMap.name", configMap.Name)
 
 			// Set the controller reference, specifying that SMFDeployment controls the underlying ConfigMap
 			if err := ctrl.SetControllerReference(smfDeployment, configMap, r.Scheme); err != nil {
-				log.Error(err, "Got error while setting Owner reference on configmap.", "SMFDeployment.namespace", namespace)
+				log.Error(err, "Got error while setting Owner reference on ConfigMap", "ConfigMap.namespace", configMap.Namespace, "ConfigMap.name", configMap.Name)
 			}
 
 			if err := r.Client.Create(ctx, configMap); err != nil {
-				log.Error(err, fmt.Sprintf("Failed to create ConfigMap %s\n", err.Error()))
+				log.Error(err, "Failed to create ConfigMap", "ConfigMap.namespace", configMap.Namespace, "ConfigMap.name", configMap.Name)
 				return reconcile.Result{}, err
 			}
 
 			configMapVersion = configMap.ResourceVersion
 		}
 	} else {
-		log.Error(err, fmt.Sprintf("Failed to create ConfigMap %s\n", err.Error()))
+		log.Error(err, "Failed to create ConfigMap")
 		return reconcile.Result{}, err
 	}
 
 	if !serviceFound {
 		service := createService(smfDeployment)
 
-		log.Info("Creating SMFDeployment service", "SMFDeployment.namespace", namespace, "Service.name", service.Name)
+		log.Info("Creating SMFDeployment service", "Service.namespace", service.Namespace, "Service.name", service.Name)
 
 		// Set the controller reference, specifying that SMFDeployment controls the underlying Service
 		if err := ctrl.SetControllerReference(smfDeployment, service, r.Scheme); err != nil {
-			log.Error(err, "Got error while setting Owner reference on SMF Service.", "SMFDeployment.namespace", namespace)
+			log.Error(err, "Got error while setting Owner reference on SMF Service", "Service.namespace", service.Namespace, "Service.name", service.Name)
 		}
 
 		if err := r.Client.Create(ctx, service); err != nil {
-			log.Error(err, fmt.Sprintf("Failed to create Service %s\n", err.Error()))
+			log.Error(err, "Failed to create Service", "Service.namespace", service.Namespace, "Service.name", service.Name)
 			return reconcile.Result{}, err
 		}
 	}
@@ -170,23 +169,23 @@ func (r *SMFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 			if ok := controllers.ValidateNetworkAttachmentDefinitions(ctx, r.Client, log, smfDeployment.Kind, deployment); ok {
 				// Set the controller reference, specifying that SMFDeployment controls the underlying Deployment
 				if err := ctrl.SetControllerReference(smfDeployment, deployment, r.Scheme); err != nil {
-					log.Error(err, "Got error while setting Owner reference on Deployment.", "SMFDeployment.namespace", namespace)
+					log.Error(err, "Got error while setting Owner reference on Deployment", "Deployment.namespace", deployment.Name, "Deployment.name", deployment.Name)
 				}
 
-				log.Info("Creating SMFDeployment", "SMFDeployment.namespace", namespace, "SMFDeployment.name", smfDeployment.Name)
+				log.Info("Creating Deployment", "Deployment.namespace", deployment.Name, "Deployment.name", deployment.Name)
 				if err := r.Client.Create(ctx, deployment); err != nil {
-					log.Error(err, "Failed to create new Deployment", "SMFDeployment.namespace", namespace, "SMFDeployment.name", smfDeployment.Name)
+					log.Error(err, "Failed to create new Deployment", "Deployment.namespace", deployment.Name, "Deployment.name", deployment.Name)
 				}
 
 				// TODO(tliron): explain why we need requeueing (do we?)
 				return reconcile.Result{RequeueAfter: time.Duration(30) * time.Second}, nil
 			} else {
-				log.Info("Not all NetworkAttachDefinitions available in current namespace. Requeue in 10 sec.", "SMFDeployment.namespace", namespace)
+				log.Info("Not all NetworkAttachDefinitions available in current namespace, requeuing")
 				return reconcile.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
 			}
 		}
 	} else {
-		log.Error(err, fmt.Sprintf("Failed to create Deployment %s\n", err.Error()))
+		log.Error(err, "Failed to create Deployment")
 		return reconcile.Result{}, err
 	}
 

diff --git a/controllers/upf/reconciler.go b/controllers/upf/reconciler.go
@@ -18,7 +18,6 @@ package upf
 
 import (
 	"context"
-	"fmt"
 	"time"
 
 	nephiov1alpha1 "github.com/nephio-project/api/nf_deployments/v1alpha1"
@@ -74,7 +73,7 @@ func (r *UPFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	err := r.Client.Get(ctx, req.NamespacedName, upfDeployment)
 	if err != nil {
 		if k8serrors.IsNotFound(err) {
-			log.Info("UPFDeployment resource not found. Ignoring since object must be deleted")
+			log.Info("UPFDeployment resource not found, ignoring because object must be deleted")
 			return reconcile.Result{}, nil
 		}
 		log.Error(err, "Failed to get UPFDeployment")
@@ -84,7 +83,7 @@ func (r *UPFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	namespace := upfDeployment.Namespace
 
 	configMapFound := false
-	configMapName := upfDeployment.Name + "-upf-configmap"
+	configMapName := upfDeployment.Name
 	var configMapVersion string
 	currentConfigMap := new(apiv1.ConfigMap)
 	if err := r.Client.Get(ctx, types.NamespacedName{Name: configMapName, Namespace: namespace}, currentConfigMap); err == nil {
@@ -106,17 +105,17 @@ func (r *UPFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		// On the subsequent runs it gets undelying depoyment Conditions and use the last one to decide if status has to be updated.
 		if deployment.DeletionTimestamp == nil {
 			if err := r.syncStatus(ctx, deployment, upfDeployment); err != nil {
-				log.Error(err, "Failed to update UPFDeployment status", "UPFDeployment.namespace", namespace, "UPFDeployment.name", upfDeployment.Name)
+				log.Error(err, "Failed to update status")
 				return reconcile.Result{}, err
 			}
 		}
 
 		if currentDeployment.Spec.Template.Annotations[controllers.ConfigMapVersionAnnotation] != configMapVersion {
-			log.Info("ConfigMap has been updated. Rolling Deployment pods.", "UPFDeployment.namespace", namespace, "UPFDeployment.name", upfDeployment.Name)
+			log.Info("ConfigMap has been updated, rolling Deployment pods", "Deployment.namespace", currentDeployment.Namespace, "Deployment.name", currentDeployment.Name)
 			currentDeployment.Spec.Template.Annotations[controllers.ConfigMapVersionAnnotation] = configMapVersion
 
 			if err := r.Update(ctx, currentDeployment); err != nil {
-				log.Error(err, "Failed to update Deployment", "UPFDeployment.namespace", currentDeployment.Namespace, "UPFDeployment.name", currentDeployment.Name)
+				log.Error(err, "Failed to update Deployment", "Deployment.namespace", currentDeployment.Namespace, "Deployment.name", currentDeployment.Name)
 				return reconcile.Result{}, err
 			}
 
@@ -126,22 +125,22 @@ func (r *UPFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 
 	if configMap, err := createConfigMap(log, upfDeployment); err == nil {
 		if !configMapFound {
-			log.Info("Creating UPFDeployment configmap", "UPFDeployment.namespace", namespace, "ConfigMap.name", configMap.Name)
+			log.Info("Creating ConfigMap", "ConfigMap.namespace", configMap.Namespace, "ConfigMap.name", configMap.Name)
 
 			// Set the controller reference, specifying that UPFDeployment controling underlying ConfigMap
 			if err := ctrl.SetControllerReference(upfDeployment, configMap, r.Scheme); err != nil {
-				log.Error(err, "Got error while setting Owner reference on ConfigMap.", "UPFDeployment.namespace", namespace)
+				log.Error(err, "Got error while setting Owner reference on ConfigMap", "ConfigMap.namespace", configMap.Namespace, "ConfigMap.name", configMap.Name)
 			}
 
 			if err := r.Client.Create(ctx, configMap); err != nil {
-				log.Error(err, fmt.Sprintf("Failed to create ConfigMap %s\n", err.Error()))
+				log.Error(err, "Failed to create ConfigMap", "ConfigMap.namespace", configMap.Namespace, "ConfigMap.name", configMap.Name)
 				return reconcile.Result{}, err
 			}
 
 			configMapVersion = configMap.ResourceVersion
 		}
 	} else {
-		log.Error(err, fmt.Sprintf("Failed to create ConfigMap %s\n", err.Error()))
+		log.Error(err, "Failed to create ConfigMap")
 		return reconcile.Result{}, err
 	}
 
@@ -151,23 +150,23 @@ func (r *UPFDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 			if ok := controllers.ValidateNetworkAttachmentDefinitions(ctx, r.Client, log, upfDeployment.Kind, deployment); ok {
 				// Set the controller reference, specifying that UPFDeployment controls the underlying Deployment
 				if err := ctrl.SetControllerReference(upfDeployment, deployment, r.Scheme); err != nil {
-					log.Error(err, "Got error while setting Owner reference on Deployment.", "UPFDeployment.namespace", namespace)
+					log.Error(err, "Got error while setting Owner reference on Deployment", "Deployment.namespace", deployment.Name, "Deployment.name", deployment.Name)
 				}
 
-				log.Info("Creating UPFDeployment", "UPFDeployment.namespace", namespace, "UPFDeployment.name", upfDeployment.Name)
+				log.Info("Creating Deployment", "Deployment.namespace", deployment.Namespace, "Deployment.name", deployment.Name)
 				if err := r.Client.Create(ctx, deployment); err != nil {
-					log.Error(err, "Failed to create new Deployment", "UPFDeployment.namespace", namespace, "UPFDeployment.name", upfDeployment.Name)
+					log.Error(err, "Failed to create new Deployment", "Deployment.namespace", deployment.Namespace, "Deployment.name", deployment.Name)
 				}
 
 				// TODO(tliron): explain why we need requeueing (do we?)
 				return reconcile.Result{RequeueAfter: time.Duration(30) * time.Second}, nil
 			} else {
-				log.Info("Not all NetworkAttachDefinitions available in current namespace. Requeue in 10 sec.", "UPFDeployment.namespace", namespace)
+				log.Info("Not all NetworkAttachDefinitions available in current namespace, requeuing")
 				return reconcile.Result{RequeueAfter: time.Duration(10) * time.Second}, nil
 			}
 		}
 	} else {
-		log.Error(err, fmt.Sprintf("Failed to create Deployment %s\n", err.Error()))
+		log.Error(err, "Failed to create Deployment")
 		return reconcile.Result{}, err
 	}