Add reconciliation wait for repository operations and package update #390

maralavi · 2021-08-12T23:51:40Z

What this PR does / why we need it:

This PR is to add reconciliation wait for package plugin repository operations and package update. It also adds progress indicators for repository operations.
Improves the processing of warning messages
It also fixes CLI plugins should not print usage unless -h is used #736 for package Plugin
Fixes tanzu package installed update ... --wait does not wait #968

Describe testing done for PR:
Manual testing in the cluster and existing unit tests & integration tests.

Example outputs:

tanzu package repository add test-repo -n test-ns --url <REPOSITORY-URL> --create-namespace

- Validating provided settings for the package repository
/ Creating namespace 'test-ns'
- Creating package repository resource
\ Resource install status: Reconciling

 Added package repository 'test-repo'

anzu package repository delete test-repo -n test-ns 
/ Deleting package repository 'test-repo'
\ Resource deletion status: Deleting

 Deleted package repository 'test-repo' from namespace 'test-ns'

Does this PR introduce a user-facing change?:
None

Release note:

Adds reconciliation wait for package plugin repository operations and package update; also adds progress indicators for repository operations.

New PR Checklist

Ensure PR contains only public links or terms
Use good commit messages
Squash the commits in this branch before merge to preserve our git history
If this PR is just an idea or POC, use a Draft PR instead of a full PR
Add appropriate kind label according to what type of issue is being addressed.

cmd/cli/plugin/package/utils.go

cmd/cli/plugin/package/package_install.go

github-actions · 2021-09-29T20:59:09Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/390/20210929205907/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2021-09-29T21:17:18Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/390/20210929211716/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2021-09-29T22:45:10Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/390/20210929224507/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2021-09-29T23:43:27Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/390/20210929234325/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

blc1996

LGTM

github-actions · 2021-09-29T23:50:40Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/390/20210929235038/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2021-09-29T23:54:13Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/390/20210929235411/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

ggpaue · 2021-09-30T17:34:37Z

LGTM

vijaykatam · 2021-09-30T21:58:27Z

cmd/cli/plugin/package/package_install.go

 	packageInstallOp.PkgInstallName = args[0]

+	cmd.SilenceUsage = true


Should this be set at cmd root level similar to https://github.com/vmware-tanzu/tanzu-framework/blob/main/pkg/v1/cli/command/core/root.go#L49?

@vijaykatam we ideally want to show the usage info for the cases were there was a mistake from the user on the flags themselves. Putting this in root level, suppresses usage message for such cases too

vijaykatam · 2021-09-30T22:07:05Z

cmd/cli/plugin/package/utils.go

@@ -0,0 +1,76 @@
+// Copyright 2021 VMware, Inc. All Rights Reserved.


[suggestion]: Consider renaming to display_utils.go so that this file doesn't become a dumping ground for unrelated functions.

vijaykatam · 2021-09-30T22:07:53Z

cmd/cli/plugin/package/utils.go

+	"github.com/vmware-tanzu/tanzu-framework/pkg/v1/tkg/tkgpackagedatamodel"
+)
+
+func displayProgress(initialMsg string, pp *tkgpackagedatamodel.PackageProgress) error {


Should this function be exported?

At least we don't have a use case for that for now. But sure, will export it; maybe other plugins find it useful.

vijaykatam · 2021-09-30T22:13:12Z

pkg/v1/tkg/tkgpackageclient/utils.go

@@ -104,3 +110,89 @@ func (parser *PackageValuesSchemaParser) walkOnValueSchemaProperties(docMap map[
 	}
 	return nil
 }
+
+// waitForResourceInstallation waits until the package get installed successfully or a failure happen
+func (p *pkgClient) waitForResourceInstallation(name, namespace string, pollInterval, pollTimeout time.Duration, progress chan string, rscType tkgpackagedatamodel.ResourceType) error {


I am confused why these functions are in utils but still have pkgClient as a receiver. If these are indeed generic utilities then the functions should be exported with their own struct type such as PackageValuesSchemaParser and additionally we should add unit test coverage.

@vijaykatam I had added in utils for package plugin assuming it was a file for shared functions and I wasn't planning to make these two functions generic for any other type. But as these functions rely on the functions defined in the pkgClient receiver and cause they are used by package and repository install, update, delete and they have become confusing the the utils.go, I just moved those to say package install and delete, as unit tests related to these wait functionalities are already in those corresponding test files.

Signed-off-by: Marjan Alavi <malavi@vmware.com>

github-actions · 2021-09-30T23:12:09Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/390/20210930231207/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

maralavi requested review from blc1996, danniel1205, ggpaue, shivaani0505, shyaamsn and a team as code owners August 12, 2021 23:51

vmwclabot added the dco-required label Aug 12, 2021

maralavi assigned vijaykatam Aug 17, 2021

maralavi force-pushed the package-plugin-add-reconcilliation-wait branch from b18a149 to 69c9425 Compare August 19, 2021 23:04

vmwclabot removed the dco-required label Aug 19, 2021

maralavi force-pushed the package-plugin-add-reconcilliation-wait branch from a2e2be0 to 9f25998 Compare August 20, 2021 06:07

shyaamsn assigned maralavi and blc1996 Sep 27, 2021

shyaamsn added the area/addons label Sep 28, 2021

maralavi force-pushed the package-plugin-add-reconcilliation-wait branch from 52ad9c3 to 5158c6c Compare September 28, 2021 20:13

maralavi added the kind/feature Categorizes issue or PR as related to a new feature label Sep 28, 2021

vmware-tanzu deleted a comment from github-actions bot Sep 28, 2021

vmware-tanzu deleted a comment from vmwclabot Sep 28, 2021

vmware-tanzu deleted a comment from github-actions bot Sep 28, 2021

maralavi added cla-not-required ok-to-merge PRs should be labelled with this before merging and removed ok-to-merge PRs should be labelled with this before merging cla-not-required labels Sep 28, 2021

maralavi force-pushed the package-plugin-add-reconcilliation-wait branch from 5158c6c to 43c5a3d Compare September 28, 2021 20:50

maralavi added area/cli area/plugin labels Sep 28, 2021

ggpaue reviewed Sep 29, 2021

View reviewed changes

cmd/cli/plugin/package/utils.go Outdated Show resolved Hide resolved

ggpaue reviewed Sep 29, 2021

View reviewed changes

cmd/cli/plugin/package/package_install.go Show resolved Hide resolved

maralavi force-pushed the package-plugin-add-reconcilliation-wait branch from 325d2ec to 11782fa Compare September 29, 2021 20:46

maralavi force-pushed the package-plugin-add-reconcilliation-wait branch from 11782fa to 46be7b3 Compare September 29, 2021 21:07

maralavi force-pushed the package-plugin-add-reconcilliation-wait branch from 46be7b3 to ab01e97 Compare September 29, 2021 22:31

maralavi force-pushed the package-plugin-add-reconcilliation-wait branch 2 times, most recently from e855afb to 145e680 Compare September 29, 2021 23:39

maralavi force-pushed the package-plugin-add-reconcilliation-wait branch from 145e680 to 3322e57 Compare September 29, 2021 23:44

blc1996 approved these changes Sep 29, 2021

View reviewed changes

ggpaue self-requested a review September 30, 2021 17:32

shyaamsn added the area/addons label Sep 30, 2021

blc1996 mentioned this pull request Sep 30, 2021

package update: make version flag optional #462

Closed

5 tasks

vijaykatam reviewed Sep 30, 2021

View reviewed changes

maralavi added 2 commits September 30, 2021 15:45

Add reconcilliation wait for repository operations and package update

2dfafa8

Signed-off-by: Marjan Alavi <malavi@vmware.com>

addressed comments

ba67b38

Signed-off-by: Marjan Alavi <malavi@vmware.com>

maralavi force-pushed the package-plugin-add-reconcilliation-wait branch from 3322e57 to ba67b38 Compare September 30, 2021 23:00

vijaykatam approved these changes Sep 30, 2021

View reviewed changes

vijaykatam added the ok-to-merge PRs should be labelled with this before merging label Sep 30, 2021

maralavi merged commit d15a82a into vmware-tanzu:main Oct 1, 2021

maralavi deleted the package-plugin-add-reconcilliation-wait branch October 1, 2021 16:41

maralavi mentioned this pull request Oct 1, 2021

CLI plugins should not print usage unless -h is used #736

Closed

9 tasks

maralavi mentioned this pull request Oct 26, 2021

tanzu package installed update ... --wait does not wait #968

Closed

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add reconciliation wait for repository operations and package update #390

Add reconciliation wait for repository operations and package update #390

maralavi commented Aug 12, 2021 •

edited

Loading

github-actions bot commented Sep 29, 2021

github-actions bot commented Sep 29, 2021

github-actions bot commented Sep 29, 2021

github-actions bot commented Sep 29, 2021

blc1996 left a comment

github-actions bot commented Sep 29, 2021

github-actions bot commented Sep 29, 2021

ggpaue commented Sep 30, 2021

vijaykatam Sep 30, 2021

maralavi Sep 30, 2021

vijaykatam Sep 30, 2021

vijaykatam Sep 30, 2021

maralavi Sep 30, 2021

vijaykatam Sep 30, 2021

maralavi Sep 30, 2021 •

edited

Loading

github-actions bot commented Sep 30, 2021

		packageInstallOp.PkgInstallName = args[0]

		cmd.SilenceUsage = true

		@@ -0,0 +1,76 @@
		// Copyright 2021 VMware, Inc. All Rights Reserved.

Add reconciliation wait for repository operations and package update #390

Add reconciliation wait for repository operations and package update #390

Conversation

maralavi commented Aug 12, 2021 • edited Loading

github-actions bot commented Sep 29, 2021

github-actions bot commented Sep 29, 2021

github-actions bot commented Sep 29, 2021

github-actions bot commented Sep 29, 2021

blc1996 left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 29, 2021

github-actions bot commented Sep 29, 2021

ggpaue commented Sep 30, 2021

vijaykatam Sep 30, 2021

Choose a reason for hiding this comment

maralavi Sep 30, 2021

Choose a reason for hiding this comment

vijaykatam Sep 30, 2021

Choose a reason for hiding this comment

vijaykatam Sep 30, 2021

Choose a reason for hiding this comment

maralavi Sep 30, 2021

Choose a reason for hiding this comment

vijaykatam Sep 30, 2021

Choose a reason for hiding this comment

maralavi Sep 30, 2021 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Sep 30, 2021

maralavi commented Aug 12, 2021 •

edited

Loading

maralavi Sep 30, 2021 •

edited

Loading