Make kctrl to exit smoothly on adding the package registry with no ch…

…anges (#1316) * Make kctrl to exit smoothly on adding the package registry with no changes Signed-off-by: rcmadhankumar <[email protected]> * Additonal checks added to the test cases Signed-off-by: rcmadhankumar <[email protected]> * review comments fixed Signed-off-by: rcmadhankumar <[email protected]> --------- Signed-off-by: rcmadhankumar <[email protected]>
carvel-dev · Oct 18, 2023 · 5b1294b · 5b1294b
1 parent 1a63f5f
commit 5b1294b
Show file tree

Hide file tree

Showing 5 changed files with 77 additions and 6 deletions.
diff --git a/cli/pkg/kctrl/cmd/package/repository/add_or_update.go b/cli/pkg/kctrl/cmd/package/repository/add_or_update.go
@@ -190,6 +190,10 @@ func (o *AddOrUpdateOptions) Run(args []string) error {
 		return err
 	}
 
+	if o.URL != "" && o.URL == existingRepository.Spec.Fetch.ImgpkgBundle.Image {
+		return NewRepoTailer(o.NamespaceFlags.Name, o.Name, o.ui, client, RepoTailerOpts{PrintCurrentState: true}).TailRepoStatus()
+	}
+
 	pkgRepository, err := o.updateExistingPackageRepository(existingRepository)
 	if err != nil {
 		return err
@@ -271,7 +275,7 @@ func (o *AddOrUpdateOptions) updateExistingPackageRepository(pkgr *kcpkg.Package
 
 func (o *AddOrUpdateOptions) waitForPackageRepositoryInstallation(client kcclient.Interface) error {
 	o.statusUI.PrintMessagef("Waiting for package repository reconciliation for '%s'", o.Name)
-	repoWatcher := NewRepoTailer(o.NamespaceFlags.Name, o.Name, o.ui, client)
+	repoWatcher := NewRepoTailer(o.NamespaceFlags.Name, o.Name, o.ui, client, RepoTailerOpts{})
 
 	err := repoWatcher.TailRepoStatus()
 	if err != nil {

diff --git a/cli/pkg/kctrl/cmd/package/repository/delete.go b/cli/pkg/kctrl/cmd/package/repository/delete.go
@@ -104,7 +104,7 @@ func (o *DeleteOptions) Run(args []string) error {
 
 func (o *DeleteOptions) waitForDeletion(client versioned.Interface) error {
 	o.statusUI.PrintMessagef("Waiting for package repository reconciliation for '%s'", o.Name)
-	repoWatcher := NewRepoTailer(o.NamespaceFlags.Name, o.Name, o.ui, client)
+	repoWatcher := NewRepoTailer(o.NamespaceFlags.Name, o.Name, o.ui, client, RepoTailerOpts{})
 
 	err := repoWatcher.TailRepoStatus()
 	if err != nil {

diff --git a/cli/pkg/kctrl/cmd/package/repository/kick.go b/cli/pkg/kctrl/cmd/package/repository/kick.go
@@ -152,7 +152,7 @@ func (o *KickOptions) triggerReconciliation(client kcclient.Interface) error {
 
 func (o *KickOptions) waitForReconciliation(client kcclient.Interface) error {
 	o.statusUI.PrintMessagef("Waiting for package repository reconciliation for '%s'", o.Name)
-	repoWatcher := NewRepoTailer(o.NamespaceFlags.Name, o.Name, o.ui, client)
+	repoWatcher := NewRepoTailer(o.NamespaceFlags.Name, o.Name, o.ui, client, RepoTailerOpts{})
 
 	err := repoWatcher.TailRepoStatus()
 	if err != nil {

diff --git a/cli/pkg/kctrl/cmd/package/repository/repo_tailer.go b/cli/pkg/kctrl/cmd/package/repository/repo_tailer.go
@@ -30,23 +30,40 @@ type RepoTailer struct {
 
 	stopperChan chan struct{}
 	watchError  error
+	opts        RepoTailerOpts
 
 	lastSeenDeployStdout string
 }
 
-func NewRepoTailer(namespace string, name string, ui ui.UI, client kcclient.Interface) *RepoTailer {
-	return &RepoTailer{Namespace: namespace, Name: name, ui: ui, statusUI: cmdcore.NewStatusLoggingUI(ui), client: client}
+type RepoTailerOpts struct {
+	PrintCurrentState bool
+}
+
+func NewRepoTailer(namespace string, name string, ui ui.UI, client kcclient.Interface, opts RepoTailerOpts) *RepoTailer {
+	return &RepoTailer{Namespace: namespace, Name: name, ui: ui, statusUI: cmdcore.NewStatusLoggingUI(ui), client: client, opts: opts}
 }
 
 func (o *RepoTailer) TailRepoStatus() error {
 	o.stopperChan = make(chan struct{})
-	_, err := o.client.PackagingV1alpha1().PackageRepositories(o.Namespace).Get(context.Background(), o.Name, metav1.GetOptions{})
+	pkgRepo, err := o.client.PackagingV1alpha1().PackageRepositories(o.Namespace).Get(context.Background(), o.Name, metav1.GetOptions{})
 	if err != nil {
 		if !(errors.IsNotFound(err)) {
 			return err
 		}
 	}
 
+	if o.opts.PrintCurrentState {
+		appStatus := o.appStatusFromPkgrStatus(pkgRepo.Status)
+		err = o.printTillCurrent(appStatus)
+		if err != nil {
+			return err
+		}
+
+		if cmdapp.HasReconciled(appStatus) {
+			return nil
+		}
+	}
+
 	informerFactory := kcexternalversions.NewFilteredSharedInformerFactory(o.client, 30*time.Minute, o.Namespace, func(opts *metav1.ListOptions) {
 		opts.FieldSelector = fmt.Sprintf("metadata.name=%s", o.Name)
 	})

diff --git a/cli/test/e2e/package_repository_test.go b/cli/test/e2e/package_repository_test.go
@@ -6,6 +6,7 @@ package e2e
 import (
 	"fmt"
 	"testing"
+	"time"
 
 	uitest "github.com/cppforlife/go-cli-ui/ui/test"
 	"github.com/stretchr/testify/require"
@@ -64,6 +65,43 @@ func TestPackageRepository(t *testing.T) {
 		kubectl.Run([]string{"get", "pkg/pkg.test.carvel.dev.2.0.0"})
 	})
 
+	logger.Section("adding of existing repository", func() {
+		start := time.Now()
+		out := kappCtrl.Run([]string{"package", "repository", "add", "-r", pkgrName, "--url", pkgrURL})
+		elapsed := time.Since(start).Seconds()
+		require.Equal(t, elapsed < 5, true, "Adding of existing package repository takes more than 5 seconds")
+		require.Contains(t, out, "Fetch succeeded")
+		require.Contains(t, out, "Template succeeded")
+		require.Contains(t, out, "Deploy succeeded")
+		require.Contains(t, out, "Succeeded")
+	})
+
+	logger.Section("adding of existing repository with new url", func() {
+
+		_, err := kappCtrl.RunWithOpts([]string{"package", "repository", "add", "-r", pkgrName, "--url", "https://carvel.dev"}, RunOpts{
+			AllowError: true,
+		})
+		require.Error(t, err)
+
+		kubectl.Run([]string{"get", kind, pkgrName})
+
+		kappCtrl.Run([]string{"package", "repository", "add", "-r", pkgrName, "--url", pkgrURL})
+
+		out := kappCtrl.Run([]string{"package", "repository", "get", "-r", pkgrName, "--json"})
+
+		output := uitest.JSONUIFromBytes(t, []byte(out))
+
+		expectedOutputRows := []map[string]string{{
+			"conditions":           "- type: ReconcileSucceeded\n  status: \"True\"\n  reason: \"\"\n  message: \"\"",
+			"status":               "Reconcile succeeded",
+			"namespace":            env.Namespace,
+			"name":                 pkgrName,
+			"source":               fmt.Sprintf("(imgpkg) %s", pkgrURL),
+			"useful_error_message": "",
+		}}
+		require.Exactly(t, expectedOutputRows, output.Tables[0].Rows)
+	})
+
 	logger.Section("kicking a repository", func() {
 		out := kappCtrl.Run([]string{"package", "repository", "kick", "-r", pkgrName})
 
@@ -130,6 +168,18 @@ func TestPackageRepository(t *testing.T) {
 		require.Exactly(t, expectedOutputRows, output.Tables[0].Rows)
 	})
 
+	logger.Section("updating a repository with no change in url", func() {
+		start := time.Now()
+		out := kappCtrl.Run([]string{"package", "repository", "update", "-r", pkgrName, "--url", pkgrURL})
+		elapsed := time.Since(start).Seconds()
+		require.Equal(t, elapsed < 5, true, "Adding of existing package repository takes more than 5 seconds")
+		require.Contains(t, out, "Fetch succeeded")
+		require.Contains(t, out, "Template succeeded")
+		require.Contains(t, out, "Deploy succeeded")
+		require.Contains(t, out, "Succeeded")
+
+	})
+
 	logger.Section("creating a repository in a new namespace that doesn't exist", func() {
 		kappCtrl.Run([]string{"package", "repository", "add", "-r", pkgrName, "--url", pkgrURL, "-n", newRepoNamespace, "--create-namespace"})