Merge pull request #427 from upbound/backport-426-to-release-0.24

[Backport release-0.24] Do not restore package statuses and use the same confirmation dialog as spaces init

cmd/up/migration/export.go

@@ -16,8 +16,8 @@ package migration
 
 import (
 	"context"
-	"fmt"
 
+	"github.com/pterm/pterm"
 	apiextensionsclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/discovery/cached/memory"
@@ -37,7 +37,7 @@ reconstructable post-migration. Consequently, the exported archive will incorpor
 those secrets by default. To exclude secrets from the export, please use the
 --excluded-resources flag.
 
-IMPORTANT: The exported archive will contain secrets. Do you wish to proceed? [y/n]`
+IMPORTANT: The exported archive will contain secrets. Do you wish to proceed?`
 
 type exportCmd struct {
 	prompter input.Prompter
@@ -117,15 +117,14 @@ func (c *exportCmd) Run(ctx context.Context, migCtx *migration.Context) error {
 	})
 
 	if !c.Yes && e.IncludedExtraResource("secrets") {
-		res, err := c.prompter.Prompt(secretsWarning, false)
-		if err != nil {
-			return err
-		}
-		if res != "y" {
+		confirm := pterm.DefaultInteractiveConfirm
+		confirm.DefaultText = secretsWarning
+		confirm.DefaultValue = true
+		result, _ := confirm.Show()
+		pterm.Println() // Blank line
+		if !result {
 			return nil
 		}
-		// Print a newline to separate the prompt from the output.
-		fmt.Println()
 	}
 
 	if err = e.Export(ctx); err != nil {

cmd/up/migration/import.go

@@ -21,6 +21,7 @@ import (
 	"strings"
 
 	"github.com/crossplane/crossplane-runtime/pkg/errors"
+	"github.com/pterm/pterm"
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/discovery/cached/memory"
 	"k8s.io/client-go/dynamic"
@@ -102,15 +103,16 @@ func (c *importCmd) Run(ctx context.Context, migCtx *migration.Context) error {
 			fmt.Println("- " + err.Error())
 		}
 		if !c.Yes {
-			res, err := c.prompter.Prompt("Do you still wish to proceed? [y/n]", false)
-			if err != nil {
-				return err
-			}
-			if res != "y" {
+			pterm.Println() // Blank line
+			confirm := pterm.DefaultInteractiveConfirm
+			confirm.DefaultText = "Do you still want to proceed?"
+			confirm.DefaultValue = false
+			result, _ := confirm.Show()
+			pterm.Println() // Blank line
+			if !result {
+				pterm.Error.Println("Preflight checks must pass in order to proceed with the import.")
 				return nil
 			}
-			// Print a newline to separate the prompt from the output.
-			fmt.Println()
 		}
 	}
 

internal/migration/importer/import.go

@@ -138,7 +138,7 @@ func (im *ControlPlaneStateImporter) Import(ctx context.Context) error { // noli
 	s, _ = upterm.CheckmarkSuccessSpinner.Start(importBaseMsg + fmt.Sprintf("0 / %d", len(baseResources)))
 	baseCounts := make(map[string]int, len(baseResources))
 	for i, gr := range baseResources {
-		count, err := r.ImportResources(ctx, gr)
+		count, err := r.ImportResources(ctx, gr, false)
 		if err != nil {
 			s.Fail(importBaseMsg + "Failed!")
 			return errors.Wrapf(err, "cannot import %q resources", gr)
@@ -175,21 +175,23 @@ func (im *ControlPlaneStateImporter) Import(ctx context.Context) error { // noli
 			return errors.Wrapf(err, "there are unhealthy %qs", k.Kind)
 		}
 	}
-	s.Success(waitPkgsMsg + "Installed and Healthy! ⏳")
 
-	waitPkgRevsMsg := "Waiting for PackageRevisions... "
-	s, _ = upterm.CheckmarkSuccessSpinner.Start(waitPkgRevsMsg)
+	// Note(turkenh): We should not need to wait for ProviderRevision, FunctionRevision, and ConfigurationRevision.
+	// Crossplane should not report packages as ready before revisions are healthy. This is a bug in Crossplane
+	// version <1.14 which was fixed with https://github.com/crossplane/crossplane/pull/4647
+	// Todo(turkenh): Remove these once Crossplane 1.13 is no longer supported.
 	for _, k := range []schema.GroupKind{
 		{Group: "pkg.crossplane.io", Kind: "ProviderRevision"},
 		{Group: "pkg.crossplane.io", Kind: "FunctionRevision"},
 		{Group: "pkg.crossplane.io", Kind: "ConfigurationRevision"},
 	} {
 		if err := im.waitForConditions(ctx, s, k, []xpv1.ConditionType{"Healthy"}); err != nil {
-			s.Fail(waitPkgRevsMsg + "Failed!")
+			s.Fail(waitPkgsMsg + "Failed!")
 			return errors.Wrapf(err, "there are unhealthy %qs", k.Kind)
 		}
 	}
-	s.Success(waitPkgRevsMsg + "Healthy! ⏳")
+
+	s.Success(waitPkgsMsg + "Installed and Healthy! ⏳")
 	//////////////////////////////////////////
 
 	// Reset the resource mapper to make sure all CRDs introduced by packages or XRDs are available.
@@ -218,7 +220,7 @@ func (im *ControlPlaneStateImporter) Import(ctx context.Context) error { // noli
 			continue
 		}
 
-		count, err := r.ImportResources(ctx, info.Name())
+		count, err := r.ImportResources(ctx, info.Name(), true)
 		if err != nil {
 			return errors.Wrapf(err, "cannot import %q resources", info.Name())
 		}
@@ -403,14 +405,15 @@ func (im *ControlPlaneStateImporter) waitForConditions(ctx context.Context, sp *
 		for _, r := range resourceList.Items {
 			paved := fieldpath.Pave(r.Object)
 			status := xpv1.ConditionedStatus{}
-			if err = paved.GetValueInto("status", &status); err != nil {
+			if err = paved.GetValueInto("status", &status); err != nil && !fieldpath.IsNotFound(err) {
 				pterm.Printf("cannot get status for %q %q with error: %v\n", gk.Kind, r.GetName(), err)
 				return
 			}
 
 			for _, c := range conditions {
 				if status.GetCondition(c).Status != corev1.ConditionTrue {
 					unmet++
+					break // At least one condition is not met, so we should break and not count the same resource multiple times.
 				}
 			}
 		}

internal/migration/importer/resource.go

@@ -22,7 +22,7 @@ import (
 )
 
 type ResourceImporter interface {
-	ImportResources(ctx context.Context, gr string) (int, error)
+	ImportResources(ctx context.Context, gr string, restoreStatus bool) (int, error)
 }
 
 type PausingResourceImporter struct {
@@ -37,15 +37,15 @@ func NewPausingResourceImporter(r ResourceReader, a ResourceApplier) *PausingRes
 	}
 }
 
-func (im *PausingResourceImporter) ImportResources(ctx context.Context, gr string) (int, error) {
+func (im *PausingResourceImporter) ImportResources(ctx context.Context, gr string, restoreStatus bool) (int, error) {
 	resources, typeMeta, err := im.reader.ReadResources(gr)
 	if err != nil {
 		return 0, errors.Wrapf(err, "cannot get %q resources", gr)
 	}
 
-	sub := false
+	hasSubresource := false
 	if typeMeta != nil {
-		sub = typeMeta.WithStatusSubresource
+		hasSubresource = typeMeta.WithStatusSubresource
 		for _, c := range typeMeta.Categories {
 			// We pause all resources that are managed, claim, or composite.
 			// - Claim/Composite: We don't want Crossplane controllers to create new resources before we import all.
@@ -61,7 +61,7 @@ func (im *PausingResourceImporter) ImportResources(ctx context.Context, gr strin
 		}
 	}
 
-	if err = im.applier.ApplyResources(ctx, resources, sub); err != nil {
+	if err = im.applier.ApplyResources(ctx, resources, restoreStatus && hasSubresource); err != nil {
 		return 0, errors.Wrapf(err, "cannot apply %q resources", gr)
 	}