-
Notifications
You must be signed in to change notification settings - Fork 193
Add reconciliation wait for repository operations and package update #390
Add reconciliation wait for repository operations and package update #390
Conversation
b18a149
to
69c9425
Compare
a2e2be0
to
9f25998
Compare
52ad9c3
to
5158c6c
Compare
5158c6c
to
43c5a3d
Compare
325d2ec
to
11782fa
Compare
Cluster Generation A/B Results: |
11782fa
to
46be7b3
Compare
Cluster Generation A/B Results: |
46be7b3
to
ab01e97
Compare
Cluster Generation A/B Results: |
e855afb
to
145e680
Compare
Cluster Generation A/B Results: |
145e680
to
3322e57
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Cluster Generation A/B Results: |
Cluster Generation A/B Results: |
LGTM |
packageInstallOp.PkgInstallName = args[0] | ||
|
||
cmd.SilenceUsage = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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
cmd/cli/plugin/package/utils.go
Outdated
@@ -0,0 +1,76 @@ | |||
// Copyright 2021 VMware, Inc. All Rights Reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[suggestion]: Consider renaming to display_utils.go so that this file doesn't become a dumping ground for unrelated functions.
cmd/cli/plugin/package/utils.go
Outdated
"github.com/vmware-tanzu/tanzu-framework/pkg/v1/tkg/tkgpackagedatamodel" | ||
) | ||
|
||
func displayProgress(initialMsg string, pp *tkgpackagedatamodel.PackageProgress) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this function be exported?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least we don't have a use case for that for now. But sure, will export it; maybe other plugins find it useful.
pkg/v1/tkg/tkgpackageclient/utils.go
Outdated
@@ -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 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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>
Signed-off-by: Marjan Alavi <malavi@vmware.com>
3322e57
to
ba67b38
Compare
Cluster Generation A/B Results: |
What this PR does / why we need it:
tanzu package installed update ... --wait
does not wait #968Describe testing done for PR:
Manual testing in the cluster and existing unit tests & integration tests.
Example outputs:
Does this PR introduce a user-facing change?:
None
Release note:
New PR Checklist