From 5b1294bdf60107c9ff20b79e519abaefd71c4682 Mon Sep 17 00:00:00 2001 From: Madhankumar Chellamuthu Date: Thu, 19 Oct 2023 02:02:55 +0530 Subject: [PATCH] Make kctrl to exit smoothly on adding the package registry with no changes (#1316) * Make kctrl to exit smoothly on adding the package registry with no changes Signed-off-by: rcmadhankumar * Additonal checks added to the test cases Signed-off-by: rcmadhankumar * review comments fixed Signed-off-by: rcmadhankumar --------- Signed-off-by: rcmadhankumar --- .../cmd/package/repository/add_or_update.go | 6 ++- .../kctrl/cmd/package/repository/delete.go | 2 +- cli/pkg/kctrl/cmd/package/repository/kick.go | 2 +- .../cmd/package/repository/repo_tailer.go | 23 +++++++-- cli/test/e2e/package_repository_test.go | 50 +++++++++++++++++++ 5 files changed, 77 insertions(+), 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 index a2200f730..f9ede874f 100644 --- 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 index 6e6b3b734..6f06f67ac 100644 --- 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 index bd6077163..5d33030d2 100644 --- 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 index b3a39f5db..1ff602e94 100644 --- 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 index 75d5b5c61..0bc735692 100644 --- 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"})