Refactor tables package to be more reusable

We still need the reflect helpers, but we allow for clients to
register their own pretty-printers, which avoids the package
dependency for our pretty-printer.  We register our pretty printers in
an init function in the relevant package (in this case,
upup/pkg/fi/printers.go)

Fix #5551

pkg/flagbuilder/BUILD.bazel

@@ -6,7 +6,7 @@ go_library(
     importpath = "k8s.io/kops/pkg/flagbuilder",
     visibility = ["//visibility:public"],
     deps = [
-        "//upup/pkg/fi/utils:go_default_library",
+        "//util/pkg/reflectutils:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
     ],

pkg/flagbuilder/build_flags.go

@@ -24,9 +24,10 @@ import (
 	"strings"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/kops/upup/pkg/fi/utils"
 
 	"github.com/golang/glog"
+
+	"k8s.io/kops/util/pkg/reflectutils"
 )
 
 // BuildFlags returns a space separated list arguments
@@ -57,7 +58,7 @@ func BuildFlagsList(options interface{}) ([]string, error) {
 		}
 		if tag == "-" {
 			glog.V(4).Infof("skipping field with %q flag tag: %s", tag, path)
-			return utils.SkipReflection
+			return reflectutils.SkipReflection
 		}
 
 		// If we specify the repeat option, we will repeat the flag rather than joining it with commas
@@ -109,7 +110,7 @@ func BuildFlagsList(options interface{}) ([]string, error) {
 					flag := fmt.Sprintf("--%s=%s", flagName, strings.Join(args, ","))
 					flags = append(flags, flag)
 				}
-				return utils.SkipReflection
+				return reflectutils.SkipReflection
 			}
 
 			return fmt.Errorf("BuildFlags of value type not handled: %T %s=%v", val.Interface(), path, val.Interface())
@@ -132,7 +133,7 @@ func BuildFlagsList(options interface{}) ([]string, error) {
 						flags = append(flags, flag)
 					}
 				}
-				return utils.SkipReflection
+				return reflectutils.SkipReflection
 			}
 
 			return fmt.Errorf("BuildFlags of value type not handled: %T %s=%v", val.Interface(), path, val.Interface())
@@ -188,9 +189,9 @@ func BuildFlagsList(options interface{}) ([]string, error) {
 			flags = append(flags, flag)
 		}
 
-		return utils.SkipReflection
+		return reflectutils.SkipReflection
 	}
-	err := utils.ReflectRecursive(reflect.ValueOf(options), walker)
+	err := reflectutils.ReflectRecursive(reflect.ValueOf(options), walker)
 	if err != nil {
 		return nil, fmt.Errorf("BuildFlagsList to reflect value: %s", err)
 	}

upup/pkg/fi/BUILD.bazel

@@ -21,6 +21,7 @@ go_library(
         "http.go",
         "lifecycle.go",
         "named.go",
+        "printers.go",
         "resources.go",
         "secrets.go",
         "target.go",
@@ -45,8 +46,10 @@ go_library(
         "//pkg/kopscodecs:go_default_library",
         "//pkg/pki:go_default_library",
         "//pkg/sshcredentials:go_default_library",
+        "//pkg/values:go_default_library",
         "//upup/pkg/fi/utils:go_default_library",
         "//util/pkg/hashing:go_default_library",
+        "//util/pkg/reflectutils:go_default_library",
         "//util/pkg/vfs:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",
         "//vendor/golang.org/x/crypto/ssh:go_default_library",

upup/pkg/fi/cloudup/BUILD.bazel

@@ -73,6 +73,7 @@ go_library(
         "//upup/pkg/fi/loader:go_default_library",
         "//upup/pkg/fi/utils:go_default_library",
         "//util/pkg/hashing:go_default_library",
+        "//util/pkg/reflectutils:go_default_library",
         "//util/pkg/vfs:go_default_library",
         "//vendor/github.com/blang/semver:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",

upup/pkg/fi/cloudup/loader.go

@@ -34,6 +34,7 @@ import (
 	"k8s.io/kops/upup/pkg/fi/assettasks"
 	"k8s.io/kops/upup/pkg/fi/loader"
 	"k8s.io/kops/upup/pkg/fi/utils"
+	"k8s.io/kops/util/pkg/reflectutils"
 	"k8s.io/kops/util/pkg/vfs"
 )
 
@@ -260,8 +261,8 @@ func (l *Loader) processDeferrals() error {
 	for taskKey, task := range l.tasks {
 		taskValue := reflect.ValueOf(task)
 
-		err := utils.ReflectRecursive(taskValue, func(path string, f *reflect.StructField, v reflect.Value) error {
-			if utils.IsPrimitiveValue(v) {
+		err := reflectutils.ReflectRecursive(taskValue, func(path string, f *reflect.StructField, v reflect.Value) error {
+			if reflectutils.IsPrimitiveValue(v) {
 				return nil
 			}
 
@@ -299,7 +300,7 @@ func (l *Loader) processDeferrals() error {
 							glog.V(11).Infof("Replacing task %q at %s:%s", *name, taskKey, path)
 							v.Set(reflect.ValueOf(primary))
 						}
-						return utils.SkipReflection
+						return reflectutils.SkipReflection
 					} else if rh, ok := intf.(*fi.ResourceHolder); ok {
 						if rh.Resource == nil {
 							//Resources can contain template 'arguments', separated by spaces
@@ -324,7 +325,7 @@ func (l *Loader) processDeferrals() error {
 								return fmt.Errorf("error setting resource value: %v", err)
 							}
 						}
-						return utils.SkipReflection
+						return reflectutils.SkipReflection
 					}
 				}
 			}

upup/pkg/fi/cloudup/populate_cluster_spec.go

@@ -38,7 +38,7 @@ import (
 	"k8s.io/kops/upup/models"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/loader"
-	"k8s.io/kops/upup/pkg/fi/utils"
+	"k8s.io/kops/util/pkg/reflectutils"
 	"k8s.io/kops/util/pkg/vfs"
 )
 
@@ -94,7 +94,7 @@ func (c *populateClusterSpec) run(clientset simple.Clientset) error {
 	// Copy cluster & instance groups, so we can modify them freely
 	cluster := &api.Cluster{}
 
-	utils.JsonMergeStruct(cluster, c.InputCluster)
+	reflectutils.JsonMergeStruct(cluster, c.InputCluster)
 
 	err := c.assignSubnets(cluster)
 	if err != nil {

upup/pkg/fi/cloudup/populate_instancegroup_spec.go

@@ -26,7 +26,7 @@ import (
 	"k8s.io/kops/pkg/apis/kops/validation"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
-	"k8s.io/kops/upup/pkg/fi/utils"
+	"k8s.io/kops/util/pkg/reflectutils"
 )
 
 // Default Machine types for various types of instance group machine
@@ -64,7 +64,7 @@ func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup,
 	}
 
 	ig := &kops.InstanceGroup{}
-	utils.JsonMergeStruct(ig, input)
+	reflectutils.JsonMergeStruct(ig, input)
 
 	// TODO: Clean up
 	if ig.IsMaster() {

upup/pkg/fi/cloudup/spec_builder.go

@@ -22,7 +22,7 @@ import (
 	api "k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/loader"
-	"k8s.io/kops/upup/pkg/fi/utils"
+	"k8s.io/kops/util/pkg/reflectutils"
 )
 
 type SpecBuilder struct {
@@ -42,8 +42,8 @@ func (l *SpecBuilder) BuildCompleteSpec(clusterSpec *api.ClusterSpec) (*api.Clus
 
 	// Master kubelet config = (base kubelet config + master kubelet config)
 	masterKubelet := &api.KubeletConfigSpec{}
-	utils.JsonMergeStruct(masterKubelet, completed.Kubelet)
-	utils.JsonMergeStruct(masterKubelet, completed.MasterKubelet)
+	reflectutils.JsonMergeStruct(masterKubelet, completed.Kubelet)
+	reflectutils.JsonMergeStruct(masterKubelet, completed.MasterKubelet)
 	completed.MasterKubelet = masterKubelet
 
 	glog.V(1).Infof("options: %s", fi.DebugAsJsonStringIndent(completed))

upup/pkg/fi/default_methods.go

@@ -20,7 +20,7 @@ import (
 	"fmt"
 	"reflect"
 
-	"k8s.io/kops/upup/pkg/fi/utils"
+	"k8s.io/kops/util/pkg/reflectutils"
 )
 
 // DefaultDeltaRunMethod implements the standard change-based run procedure:
@@ -108,7 +108,7 @@ func DefaultDeltaRunMethod(e Task, c *Context) error {
 
 // invokeCheckChanges calls the checkChanges method by reflection
 func invokeCheckChanges(a, e, changes Task) error {
-	rv, err := utils.InvokeMethod(e, "CheckChanges", a, e, changes)
+	rv, err := reflectutils.InvokeMethod(e, "CheckChanges", a, e, changes)
 	if err != nil {
 		return err
 	}
@@ -120,7 +120,7 @@ func invokeCheckChanges(a, e, changes Task) error {
 
 // invokeFind calls the find method by reflection
 func invokeFind(e Task, c *Context) (Task, error) {
-	rv, err := utils.InvokeMethod(e, "Find", c)
+	rv, err := reflectutils.InvokeMethod(e, "Find", c)
 	if err != nil {
 		return nil, err
 	}
@@ -136,9 +136,9 @@ func invokeFind(e Task, c *Context) (Task, error) {
 
 // invokeShouldCreate calls the ShouldCreate method by reflection, if it exists
 func invokeShouldCreate(a, e, changes Task) (bool, error) {
-	rv, err := utils.InvokeMethod(e, "ShouldCreate", a, e, changes)
+	rv, err := reflectutils.InvokeMethod(e, "ShouldCreate", a, e, changes)
 	if err != nil {
-		if utils.IsMethodNotFound(err) {
+		if reflectutils.IsMethodNotFound(err) {
 			return true, nil
 		}
 		return false, err

upup/pkg/fi/dryrun_target.go

@@ -28,7 +28,7 @@ import (
 	"github.com/golang/glog"
 	"k8s.io/kops/pkg/assets"
 	"k8s.io/kops/pkg/diff"
-	"k8s.io/kops/upup/pkg/fi/utils"
+	"k8s.io/kops/util/pkg/reflectutils"
 )
 
 // DryRunTarget is a special Target that does not execute anything, but instead tracks all changes.
@@ -167,7 +167,7 @@ func (t *DryRunTarget) PrintReport(taskMap map[string]Task, out io.Writer) error
 							continue
 						}
 
-						fieldValue := ValueAsString(field)
+						fieldValue := reflectutils.ValueAsString(field)
 
 						shouldPrint := true
 						if fieldName == "Name" {
@@ -338,7 +338,7 @@ func buildChangeList(a, e, changes Task) ([]change, error) {
 				}
 
 				if !ignored && description == "" {
-					description = fmt.Sprintf(" %v -> %v", ValueAsString(fieldValA), ValueAsString(fieldValE))
+					description = fmt.Sprintf(" %v -> %v", reflectutils.ValueAsString(fieldValA), reflectutils.ValueAsString(fieldValE))
 				}
 			}
 			if ignored {
@@ -398,104 +398,6 @@ func getTaskName(t Task) string {
 	return s
 }
 
-// ValueAsString returns a human-readable string representation of the passed value
-func ValueAsString(value reflect.Value) string {
-	b := &bytes.Buffer{}
-
-	walker := func(path string, field *reflect.StructField, v reflect.Value) error {
-		if utils.IsPrimitiveValue(v) || v.Kind() == reflect.String {
-			fmt.Fprintf(b, "%v", v.Interface())
-			return utils.SkipReflection
-		}
-
-		switch v.Kind() {
-		case reflect.Ptr, reflect.Interface, reflect.Slice, reflect.Map:
-			if v.IsNil() {
-				fmt.Fprintf(b, "<nil>")
-				return utils.SkipReflection
-			}
-		}
-
-		switch v.Kind() {
-		case reflect.Ptr, reflect.Interface:
-			return nil // descend into value
-
-		case reflect.Slice:
-			len := v.Len()
-			fmt.Fprintf(b, "[")
-			for i := 0; i < len; i++ {
-				av := v.Index(i)
-
-				if i != 0 {
-					fmt.Fprintf(b, ", ")
-				}
-				fmt.Fprintf(b, "%s", ValueAsString(av))
-			}
-			fmt.Fprintf(b, "]")
-			return utils.SkipReflection
-
-		case reflect.Map:
-			keys := v.MapKeys()
-			fmt.Fprintf(b, "{")
-			for i, key := range keys {
-				mv := v.MapIndex(key)
-
-				if i != 0 {
-					fmt.Fprintf(b, ", ")
-				}
-				fmt.Fprintf(b, "%s: %s", ValueAsString(key), ValueAsString(mv))
-			}
-			fmt.Fprintf(b, "}")
-			return utils.SkipReflection
-
-		case reflect.Struct:
-			intf := v.Addr().Interface()
-			if _, ok := intf.(Resource); ok {
-				fmt.Fprintf(b, "<resource>")
-			} else if _, ok := intf.(*ResourceHolder); ok {
-				fmt.Fprintf(b, "<resource>")
-			} else if compareWithID, ok := intf.(CompareWithID); ok {
-				id := compareWithID.CompareWithID()
-				name := ""
-				hasName, ok := intf.(HasName)
-				if ok {
-					name = StringValue(hasName.GetName())
-				}
-				if id == nil {
-					// Uninformative, but we can often print the name instead
-					if name != "" {
-						fmt.Fprintf(b, "name:%s", name)
-					} else {
-						fmt.Fprintf(b, "id:<nil>")
-					}
-				} else {
-					// Uninformative, but we can often print the name instead
-					if name != "" {
-						fmt.Fprintf(b, "name:%s id:%s", name, *id)
-					} else {
-						fmt.Fprintf(b, "id:%s", *id)
-					}
-
-				}
-			} else {
-				glog.V(4).Infof("Unhandled kind in asString for %q: %T", path, v.Interface())
-				fmt.Fprint(b, DebugAsJsonString(intf))
-			}
-			return utils.SkipReflection
-
-		default:
-			glog.Infof("Unhandled kind in asString for %q: %T", path, v.Interface())
-			return fmt.Errorf("Unhandled kind for %q: %v", path, v.Kind())
-		}
-	}
-
-	err := utils.ReflectRecursive(value, walker)
-	if err != nil {
-		glog.Fatalf("unexpected error during reflective walk: %v", err)
-	}
-	return b.String()
-}
-
 // Finish is called at the end of a run, and prints a list of changes to the configured Writer
 func (t *DryRunTarget) Finish(taskMap map[string]Task) error {
 	return t.PrintReport(taskMap, t.out)

upup/pkg/fi/errors.go

@@ -21,7 +21,8 @@ import (
 
 	"k8s.io/apimachinery/pkg/api/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-	"k8s.io/kops/upup/pkg/fi/utils"
+
+	"k8s.io/kops/util/pkg/reflectutils"
 )
 
 func RequiredField(key string) error {
@@ -33,6 +34,6 @@ func CannotChangeField(key string) error {
 }
 
 func FieldIsImmutable(newVal, oldVal interface{}, fldPath *field.Path) *field.Error {
-	details := fmt.Sprintf("%s: old=%v new=%v", validation.FieldImmutableErrorMsg, utils.FormatValue(oldVal), utils.FormatValue(newVal))
+	details := fmt.Sprintf("%s: old=%v new=%v", validation.FieldImmutableErrorMsg, reflectutils.FormatValue(oldVal), reflectutils.FormatValue(newVal))
 	return field.Invalid(fldPath, newVal, details)
 }

upup/pkg/fi/loader/BUILD.bazel

@@ -10,6 +10,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//upup/pkg/fi/utils:go_default_library",
+        "//util/pkg/reflectutils:go_default_library",
         "//util/pkg/vfs:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",