Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

List plugins deactivated for context unset and context delete #483

Merged
merged 1 commit into from
Sep 29, 2023
Merged

List plugins deactivated for context unset and context delete #483

merged 1 commit into from
Sep 29, 2023

Conversation

chandrareddyp
Copy link
Contributor

@chandrareddyp chandrareddyp commented Sep 15, 2023 •
edited
Loading

What this PR does / why we need it

This pull request enhances the user experience for the tanzu context delete and tanzu context unset commands by listing all installed plugins as deactivated.

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR


1. E2E tests are updated to test the `tanzu context unset `and `tanzu context delete` use cases.

2. Manual testing has done, below are the details for `tanzu context unset `and `tanzu context delete` use cases
❯ t context create --endpoint unstable.tmc-dev.cloud.vmware.com --staging  --name tmc2   
Flag --name has been deprecated, it has been replaced by using an argument to the command
[i] API token env var is set

[ok] successfully created a TMC context
[i] Checking for required plugins...
[i] Installing plugin 'account:v0.1.3' with target 'mission-control'
[i] Plugin binary for 'account:v0.1.3' found in cache
[i] Installing plugin 'iam:v0.1.3' with target 'mission-control'
[i] Plugin binary for 'iam:v0.1.3' found in cache
[i] Installing plugin 'cluster:v0.1.6' with target 'mission-control'
[i] Plugin binary for 'cluster:v0.1.6' found in cache
[i] Successfully installed all required plugins
❯ 
❯ t context unset tmc2
The context tmc2 for the target mission-control has been set as inactive
Deactivating plugin 'continuousdelivery:v0.1.3' for context 'tmc2'
Deactivating plugin 'data-protection:v0.1.3' for context 'tmc2'
Deactivating plugin 'secret:v0.1.3' for context 'tmc2'
Deactivating plugin 'workspace:v0.1.5' for context 'tmc2'
Deactivating plugin 'audit:v0.1.3' for context 'tmc2'
Deactivating plugin 'helm:v0.1.3' for context 'tmc2'
Deactivating plugin 'policy:v0.1.3' for context 'tmc2'
Deactivating plugin 'tanzupackage:v0.2.2' for context 'tmc2'
Deactivating plugin 'clustergroup:v0.1.3' for context 'tmc2'
Deactivating plugin 'provider-aks-cluster:v0.1.3' for context 'tmc2'
Deactivating plugin 'events:v0.1.3' for context 'tmc2'
Deactivating plugin 'inspection:v0.1.3' for context 'tmc2'
Deactivating plugin 'apply:v0.3.0' for context 'tmc2'
Deactivating plugin 'agentartifacts:v0.1.3' for context 'tmc2'
Deactivating plugin 'integration:v0.1.3' for context 'tmc2'
Deactivating plugin 'management-cluster:v0.1.3' for context 'tmc2'
Deactivating plugin 'ekscluster:v0.1.3' for context 'tmc2'
Deactivating plugin 'provider-eks-cluster:v0.1.3' for context 'tmc2'
Deactivating plugin 'setting:v0.2.1' for context 'tmc2'
Deactivating plugin 'aks-cluster:v0.1.6' for context 'tmc2'
❯
❯ t context delete tmc2
Deleting the context entry from the config will remove it from the list of tracked contexts. You will need to use `tanzu context create` to re-create this context. Are you sure you want to continue? [y/N]: y
[i] Deleting entry for cluster tmc2
❯ 
❯ t context create --endpoint unstable.tmc-dev.cloud.vmware.com --staging  --name tmc2
Flag --name has been deprecated, it has been replaced by using an argument to the command
[i] API token env var is set

[ok] successfully created a TMC context
[i] Checking for required plugins...
[i] Installing plugin 'cluster:v0.1.6' with target 'mission-control'
[i] Plugin binary for 'cluster:v0.1.6' found in cache
[i] Installing plugin 'iam:v0.1.3' with target 'mission-control'
[i] Plugin binary for 'iam:v0.1.3' found in cache
[i] Installing plugin 'account:v0.1.3' with target 'mission-control'
[i] Plugin binary for 'account:v0.1.3' found in cache
[i] Successfully installed all required plugins
❯ t context delete tmc2
Deleting the context entry from the config will remove it from the list of tracked contexts. You will need to use `tanzu context create` to re-create this context. Are you sure you want to continue? [y/N]: y
[i] Deleting entry for cluster tmc2
Deactivating plugin 'audit:v0.1.3' for context 'tmc2'
Deactivating plugin 'helm:v0.1.3' for context 'tmc2'
Deactivating plugin 'policy:v0.1.3' for context 'tmc2'
Deactivating plugin 'continuousdelivery:v0.1.3' for context 'tmc2'
Deactivating plugin 'data-protection:v0.1.3' for context 'tmc2'
Deactivating plugin 'secret:v0.1.3' for context 'tmc2'
Deactivating plugin 'workspace:v0.1.5' for context 'tmc2'
Deactivating plugin 'inspection:v0.1.3' for context 'tmc2'
Deactivating plugin 'apply:v0.3.0' for context 'tmc2'
Deactivating plugin 'tanzupackage:v0.2.2' for context 'tmc2'
Deactivating plugin 'clustergroup:v0.1.3' for context 'tmc2'
Deactivating plugin 'provider-aks-cluster:v0.1.3' for context 'tmc2'
Deactivating plugin 'events:v0.1.3' for context 'tmc2'
Deactivating plugin 'agentartifacts:v0.1.3' for context 'tmc2'
Deactivating plugin 'integration:v0.1.3' for context 'tmc2'
Deactivating plugin 'management-cluster:v0.1.3' for context 'tmc2'
Deactivating plugin 'provider-eks-cluster:v0.1.3' for context 'tmc2'
Deactivating plugin 'setting:v0.2.1' for context 'tmc2'
Deactivating plugin 'aks-cluster:v0.1.6' for context 'tmc2'
Deactivating plugin 'ekscluster:v0.1.3' for context 'tmc2'
❯

Release note

The UX has been updated for the `tanzu context delete` and `tanzu context unset` commands to list plugins that are being deactivated.

Additional information

Special notes for your reviewer

@chandrareddyp chandrareddyp requested a review from a team as a code owner September 15, 2023 20:00
@chandrareddyp chandrareddyp force-pushed the chandra/topic/deactivate-plugins-for-context-inactive branch 2 times, most recently from c19075a to 217064b Compare September 19, 2023 16:04
@chandrareddyp chandrareddyp changed the title Chandra/topic/deactivate plugins for context inactive List plugins deactivated for context unset and context delete Sep 19, 2023
marckhouzam
marckhouzam reviewed Sep 22, 2023
View reviewed changes
Copy link
Contributor

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good and addresses the original request of adding the list of plugins that are being deactivated when doing context unset and context delete.

However, with this improvement we probably want to be consistent and do something similar in other cases. Specifically:

  1. tanzu context use and tanzu context create do not list the plugins that become active, if they are already installed
    • so if I do tanzu context unset mycontext I now get the list of plugin that are deactivated, but if I do immediately after tanzu context use mycontext I only see [i] All required plugins are already installed and up-to-date
  2. similarly, when for those same tanzu context use and tanzu context create, if there was another active context before, we do not show the plugins that are being deactivated from the old context and replaced by the plugins from the new context.

That second point should be discussed as it may be showing too much information

pkg/command/context.go Outdated Show resolved Hide resolved
@chandrareddyp chandrareddyp force-pushed the chandra/topic/deactivate-plugins-for-context-inactive branch 4 times, most recently from de7ab19 to a723780 Compare September 28, 2023 22:44
marckhouzam
marckhouzam approved these changes Sep 29, 2023
View reviewed changes
Copy link
Contributor

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a rebase.
Also, could you add a release note in the PR description? This change is an improvment to end-users and should be mentioned.

After that, LGTM
Thanks!

@chandrareddyp chandrareddyp force-pushed the chandra/topic/deactivate-plugins-for-context-inactive branch from a723780 to 96e9acd Compare September 29, 2023 16:45
@chandrareddyp chandrareddyp mentioned this pull request Sep 29, 2023
Open
@chandrareddyp
Copy link
Contributor Author

This looks good and addresses the original request of adding the list of plugins that are being deactivated when doing context unset and context delete.

However, with this improvement we probably want to be consistent and do something similar in other cases. Specifically:

  1. tanzu context use and tanzu context create do not list the plugins that become active, if they are already installed

    • so if I do tanzu context unset mycontext I now get the list of plugin that are deactivated, but if I do immediately after tanzu context use mycontext I only see [i] All required plugins are already installed and up-to-date

  2. similarly, when for those same tanzu context use and tanzu context create, if there was another active context before, we do not show the plugins that are being deactivated from the old context and replaced by the plugins from the new context.

That second point should be discussed as it may be showing too much information

As discussed created a separate issue #509 to handle context create and use, use cases.

@chandrareddyp chandrareddyp force-pushed the chandra/topic/deactivate-plugins-for-context-inactive branch from d6bcf29 to 3c96640 Compare September 29, 2023 16:58
@chandrareddyp chandrareddyp merged commit 8b5885b into main Sep 29, 2023
@marckhouzam marckhouzam added this to the 1.1.0 milestone Oct 20, 2023
@chandrareddyp chandrareddyp deleted the chandra/topic/deactivate-plugins-for-context-inactive branch November 14, 2023 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marckhouzam marckhouzam marckhouzam approved these changes

Assignees
No one assigned
Labels
cla-not-required
Projects
None yet
Milestone
v1.1.0
Development

Successfully merging this pull request may close these issues.
3 participants
@chandrareddyp @marckhouzam @vmwclabot