From ee18566efa289aad09f61d0b8281e635823abba0 Mon Sep 17 00:00:00 2001
From: Angus Lees <gus@inodes.org>
Date: Wed, 2 Aug 2017 13:20:48 +1000
Subject: [PATCH] Improve log description of object

- Move fqname to utils
- Add ResourceNameFor and use it to get a lowercased/pluralised
  version of the resource name
---
 cmd/delete.go      |  2 +-
 cmd/diff.go        |  2 +-
 cmd/update.go      | 13 ++-------
 utils/meta.go      | 31 +++++++++++++++++++++
 utils/meta_test.go | 68 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 104 insertions(+), 12 deletions(-)

diff --git a/cmd/delete.go b/cmd/delete.go
index 9ee289f5..487d9b3d 100644
--- a/cmd/delete.go
+++ b/cmd/delete.go
@@ -84,7 +84,7 @@ var deleteCmd = &cobra.Command{
 		}
 
 		for _, obj := range objs {
-			desc := fmt.Sprintf("%s/%s", obj.GetKind(), fqName(obj))
+			desc := fmt.Sprintf("%s %s", utils.ResourceNameFor(disco, obj), utils.FqName(obj))
 			log.Info("Deleting ", desc)
 
 			c, err := utils.ClientForResource(clientpool, disco, obj, defaultNs)
diff --git a/cmd/diff.go b/cmd/diff.go
index cb82a46b..e1cf8dc6 100644
--- a/cmd/diff.go
+++ b/cmd/diff.go
@@ -71,7 +71,7 @@ var diffCmd = &cobra.Command{
 
 		diffFound := false
 		for _, obj := range objs {
-			desc := fmt.Sprintf("%s/%s", obj.GetKind(), fqName(obj))
+			desc := fmt.Sprintf("%s %s", utils.ResourceNameFor(disco, obj), utils.FqName(obj))
 			log.Debugf("Fetching ", desc)
 
 			c, err := utils.ClientForResource(clientpool, disco, obj, defaultNs)
diff --git a/cmd/update.go b/cmd/update.go
index e8929d6f..a69038f9 100644
--- a/cmd/update.go
+++ b/cmd/update.go
@@ -121,7 +121,7 @@ var updateCmd = &cobra.Command{
 				utils.SetMetaDataAnnotation(obj, AnnotationGcTag, gcTag)
 			}
 
-			desc := fmt.Sprintf("%s/%s", obj.GetKind(), fqName(obj))
+			desc := fmt.Sprintf("%s %s", utils.ResourceNameFor(disco, obj), utils.FqName(obj))
 			log.Info("Updating ", desc, dryRunText)
 
 			rc, err := utils.ClientForResource(clientpool, disco, obj, defaultNs)
@@ -177,7 +177,7 @@ var updateCmd = &cobra.Command{
 					return err
 				}
 				gvk := o.GetObjectKind().GroupVersionKind()
-				desc := fmt.Sprintf("%s/%s (%s)", gvk.Kind, fqName(meta), gvk.GroupVersion())
+				desc := fmt.Sprintf("%s %s (%s)", utils.ResourceNameFor(disco, o), utils.FqName(meta), gvk.GroupVersion())
 				log.Debugf("Considering %v for gc", desc)
 				if eligibleForGc(meta, gcTag) && !seenUids.Has(string(meta.GetUID())) {
 					log.Info("Garbage collecting ", desc, dryRunText)
@@ -199,13 +199,6 @@ var updateCmd = &cobra.Command{
 	},
 }
 
-func fqName(o metav1.Object) string {
-	if o.GetNamespace() == "" {
-		return o.GetName()
-	}
-	return fmt.Sprintf("%s.%s", o.GetNamespace(), o.GetName())
-}
-
 func stringListContains(list []string, value string) bool {
 	for _, item := range list {
 		if item == value {
@@ -222,7 +215,7 @@ func gcDelete(clientpool dynamic.ClientPool, disco discovery.DiscoveryInterface,
 	}
 
 	uid := obj.GetUID()
-	desc := fmt.Sprintf("%s/%s", o.GetObjectKind().GroupVersionKind().Kind, fqName(obj))
+	desc := fmt.Sprintf("%s %s", utils.ResourceNameFor(disco, o), utils.FqName(obj))
 
 	deleteOpts := metav1.DeleteOptions{
 		Preconditions: &metav1.Preconditions{UID: &uid},
diff --git a/utils/meta.go b/utils/meta.go
index 8c83afdb..110aa769 100644
--- a/utils/meta.go
+++ b/utils/meta.go
@@ -3,8 +3,11 @@ package utils
 import (
 	"fmt"
 	"strconv"
+	"strings"
 
+	log "github.com/sirupsen/logrus"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/version"
 	"k8s.io/client-go/discovery"
 )
@@ -71,3 +74,31 @@ func SetMetaDataAnnotation(obj metav1.Object, key, value string) {
 	a[key] = value
 	obj.SetAnnotations(a)
 }
+
+// ResourceNameFor returns a lowercase plural form of a type, for
+// human messages.  Returns lowercased kind if discovery lookup fails.
+func ResourceNameFor(disco discovery.ServerResourcesInterface, o runtime.Object) string {
+	gvk := o.GetObjectKind().GroupVersionKind()
+	rls, err := disco.ServerResourcesForGroupVersion(gvk.GroupVersion().String())
+	if err != nil {
+		log.Debugf("Discovery failed for %s: %s, falling back to kind", gvk, err)
+		return strings.ToLower(gvk.Kind)
+	}
+
+	for _, rl := range rls.APIResources {
+		if rl.Kind == gvk.Kind {
+			return rl.Name
+		}
+	}
+
+	log.Debugf("Discovery failed to find %s, falling back to kind", gvk)
+	return strings.ToLower(gvk.Kind)
+}
+
+// FqName returns "namespace.name"
+func FqName(o metav1.Object) string {
+	if o.GetNamespace() == "" {
+		return o.GetName()
+	}
+	return fmt.Sprintf("%s.%s", o.GetNamespace(), o.GetName())
+}
diff --git a/utils/meta_test.go b/utils/meta_test.go
index 2bcf32e9..8b1f47b2 100644
--- a/utils/meta_test.go
+++ b/utils/meta_test.go
@@ -3,7 +3,12 @@ package utils
 import (
 	"testing"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/version"
+	fakediscovery "k8s.io/client-go/discovery/fake"
+	ktesting "k8s.io/client-go/testing"
 )
 
 func TestParseVersion(t *testing.T) {
@@ -63,3 +68,66 @@ func TestVersionCompare(t *testing.T) {
 		}
 	}
 }
+
+func TestResourceNameFor(t *testing.T) {
+	obj := &unstructured.Unstructured{
+		Object: map[string]interface{}{
+			"apiVersion": "tests/v1alpha1",
+			"kind":       "Test",
+			"metadata": map[string]interface{}{
+				"name":      "myname",
+				"namespace": "mynamespace",
+			},
+		},
+	}
+
+	fake := &ktesting.Fake{
+		Resources: []*metav1.APIResourceList{
+			{
+				GroupVersion: "tests/v1alpha1",
+				APIResources: []metav1.APIResource{
+					{
+						Name: "tests",
+						Kind: "Test",
+					},
+				},
+			},
+		},
+	}
+	disco := &fakediscovery.FakeDiscovery{Fake: fake}
+
+	if n := ResourceNameFor(disco, obj); n != "tests" {
+		t.Errorf("Got resource name %q for %v", n, obj)
+	}
+
+	obj.SetKind("Unknown")
+	if n := ResourceNameFor(disco, obj); n != "unknown" {
+		t.Errorf("Got resource name %q for %v", n, obj)
+	}
+
+	obj.SetGroupVersionKind(schema.GroupVersionKind{Group: "unknown", Version: "noversion", Kind: "SomeKind"})
+	if n := ResourceNameFor(disco, obj); n != "somekind" {
+		t.Errorf("Got resource name %q for %v", n, obj)
+	}
+}
+
+func TestFqName(t *testing.T) {
+	obj := &unstructured.Unstructured{
+		Object: map[string]interface{}{
+			"apiVersion": "tests/v1alpha1",
+			"kind":       "Test",
+			"metadata": map[string]interface{}{
+				"name": "myname",
+			},
+		},
+	}
+
+	if n := FqName(obj); n != "myname" {
+		t.Errorf("Got %q for %v", n, obj)
+	}
+
+	obj.SetNamespace("mynamespace")
+	if n := FqName(obj); n != "mynamespace.myname" {
+		t.Errorf("Got %q for %v", n, obj)
+	}
+}
-- 
GitLab