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

Perform plugin sync for single target only #521

Merged
merged 1 commit into from
Oct 19, 2023
Merged

Perform plugin sync for single target only #521

merged 1 commit into from
Oct 19, 2023

Conversation

chandrareddyp
Copy link
Contributor

@chandrareddyp chandrareddyp commented Oct 4, 2023 •
edited
Loading

What this PR does / why we need it

This pull request updates existing functionality:

Before this pull request, 'tanzu context create,' and 'tanzu context use' performed sync operations for all active contexts. As part of this PR, the functionality has been updated as follows:

  • 'tanzu context create' now performs 'plugin sync' only for the newly created context, not for all active contexts.
  • 'tanzu context use' now triggers 'plugin sync' only for the context being set as active, not for all active contexts."
  • There is no change in 'tanzu plugin sync' command, it does performs the sync for all active contexts.

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR

  • 1)Use case # Context create, we can see while creating k8s context, the sync is not performed for TMC context even though its active.
❯ t context create --kubeconfig /Users/cpamuluri/.kube/config --kubecontext kind-kind11 --name k33
❯ t context list
  NAME         ISACTIVE  TYPE             ENDPOINT                               KUBECONFIGPATH                 KUBECONTEXT
  tmc1         true      mission-control  unstable.tmc-dev.cloud.vmware.com:443  n/a                            n/a
  kind-kind11  false     kubernetes                                              /Users/cpamuluri/.kube/config  kind-kind11
  tmc2         false     mission-control  unstable.tmc-dev.cloud.vmware.com:443  n/a                            n/a
  k22          false     kubernetes                                              /Users/cpamuluri/.kube/config  kind-kind11
❯ t context create --kubeconfig /Users/cpamuluri/.kube/config --kubecontext kind-kind11 --name k33
Flag --name has been deprecated, it has been replaced by using an argument to the command
[ok] successfully created a kubernetes context using the kubeconfig /Users/cpamuluri/.kube/config
[i] Checking for required plugins for context k33...
[i] All required plugins are already installed and up-to-date
❯ t plugin list
Standalone Plugins
  NAME  DESCRIPTION  TARGET  VERSION  STATUS

Plugins from Context:  tmc1
  NAME                  DESCRIPTION                                                     TARGET           VERSION  STATUS
  account               Account for tmc resources                                       mission-control  v0.1.10  not installed
  agentartifacts        helm for tmc resources                                          mission-control  v0.1.9   not installed
  aks-cluster           Manage and unmanage tmc aks clusters                            mission-control  v0.2.0   not installed
  apply                 Create/Update a resource with a resource file                   mission-control  v0.3.6   not installed
  audit                 Run an audit request on an org                                  mission-control  v0.1.9   not installed
  cluster                                                                               mission-control  v0.2.4   not installed
  clustergroup          A group of Kubernetes clusters                                  mission-control  v0.1.9   not installed
  continuousdelivery    Continuousdelivery for tmc resources                            mission-control  v0.1.9   not installed
  data-protection       Data protection for tmc resources                               mission-control  v0.1.9   not installed
  ekscluster                                                                            mission-control  v0.1.9   not installed
  events                Events for any meaningful user activity or system state change  mission-control  v0.1.9   not installed
  helm                  helm for tmc resources                                          mission-control  v0.1.9   not installed
  iam                   IAM Policies for tmc resources                                  mission-control  v0.1.9   not installed
  inspection            Inspection for tmc resources                                    mission-control  v0.1.9   not installed
  integration           Get available integrations and their information from registry  mission-control  v0.1.9   not installed
  management-cluster    A TMC registered Management cluster                             mission-control  v0.2.9   not installed
  policy                Policy for tmc resources                                        mission-control  v0.1.9   not installed
  provider-aks-cluster  Manage and unmanage tmc provider aks clusters                   mission-control  v0.1.10  not installed
  provider-eks-cluster                                                                  mission-control  v0.1.9   not installed
  secret                secret for tmc resources                                        mission-control  v0.1.9   not installed
  setting               Setting plugin for tmc resources                                mission-control  v0.2.7   not installed
  tanzupackage          Tanzupackage for tmc resources                                  mission-control  v0.2.8   not installed
  workspace             A group of Kubernetes namespaces                                mission-control  v0.1.11  not installed

[!] As shown above, some recommended plugins have not been installed or are outdated. To install them please run 'tanzu plugin sync'.
❯
❯ t context list
  NAME         ISACTIVE  TYPE             ENDPOINT                               KUBECONFIGPATH                 KUBECONTEXT
  tmc1         true      mission-control  unstable.tmc-dev.cloud.vmware.com:443  n/a                            n/a
  kind-kind11  false     kubernetes                                              /Users/cpamuluri/.kube/config  kind-kind11
  tmc2         false     mission-control  unstable.tmc-dev.cloud.vmware.com:443  n/a                            n/a
  k22          false     kubernetes                                              /Users/cpamuluri/.kube/config  kind-kind11
  k33          true      kubernetes                                              /Users/cpamuluri/.kube/config  kind-kind11
❯
  • 2)Use case # Context use; We can see that when we perform 'tanzu context use' the sync is not performed for all context's, in this case, sync is not performed for the TMC context, its performed only for the k8s context for which 'tanzu context use' is performed.
❯
❯
❯ t context unset k33
The context 'k33' of type 'kubernetes' has been set as inactive
❯
❯
❯ t plugin list
Standalone Plugins
  NAME  DESCRIPTION  TARGET  VERSION  STATUS

Plugins from Context:  tmc1
  NAME                  DESCRIPTION                                                     TARGET           VERSION  STATUS
  account               Account for tmc resources                                       mission-control  v0.1.10  not installed
  agentartifacts        helm for tmc resources                                          mission-control  v0.1.9   not installed
  aks-cluster           Manage and unmanage tmc aks clusters                            mission-control  v0.2.0   not installed
  apply                 Create/Update a resource with a resource file                   mission-control  v0.3.6   not installed
  audit                 Run an audit request on an org                                  mission-control  v0.1.9   not installed
  cluster                                                                               mission-control  v0.2.4   not installed
  clustergroup          A group of Kubernetes clusters                                  mission-control  v0.1.9   not installed
  continuousdelivery    Continuousdelivery for tmc resources                            mission-control  v0.1.9   not installed
  data-protection       Data protection for tmc resources                               mission-control  v0.1.9   not installed
  ekscluster                                                                            mission-control  v0.1.9   not installed
  events                Events for any meaningful user activity or system state change  mission-control  v0.1.9   not installed
  helm                  helm for tmc resources                                          mission-control  v0.1.9   not installed
  iam                   IAM Policies for tmc resources                                  mission-control  v0.1.9   not installed
  inspection            Inspection for tmc resources                                    mission-control  v0.1.9   not installed
  integration           Get available integrations and their information from registry  mission-control  v0.1.9   not installed
  management-cluster    A TMC registered Management cluster                             mission-control  v0.2.9   not installed
  policy                Policy for tmc resources                                        mission-control  v0.1.9   not installed
  provider-aks-cluster  Manage and unmanage tmc provider aks clusters                   mission-control  v0.1.10  not installed
  provider-eks-cluster                                                                  mission-control  v0.1.9   not installed
  secret                secret for tmc resources                                        mission-control  v0.1.9   not installed
  setting               Setting plugin for tmc resources                                mission-control  v0.2.7   not installed
  tanzupackage          Tanzupackage for tmc resources                                  mission-control  v0.2.8   not installed
  workspace             A group of Kubernetes namespaces                                mission-control  v0.1.11  not installed

[!] As shown above, some recommended plugins have not been installed or are outdated. To install them please run 'tanzu plugin sync'.
❯ t context use k33
[i] Checking for required plugins for context k33...
[i] All required plugins are already installed and up-to-date
❯
❯
❯ t plugin list
Standalone Plugins
  NAME  DESCRIPTION  TARGET  VERSION  STATUS

Plugins from Context:  tmc1
  NAME                  DESCRIPTION                                                     TARGET           VERSION  STATUS
  account               Account for tmc resources                                       mission-control  v0.1.10  not installed
  agentartifacts        helm for tmc resources                                          mission-control  v0.1.9   not installed
  aks-cluster           Manage and unmanage tmc aks clusters                            mission-control  v0.2.0   not installed
  apply                 Create/Update a resource with a resource file                   mission-control  v0.3.6   not installed
  audit                 Run an audit request on an org                                  mission-control  v0.1.9   not installed
  cluster                                                                               mission-control  v0.2.4   not installed
  clustergroup          A group of Kubernetes clusters                                  mission-control  v0.1.9   not installed
  continuousdelivery    Continuousdelivery for tmc resources                            mission-control  v0.1.9   not installed
  data-protection       Data protection for tmc resources                               mission-control  v0.1.9   not installed
  ekscluster                                                                            mission-control  v0.1.9   not installed
  events                Events for any meaningful user activity or system state change  mission-control  v0.1.9   not installed
  helm                  helm for tmc resources                                          mission-control  v0.1.9   not installed
  iam                   IAM Policies for tmc resources                                  mission-control  v0.1.9   not installed
  inspection            Inspection for tmc resources                                    mission-control  v0.1.9   not installed
  integration           Get available integrations and their information from registry  mission-control  v0.1.9   not installed
  management-cluster    A TMC registered Management cluster                             mission-control  v0.2.9   not installed
  policy                Policy for tmc resources                                        mission-control  v0.1.9   not installed
  provider-aks-cluster  Manage and unmanage tmc provider aks clusters                   mission-control  v0.1.10  not installed
  provider-eks-cluster                                                                  mission-control  v0.1.9   not installed
  secret                secret for tmc resources                                        mission-control  v0.1.9   not installed
  setting               Setting plugin for tmc resources                                mission-control  v0.2.7   not installed
  tanzupackage          Tanzupackage for tmc resources                                  mission-control  v0.2.8   not installed
  workspace             A group of Kubernetes namespaces                                mission-control  v0.1.11  not installed

[!] As shown above, some recommended plugins have not been installed or are outdated. To install them please run 'tanzu plugin sync'.
❯

Release note

- The `tanzu contex create` functionality updated, after context create, it performs the sync operation only for the newly created context, not for all active contexts.
- The `tanzu context use` functionality updated, after activating the given context, it performs the sync operation only for the newly activated context, do not perform sync for all active contexts.

Additional information

Special notes for your reviewer

@chandrareddyp chandrareddyp requested a review from a team as a code owner October 4, 2023 17:55
@chandrareddyp chandrareddyp force-pushed the topic/chandra/sync-single-context branch 2 times, most recently from e234f33 to 1c30aa8 Compare October 5, 2023 02:36
@chandrareddyp chandrareddyp changed the title Perform plugin sync only for one context Perform plugin sync for single target only Oct 5, 2023
@chandrareddyp chandrareddyp force-pushed the topic/chandra/sync-single-context branch 2 times, most recently from 2634365 to f1e1f3c Compare October 5, 2023 12:27
marckhouzam
marckhouzam reviewed Oct 5, 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.

I like this! It will be much nicer for users.

Some comments in-line.

pkg/pluginmanager/manager.go Outdated Show resolved Hide resolved
pkg/command/plugin.go Outdated Show resolved Hide resolved
pkg/command/plugin.go Outdated Show resolved Hide resolved
pkg/command/plugin.go Outdated Show resolved Hide resolved
pkg/pluginmanager/manager_test.go Show resolved Hide resolved
test/e2e/plugin_sync/tmc/plugin_sync_tmc_lifecycle_test.go Outdated Show resolved Hide resolved
test/e2e/plugin_sync/tmc/plugin_sync_tmc_lifecycle_test.go Outdated Show resolved Hide resolved
test/e2e/plugin_sync/tmc/plugin_sync_tmc_lifecycle_test.go Show resolved Hide resolved
pkg/pluginmanager/manager.go Outdated Show resolved Hide resolved
pkg/command/context.go Outdated Show resolved Hide resolved
@anujc25
Copy link
Contributor

anujc25 commented Oct 5, 2023

After the change vmware-tanzu/tanzu-plugin-runtime#112 is merged, we should be using ContextType instead of Target here. Also, we should use a flag called type or context-type instead of target.
E.g. tanzu plugin sync --type <kubernetes/mission-control/tae>

@marckhouzam
Copy link
Contributor

As a follow-up to this PR we should discuss improving the SyncPluginsForTarget() API from tanzu-plugin-runtime

marckhouzam
marckhouzam reviewed Oct 11, 2023
View reviewed changes
test/e2e/plugin_sync/tmc/plugin_sync_tmc_lifecycle_test.go Outdated Show resolved Hide resolved
pkg/command/plugin.go Outdated Show resolved Hide resolved
@chandrareddyp chandrareddyp force-pushed the topic/chandra/sync-single-context branch 3 times, most recently from 5c17e63 to 804e3f1 Compare October 13, 2023 21:04
marckhouzam
marckhouzam reviewed Oct 15, 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.

Thanks @chandrareddyp.

Synching a single context may require a little more discussion now that we have decided that all context are mutually-exclusive when active, except for TMC. This means that there can be at most two active contexts at the same time:

  1. a TMC context
  2. a kubernetes OR tae context

For the non-TMC context, do we want to require the user to specify the correct type (k8s or tae) or do we want to just allow to say "sync the other context than the TMC one"? This makes me wonder if we don't actually want to use --target for synching.

Once we agree on the approach, the commit message may need to be updated.
Also the PR description may need to be updated.

And finally, we must discuss #521 (comment)

test/e2e/plugin_sync/tmc/plugin_sync_tmc_lifecycle_test.go Show resolved Hide resolved
test/e2e/plugin_sync/tmc/plugin_sync_tmc_lifecycle_test.go Outdated Show resolved Hide resolved
pkg/command/plugin.go Outdated Show resolved Hide resolved
pkg/command/plugin.go Outdated Show resolved Hide resolved
pkg/command/plugin.go Outdated Show resolved Hide resolved
pkg/pluginmanager/manager_test.go Show resolved Hide resolved
test/e2e/plugin_sync/tmc/plugin_sync_tmc_lifecycle_test.go Show resolved Hide resolved
pkg/pluginmanager/manager_test.go Outdated Show resolved Hide resolved
test/e2e/plugin_sync/tmc/plugin_sync_tmc_lifecycle_test.go Outdated Show resolved Hide resolved
test/e2e/plugin_sync/tmc/plugin_sync_tmc_lifecycle_test.go Outdated Show resolved Hide resolved
@chandrareddyp chandrareddyp force-pushed the topic/chandra/sync-single-context branch 2 times, most recently from 3d65cf1 to 4888c55 Compare October 17, 2023 15:37
@chandrareddyp
Copy link
Contributor Author

Thanks @chandrareddyp.

Synching a single context may require a little more discussion now that we have decided that all context are mutually-exclusive when active, except for TMC. This means that there can be at most two active contexts at the same time:

  1. a TMC context
  2. a kubernetes OR tae context

For the non-TMC context, do we want to require the user to specify the correct type (k8s or tae) or do we want to just allow to say "sync the other context than the TMC one"? This makes me wonder if we don't actually want to use --target for synching.

Once we agree on the approach, the commit message may need to be updated. Also the PR description may need to be updated.

And finally, we must discuss #521 (comment)

After a discussion with the team, we have decided to remove the -t or --type flag from the 'tanzu plugin sync' command. If an end-user wants to perform the sync for a specific context type, they can use the tanzu context use CONTEXT-NAME command.

@chandrareddyp chandrareddyp force-pushed the topic/chandra/sync-single-context branch from c796c65 to e1b0070 Compare October 19, 2023 15:53
Sync Single Target Plugins Only
aeaa4ae 
This pull request updates existing functionality of context create/use as follows:

1. 'tanzu context create' now performs 'plugin sync' only for the newly created context, not for all active contexts.

2. 'tanzu context use' now triggers 'plugin sync' only for the context being set as active, not for all currently active contexts.
@chandrareddyp chandrareddyp force-pushed the topic/chandra/sync-single-context branch from e1b0070 to aeaa4ae Compare October 19, 2023 16:03
marckhouzam
marckhouzam approved these changes Oct 19, 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.

LGTM
Thanks for this very nice change!

@chandrareddyp chandrareddyp merged commit b8c291b into main Oct 19, 2023
8 checks passed
@marckhouzam marckhouzam added this to the 1.1.0 milestone Oct 20, 2023
mpanchajanya pushed a commit that referenced this pull request Oct 24, 2023
@chandrareddyp @mpanchajanya
Sync Single Context Plugins Only (#521)
5531eed 
(cherry picked from commit b8c291b)
mpanchajanya pushed a commit that referenced this pull request Oct 24, 2023
@chandrareddyp @mpanchajanya
Sync Single Context Plugins Only (#521)
628cc2f 
(cherry picked from commit b8c291b)
mpanchajanya pushed a commit that referenced this pull request Oct 24, 2023
@chandrareddyp @mpanchajanya
Sync Single Context Plugins Only (#521)
a3572bd 
(cherry picked from commit b8c291b)
@chandrareddyp chandrareddyp deleted the topic/chandra/sync-single-context 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.
4 participants
@chandrareddyp @anujc25 @marckhouzam @vmwclabot