chore: add more logging for failures to get label metadata (#14275)

* chore: add more logging for failures to get label metadata Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * context Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * fix test Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * fix test Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --------- Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Signed-off-by: jannfis <jann@mistrust.net> Co-authored-by: jannfis <jann@mistrust.net>
argoproj · Jul 1, 2023 · 5f455af · 5f455af
1 parent 53db27e
commit 5f455af
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 18 deletions.
diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go
@@ -783,6 +783,11 @@ func (s *Service) runManifestGenAsync(ctx context.Context, repoRoot, commitSHA,
 		}
 	}
 	if err != nil {
+		logCtx := log.WithFields(log.Fields{
+			"application":  q.AppName,
+			"appNamespace": q.Namespace,
+		})
+
 		// If manifest generation error caching is enabled
 		if s.initConstants.PauseGenerationAfterFailedGenerationAttempts > 0 {
 			cache.LogDebugManifestCacheKeyFields("getting manifests cache", "GenerateManifests error", cacheKey, q.ApplicationSource, q.RefSources, q, q.Namespace, q.TrackingMethod, q.AppLabelKey, q.AppName, refSourceCommitSHAs)
@@ -792,7 +797,7 @@ func (s *Service) runManifestGenAsync(ctx context.Context, repoRoot, commitSHA,
 			innerRes := &cache.CachedManifestResponse{}
 			cacheErr := s.cache.GetManifests(cacheKey, appSourceCopy, q.RefSources, q, q.Namespace, q.TrackingMethod, q.AppLabelKey, q.AppName, innerRes, refSourceCommitSHAs)
 			if cacheErr != nil && cacheErr != cache.ErrCacheMiss {
-				log.Warnf("manifest cache set error %s: %v", appSourceCopy.String(), cacheErr)
+				logCtx.Warnf("manifest cache get error %s: %v", appSourceCopy.String(), cacheErr)
 				ch.errCh <- cacheErr
 				return
 			}
@@ -810,7 +815,7 @@ func (s *Service) runManifestGenAsync(ctx context.Context, repoRoot, commitSHA,
 			innerRes.MostRecentError = err.Error()
 			cacheErr = s.cache.SetManifests(cacheKey, appSourceCopy, q.RefSources, q, q.Namespace, q.TrackingMethod, q.AppLabelKey, q.AppName, innerRes, refSourceCommitSHAs)
 			if cacheErr != nil {
-				log.Warnf("manifest cache set error %s: %v", appSourceCopy.String(), cacheErr)
+				logCtx.Warnf("manifest cache set error %s: %v", appSourceCopy.String(), cacheErr)
 				ch.errCh <- cacheErr
 				return
 			}
@@ -1396,7 +1401,7 @@ func GenerateManifests(ctx context.Context, appPath, repoRoot, revision string,
 			if q.AppLabelKey != "" && q.AppName != "" && !kube.IsCRD(target) {
 				err = resourceTracking.SetAppInstance(target, q.AppLabelKey, q.AppName, q.Namespace, v1alpha1.TrackingMethod(q.TrackingMethod))
 				if err != nil {
-					return nil, err
+					return nil, fmt.Errorf("failed to set app instance tracking info on manifest: %w", err)
 				}
 			}
 			manifestStr, err := json.Marshal(target.Object)

diff --git a/util/argo/resource_tracking.go b/util/argo/resource_tracking.go
@@ -4,12 +4,13 @@ import (
 	"fmt"
 	"strings"
 
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+
 	"github.com/argoproj/argo-cd/v2/common"
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	"github.com/argoproj/argo-cd/v2/util/kube"
 	argokube "github.com/argoproj/argo-cd/v2/util/kube"
 	"github.com/argoproj/argo-cd/v2/util/settings"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
 const (
@@ -31,7 +32,7 @@ type ResourceTracking interface {
 	Normalize(config, live *unstructured.Unstructured, labelKey, trackingMethod string) error
 }
 
-//AppInstanceValue store information about resource tracking info
+// AppInstanceValue store information about resource tracking info
 type AppInstanceValue struct {
 	ApplicationName string
 	Group           string
@@ -140,7 +141,11 @@ func (rt *resourceTracking) SetAppInstance(un *unstructured.Unstructured, key, v
 	}
 	switch trackingMethod {
 	case TrackingMethodLabel:
-		return argokube.SetAppInstanceLabel(un, key, val)
+		err := argokube.SetAppInstanceLabel(un, key, val)
+		if err != nil {
+			return fmt.Errorf("failed to set app instance label: %w", err)
+		}
+		return nil
 	case TrackingMethodAnnotation:
 		return setAppInstanceAnnotation()
 	case TrackingMethodAnnotationAndLabel:
@@ -151,18 +156,26 @@ func (rt *resourceTracking) SetAppInstance(un *unstructured.Unstructured, key, v
 		if len(val) > LabelMaxLength {
 			val = val[:LabelMaxLength]
 		}
-		return argokube.SetAppInstanceLabel(un, key, val)
+		err = argokube.SetAppInstanceLabel(un, key, val)
+		if err != nil {
+			return fmt.Errorf("failed to set app instance label: %w", err)
+		}
+		return nil
 	default:
-		return argokube.SetAppInstanceLabel(un, key, val)
+		err := argokube.SetAppInstanceLabel(un, key, val)
+		if err != nil {
+			return fmt.Errorf("failed to set app instance label: %w", err)
+		}
+		return nil
 	}
 }
 
-//BuildAppInstanceValue build resource tracking id in format <application-name>;<group>/<kind>/<namespace>/<name>
+// BuildAppInstanceValue build resource tracking id in format <application-name>;<group>/<kind>/<namespace>/<name>
 func (rt *resourceTracking) BuildAppInstanceValue(value AppInstanceValue) string {
 	return fmt.Sprintf("%s:%s/%s:%s/%s", value.ApplicationName, value.Group, value.Kind, value.Namespace, value.Name)
 }
 
-//ParseAppInstanceValue parse resource tracking id from format <application-name>:<group>/<kind>:<namespace>/<name> to struct
+// ParseAppInstanceValue parse resource tracking id from format <application-name>:<group>/<kind>:<namespace>/<name> to struct
 func (rt *resourceTracking) ParseAppInstanceValue(value string) (*AppInstanceValue, error) {
 	var appInstanceValue AppInstanceValue
 	parts := strings.Split(value, ":")
@@ -198,7 +211,7 @@ func (rt *resourceTracking) Normalize(config, live *unstructured.Unstructured, l
 
 	label, err := kube.GetAppInstanceLabel(live, labelKey)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to get app instance label: %w", err)
 	}
 	if label == "" {
 		return nil
@@ -215,12 +228,12 @@ func (rt *resourceTracking) Normalize(config, live *unstructured.Unstructured, l
 
 	label, err = argokube.GetAppInstanceLabel(config, labelKey)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to get app instance label: %w", err)
 	}
 	if label == "" {
 		err = argokube.RemoveLabel(live, labelKey)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to remove app instance label: %w", err)
 		}
 	}
 

diff --git a/util/kube/kube.go b/util/kube/kube.go
@@ -1,6 +1,7 @@
 package kube
 
 import (
+	"fmt"
 	"regexp"
 
 	"github.com/argoproj/gitops-engine/pkg/utils/kube"
@@ -23,7 +24,7 @@ func SetAppInstanceLabel(target *unstructured.Unstructured, key, val string) err
 	// Do not use target.GetLabels(), https://github.com/argoproj/argo-cd/issues/13730
 	labels, _, err := unstructured.NestedStringMap(target.Object, "metadata", "labels")
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to get labels from target object %s %s/%s: %w", target.GroupVersionKind().String(), target.GetNamespace(), target.GetName(), err)
 	}
 	if labels == nil {
 		labels = make(map[string]string)
@@ -131,7 +132,7 @@ func GetAppInstanceLabel(un *unstructured.Unstructured, key string) (string, err
 	// Do not use target.GetLabels(), https://github.com/argoproj/argo-cd/issues/13730
 	labels, _, err := unstructured.NestedStringMap(un.Object, "metadata", "labels")
 	if err != nil {
-		return "", err
+		return "", fmt.Errorf("failed to get labels for %s %s/%s: %w", un.GroupVersionKind().String(), un.GetNamespace(), un.GetName(), err)
 	}
 	if labels != nil {
 		return labels[key], nil
@@ -144,7 +145,7 @@ func RemoveLabel(un *unstructured.Unstructured, key string) error {
 	// Do not use target.GetLabels(), https://github.com/argoproj/argo-cd/issues/13730
 	labels, _, err := unstructured.NestedStringMap(un.Object, "metadata", "labels")
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to get labels for %s %s/%s: %w", un.GroupVersionKind().String(), un.GetNamespace(), un.GetName(), err)
 	}
 	if labels == nil {
 		return nil

diff --git a/util/kube/kube_test.go b/util/kube/kube_test.go
@@ -242,7 +242,7 @@ func TestGetAppInstanceLabelWithInvalidData(t *testing.T) {
 	assert.Nil(t, err)
 	_, err = GetAppInstanceLabel(&obj, "valid-label")
 	assert.Error(t, err)
-	assert.Equal(t, ".metadata.labels accessor error: contains non-string key in the map: <nil> is of the type <nil>, expected string", err.Error())
+	assert.Equal(t, "failed to get labels for /v1, Kind=Service /my-service: .metadata.labels accessor error: contains non-string key in the map: <nil> is of the type <nil>, expected string", err.Error())
 }
 
 func TestRemoveLabel(t *testing.T) {
@@ -268,5 +268,5 @@ func TestRemoveLabelWithInvalidData(t *testing.T) {
 
 	err = RemoveLabel(&obj, "valid-label")
 	assert.Error(t, err)
-	assert.Equal(t, ".metadata.labels accessor error: contains non-string key in the map: <nil> is of the type <nil>, expected string", err.Error())
+	assert.Equal(t, "failed to get labels for /v1, Kind=Service /my-service: .metadata.labels accessor error: contains non-string key in the map: <nil> is of the type <nil>, expected string", err.Error())
 }