Fix dry run output for migrate providerid

Author: detiber

cmd/helper/base/testutils/testutils.go

@@ -36,6 +36,7 @@ import (
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
+	"sigs.k8s.io/cluster-api-provider-packet/cmd/helper/base"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
 	"sigs.k8s.io/cluster-api/controllers/remote"
 	"sigs.k8s.io/cluster-api/util"
@@ -383,26 +384,26 @@ func VerifySuccessOutputDryRun(t *testing.T, output string) {
 	t.Helper()
 	g := gomega.NewWithT(t)
 	// TODO: update prefix strings to be variables instead of hardcoded
-	g.Expect(output).To(gomega.HavePrefix("(Dry Run)"))
+	g.Expect(output).To(gomega.HavePrefix(base.DryRunPrefix))
 }
 
 func VerifySuccessOutputChanged(t *testing.T, output string) {
 	t.Helper()
 	g := gomega.NewWithT(t)
 	// TODO: update prefix strings to be variables instead of hardcoded
-	g.Expect(output).To(gomega.HavePrefix("✅"))
+	g.Expect(output).To(gomega.HavePrefix(base.SuccessPrefix))
 }
 
 func VerifySuccessOutputUnchanged(t *testing.T, output string) {
 	t.Helper()
 	g := gomega.NewWithT(t)
 	// TODO: update prefix strings to be variables instead of hardcoded
-	g.Expect(output).To(gomega.HavePrefix("✔"))
+	g.Expect(output).To(gomega.HavePrefix(base.NoOpPrefix))
 }
 
 func VerifySuccessOutputSkipped(t *testing.T, output string) {
 	t.Helper()
 	g := gomega.NewWithT(t)
 	// TODO: update prefix strings to be variables instead of hardcoded
-	g.Expect(output).To(gomega.HavePrefix("Skipping "))
+	g.Expect(output).To(gomega.HavePrefix(base.SkipPrefix))
 }

cmd/helper/base/tool.go

@@ -43,6 +43,13 @@ import (
 	"sigs.k8s.io/yaml"
 )
 
+const (
+	DryRunPrefix  = "(Dry Run) "
+	SuccessPrefix = "✅ "
+	NoOpPrefix    = "✔ "
+	SkipPrefix    = "Skipping "
+)
+
 func ObjectToName(obj controllerutil.Object) string {
 	if obj.GetNamespace() != "" {
 		return fmt.Sprintf("%s/%s", obj.GetNamespace(), obj.GetName())
@@ -172,20 +179,21 @@ func (t *Tool) WorkloadPatchOrCreateUnstructured(
 			}
 
 			diff := cmp.Diff(redactedExisting, redacted, diff.IgnoreUnset())
-			logger.Info("(Dry Run) Would patch resource", "kind", obj.GetKind(), "name", name, "diff", diff)
-			fmt.Fprintf(t.GetBufferFor(c), "(Dry Run) Would patch %s %s\n%s\n", obj.GetKind(), name, indent.String(diff, 4))
+			logger.Info(DryRunPrefix+"Would patch resource", "kind", obj.GetKind(), "name", name, "diff", diff)
+			fmt.Fprintf(t.GetBufferFor(c), "%sWould patch %s %s\n%s\n",
+				DryRunPrefix, obj.GetKind(), name, indent.String(diff, 4))
 
 			return nil
 		}
 
 		logger.Info("Successfully patched resource", "kind", obj.GetKind(), "name", name)
-		fmt.Fprintf(t.GetBufferFor(c), "✅ %s %s has been successfully patched\n", obj.GetKind(), name)
+		fmt.Fprintf(t.GetBufferFor(c), "%s%s %s has been successfully patched\n", SuccessPrefix, obj.GetKind(), name)
 	}
 
 	kind := obj.GetObjectKind().GroupVersionKind().Kind
 	name := ObjectToName(obj)
 	logger.Info("Resource already up to date", "kind", kind, "name", name)
-	fmt.Fprintf(stdout, "✔ %s %s already up to date\n", kind, name)
+	fmt.Fprintf(stdout, "%s%s %s already up to date\n", NoOpPrefix, kind, name)
 
 	return nil
 }
@@ -269,14 +277,14 @@ func (t *Tool) WorkloadCreate(
 		// Convert the resource into yaml for printing
 		data, _ := yaml.Marshal(redacted)
 
-		logger.Info("(Dry Run) Would create resource", "kind", gvk.Kind, "name", name, "object", data)
-		fmt.Fprintf(t.GetBufferFor(c), "(Dry Run) Would create %s %s\n%s", gvk.Kind, name, indent.String(string(data), 4))
+		logger.Info(DryRunPrefix+"Would create resource", "kind", gvk.Kind, "name", name, "object", data)
+		fmt.Fprintf(t.GetBufferFor(c), "%sWould create %s %s\n%s", DryRunPrefix, gvk.Kind, name, indent.String(string(data), 4))
 
 		return nil
 	}
 
 	logger.Info("Successfully created resource", "kind", gvk.Kind, "name", name)
-	fmt.Fprintf(t.GetBufferFor(c), "✅ %s %s has been successfully created\n", gvk.Kind, name)
+	fmt.Fprintf(t.GetBufferFor(c), "%s%s %s has been successfully created\n", SuccessPrefix, gvk.Kind, name)
 
 	return nil
 }
@@ -309,14 +317,14 @@ func (t *Tool) WorkloadDelete(
 	name := ObjectToName(obj)
 
 	if t.DryRun() {
-		logger.Info("(Dry Run) Would delete resource", "kind", gvk.Kind, "name", name)
-		fmt.Fprintf(t.GetBufferFor(c), "(Dry Run) Would delete %s %s\n", gvk.Kind, name)
+		logger.Info(DryRunPrefix+"Would delete resource", "kind", gvk.Kind, "name", name)
+		fmt.Fprintf(t.GetBufferFor(c), "%sWould delete %s %s\n", DryRunPrefix, gvk.Kind, name)
 
 		return nil
 	}
 
 	logger.Info("Successfully deleted resource", "kind", gvk.Kind, "name", name)
-	fmt.Fprintf(t.GetBufferFor(c), "✅ %s %s has been successfully deleted\n", gvk.Kind, name)
+	fmt.Fprintf(t.GetBufferFor(c), "%s%s %s has been successfully deleted\n", SuccessPrefix, gvk.Kind, name)
 
 	return nil
 }

cmd/helper/migrate/providerid/migrator/migrator.go

@@ -19,10 +19,12 @@ import (
 	"fmt"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/blang/semver"
 	"github.com/docker/distribution/reference"
 	"github.com/go-logr/logr"
+	"github.com/muesli/reflow/indent"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -33,6 +35,7 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
 	capiutil "sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/yaml"
 )
 
 const (
@@ -357,7 +360,7 @@ func (m *Migrator) MigrateNode(ctx context.Context, node *corev1.Node, c *cluste
 
 	if strings.HasPrefix(node.Spec.ProviderID, newProviderIDPrefix) {
 		logger.Info("Node already has the updated providerID")
-		fmt.Fprintf(m.GetBufferFor(c), "✔ Node %s already has the updated providerID\n", node.Name)
+		fmt.Fprintf(m.GetBufferFor(c), base.NoOpPrefix+"Node %s already has the updated providerID\n", node.Name)
 		m.updateNodeStatus(c, node, true)
 
 		return nil
@@ -378,10 +381,17 @@ func (m *Migrator) MigrateNode(ctx context.Context, node *corev1.Node, c *cluste
 		func() error {
 			if err := m.WorkloadCreate(ctx, logger, c, node); err != nil {
 				if m.DryRun() && apierrors.IsAlreadyExists(err) {
-					// TODO: output diff
+					node.SetManagedFields(nil)
+					node.SetCreationTimestamp(metav1.NewTime(time.Time{}))
+					node.SetUID("")
+					node.SetSelfLink("")
+
+					// Convert the resource into yaml for printing
+					data, _ := yaml.Marshal(node)
+
 					// add dry run success output here since Create will fail with an already exists error during dry run
-					logger.Info("(Dry Run) Would create Node")
-					fmt.Fprintf(m.GetBufferFor(c), "(Dry Run) Would create Node %s\n", base.ObjectToName(node))
+					logger.Info(base.DryRunPrefix+"Would create Node", "Node", data)
+					fmt.Fprintf(m.GetBufferFor(c), base.DryRunPrefix+"Would create Node %s\n%s", base.ObjectToName(node), indent.String(string(data), 4))
 
 					return nil
 				}

cmd/helper/migrate/providerid/migrator/migrator_test.go

@@ -16,6 +16,7 @@ package migrator_test
 import (
 	"context"
 	"math/rand"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -240,10 +241,7 @@ func TestMigrator_MigrateNodeDry(t *testing.T) {
 	g.Expect(postDryRunNode).To(testutils.BeDerivativeOf(expectedNode))
 
 	postDryRunOutputDiff := strings.TrimPrefix(postDryRunOutput, preDryRunOutput)
-	postDryRunOutputLines := strings.Split(postDryRunOutputDiff, "\n")
-	g.Expect(postDryRunOutputLines).To(HaveLen(3))
-	testutils.VerifySuccessOutputDryRun(t, postDryRunOutputLines[0])
-	testutils.VerifySuccessOutputDryRun(t, postDryRunOutputLines[1])
+	g.Expect(postDryRunOutputDiff).To(MatchRegexp("^(%s.*\n){2}?", regexp.QuoteMeta(base.DryRunPrefix)))
 
 	actualPostDryRunNode := new(corev1.Node)
 	nodeKey := client.ObjectKey{Name: node.Name} //nolint:exhaustivestruct

cmd/helper/upgrade/cloudprovider/upgrader/upgrader.go

@@ -303,8 +303,8 @@ func removeOldCCMSecret(ctx context.Context, logger logr.Logger, u *base.Tool, c
 	case err != nil && !apierrors.IsNotFound(err):
 		return err
 	case err == nil:
-		logger.Info("Skipping removal of secret because Packet CSI is deployed", "name", secretName)
-		fmt.Fprintf(stdout, "Skipping removal of Secret %s because Packet CSI is deployed", secretName)
+		logger.Info(base.SkipPrefix+"removal of secret because Packet CSI is deployed", "name", secretName)
+		fmt.Fprintf(stdout, "%sremoval of Secret %s because Packet CSI is deployed", base.SkipPrefix, secretName)
 
 		return nil
 	}
@@ -313,7 +313,7 @@ func removeOldCCMSecret(ctx context.Context, logger logr.Logger, u *base.Tool, c
 	if err := u.WorkloadGet(ctx, c, ccmSecretKey, ccmSecret); err != nil {
 		if apierrors.IsNotFound(err) {
 			logger.Info("Secret already removed", "name", secretName)
-			fmt.Fprintf(stdout, "✔ Secret %s already deleted\n", secretName)
+			fmt.Fprintf(stdout, "%sSecret %s already deleted\n", base.NoOpPrefix, secretName)
 
 			return nil
 		}
@@ -333,7 +333,7 @@ func removeCCMDeployment(ctx context.Context, logger logr.Logger, u *base.Tool,
 	if err := u.WorkloadGet(ctx, c, ccmKey, ccmDeployment); err != nil {
 		if apierrors.IsNotFound(err) {
 			logger.Info("Deployment already removed", "name", deploymentName)
-			fmt.Fprintf(stdout, "✔ Deployment %s already deleted\n", deploymentName)
+			fmt.Fprintf(stdout, "%sDeployment %s already deleted\n", base.NoOpPrefix, deploymentName)
 
 			return nil
 		}
@@ -359,7 +359,7 @@ func migrateSecret(ctx context.Context, logger logr.Logger, u *base.Tool, c *clu
 		// If there was no error, then the secret already exists and there is no need to proceed
 
 		logger.Info("Secret already exists", "name", cpemSecretName)
-		fmt.Fprintf(stdout, "✔ Secret %s/%s already exists\n", cpemSecret.Namespace, cpemSecret.Name)
+		fmt.Fprintf(stdout, "%sSecret %s/%s already exists\n", base.NoOpPrefix, cpemSecret.Namespace, cpemSecret.Name)
 
 		return nil
 	}