Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
- Update error and warning messages
- Add contextName as part of the `tanzu plugin list` for json/yaml output when
  plugin is recommended
- Add comment to the IsPluginActive function
- Hide `--wide` flag for `tanzu plugin list`
- Update status to `update needed` and `not installed` instead of
  `update recommended` and `install recommeneded` respectively
- Update column names for the to be installed plugins table when running
  `tanzu plugin sync` and `tanzu context use/create` to show `CURRENT`
  and `INSTALLING`.
  • Loading branch information
anujc25 committed Apr 18, 2024
1 parent f34fbc6 commit fb57491
Show file tree
Hide file tree
Showing 10 changed files with 48 additions and 67 deletions.
12 changes: 6 additions & 6 deletions pkg/command/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ func createCtx(cmd *cobra.Command, args []string) (err error) {

// Sync all required plugins
if err := syncContextPlugins(cmd, ctx.ContextType, ctxName); err != nil {
log.Warningf("unable to automatically sync the plugins recommended by the active context. Please run 'tanzu plugin sync' command to sync plugins manually, error: '%v'", err.Error())
log.Warningf("unable to automatically sync the plugins recommended by the new context. Please run 'tanzu plugin sync' to sync plugins manually, error: '%v'", err.Error())
}
return nil
}
Expand Down Expand Up @@ -306,12 +306,12 @@ func syncContextPlugins(cmd *cobra.Command, contextType configtypes.ContextType,
}

if len(pluginsNeedToBeInstalled) == 0 {
log.Success("All recommended plugins are already installed.")
log.Success("All recommended plugins are already installed and up-to-date.")
return nil
}

errList := make([]error, 0)
log.Infof("Installing following plugins recommended by the context '%s':", ctxName)
log.Infof("Installing the following plugins recommended by context '%s':", ctxName)
displayToBeInstalledPluginsAsTable(plugins, cmd.ErrOrStderr())
for i := range pluginsNeedToBeInstalled {
err = pluginmanager.InstallStandalonePlugin(pluginsNeedToBeInstalled[i].Name, pluginsNeedToBeInstalled[i].RecommendedVersion, pluginsNeedToBeInstalled[i].Target)
Expand All @@ -329,8 +329,8 @@ func syncContextPlugins(cmd *cobra.Command, contextType configtypes.ContextType,

// displayToBeInstalledPluginsAsTable takes a list of plugins and displays the plugin info as a table
func displayToBeInstalledPluginsAsTable(plugins []discovery.Discovered, writer io.Writer) {
outputPlugins := component.NewOutputWriterWithOptions(writer, outputFormat, []component.OutputWriterOption{}, "Name", "Target", "Installed", "Recommended")
outputPlugins.MarkDynamicKeys("Installed")
outputPlugins := component.NewOutputWriterWithOptions(writer, outputFormat, []component.OutputWriterOption{}, "Name", "Target", "Current", "Installing")
outputPlugins.MarkDynamicKeys("Current")
for i := range plugins {
if plugins[i].Status == common.PluginStatusNotInstalled || plugins[i].Status == common.PluginStatusUpdateAvailable {
outputPlugins.AddRow(plugins[i].Name, plugins[i].Target, plugins[i].InstalledVersion, plugins[i].RecommendedVersion)
Expand Down Expand Up @@ -1250,7 +1250,7 @@ func useCtx(cmd *cobra.Command, args []string) error { //nolint:gocyclo

// Sync all required plugins
if err := syncContextPlugins(cmd, ctx.ContextType, ctxName); err != nil {
log.Warningf("unable to automatically sync the plugins recommended by the active context. Please run 'tanzu plugin sync' command to sync plugins manually, error: '%v'", err.Error())
log.Warningf("unable to automatically sync the plugins recommended by the active context. Please run 'tanzu plugin sync' to sync plugins manually, error: '%v'", err.Error())
}
return nil
}
Expand Down
21 changes: 15 additions & 6 deletions pkg/command/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func newPluginCmd() *cobra.Command {
listPluginCmd.Flags().StringVarP(&outputFormat, "output", "o", "", "Output format (yaml|json|table)")
utils.PanicOnErr(listPluginCmd.RegisterFlagCompletionFunc("output", completionGetOutputFormats))
listPluginCmd.Flags().BoolVar(&showAllColumns, "wide", false, "display additional columns for plugins")
utils.PanicOnErr(listPluginCmd.Flags().MarkHidden("wide"))

describePluginCmd.Flags().StringVarP(&outputFormat, "output", "o", "", "Output format (yaml|json|table)")
utils.PanicOnErr(describePluginCmd.RegisterFlagCompletionFunc("output", completionGetOutputFormats))
Expand Down Expand Up @@ -172,6 +173,7 @@ func newDescribePluginCmd() *cobra.Command {
defer fmt.Fprintln(cmd.OutOrStdout())
}
output := component.NewOutputWriterWithOptions(cmd.OutOrStdout(), outputFormat, []component.OutputWriterOption{}, "name", "version", "status", "target", "description", "installationPath")

if len(args) != 1 {
return fmt.Errorf("must provide one plugin name as a positional argument")
}
Expand Down Expand Up @@ -451,9 +453,11 @@ func syncPlugins(cmd *cobra.Command) error {
}

for contextType, context := range contextMap {
err = syncContextPlugins(cmd, contextType, context.Name)
if err != nil {
errList = append(errList, err)
if strings.TrimSpace(context.Name) != "" {
err = syncContextPlugins(cmd, contextType, context.Name)
if err != nil {
errList = append(errList, err)
}
}
}
return kerrors.NewAggregate(errList)
Expand All @@ -466,6 +470,7 @@ type pluginListInfo struct {
installed string
recommended string
status string
contextName string // used only to specify which context recommends the plugin
active bool
}

Expand Down Expand Up @@ -507,7 +512,7 @@ func displayInstalledPlugins(installedPlugins []cli.PluginInfo, recommendedConte
active: pluginsupplier.IsPluginActive(&installedPlugins[index]),
}
if p.recommended != "" && p.installed != p.recommended {
p.status = common.PluginStatusRecommendUpdate
p.status = "update needed"
pluginSyncRequired = true
}
plugins = append(plugins, p)
Expand All @@ -520,7 +525,8 @@ func displayInstalledPlugins(installedPlugins []cli.PluginInfo, recommendedConte
target: string(recommendedContextPlugins[index].Target),
installed: "",
recommended: recommendedContextPlugins[index].RecommendedVersion,
status: common.PluginStatusRecommendInstall,
status: common.PluginStatusNotInstalled,
contextName: recommendedContextPlugins[index].ContextName,
active: false,
}
plugins = append(plugins, p)
Expand All @@ -539,12 +545,15 @@ func displayInstalledPlugins(installedPlugins []cli.PluginInfo, recommendedConte
outputPluginWriter.SetKeys(columnsNames...)
outputPluginWriter.MarkDynamicKeys("Recommended") // Marking this column as dynamic so that it will only be shown if at least one row is non-empty
for index := range plugins {
// Output writer will ignore and not show additional row data if the row has more data compared to defined keys(column headers).
// So in this case if showAllColumns=false than last value for row (plugins[index].active) will not be shown. So it is safe to provide
// all values to the Row.
outputPluginWriter.AddRow(plugins[index].name, plugins[index].description, plugins[index].target, plugins[index].installed, plugins[index].recommended, plugins[index].status, plugins[index].active)
}
} else {
outputPluginWriter.SetKeys("Name", "Description", "Target", "Installed", "Recommended", "Status", "Active", "Context", "Version") // Add 'Context' and 'Version' fields for backwards compatibility
for index := range plugins {
outputPluginWriter.AddRow(plugins[index].name, plugins[index].description, plugins[index].target, plugins[index].installed, plugins[index].recommended, plugins[index].status, plugins[index].active, "", plugins[index].installed)
outputPluginWriter.AddRow(plugins[index].name, plugins[index].description, plugins[index].target, plugins[index].installed, plugins[index].recommended, plugins[index].status, plugins[index].active, plugins[index].contextName, plugins[index].installed)
}
}

Expand Down
12 changes: 5 additions & 7 deletions pkg/common/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,11 @@ package common

// Plugin status and scope constants
const (
PluginStatusInstalled = "installed"
PluginStatusNotInstalled = "not installed"
PluginStatusRecommendInstall = "install recommended"
PluginStatusRecommendUpdate = "update recommended"
PluginStatusUpdateAvailable = "update available"
PluginScopeStandalone = "Standalone"
PluginScopeContext = "Context"
PluginStatusInstalled = "installed"
PluginStatusNotInstalled = "not installed"
PluginStatusUpdateAvailable = "update available"
PluginScopeStandalone = "Standalone"
PluginScopeContext = "Context"
)

// DiscoveryType constants
Expand Down
29 changes: 0 additions & 29 deletions pkg/pluginmanager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -1099,35 +1099,6 @@ func doDeletePluginsFromCatalog(plugins []cli.PluginInfo) error {
return kerrors.NewAggregate(errList)
}

// SyncPlugins will install the plugins required by the current contexts.
// If the central-repo is disabled, all discovered plugins will be installed.
func SyncPlugins() error {
log.Info("Checking for required plugins...")
errList := make([]error, 0)
// We no longer sync standalone plugins.
// With a centralized approach to discovering plugins, synchronizing
// standalone plugins would install ALL plugins available for ALL
// products.
// Instead, we only synchronize any plugins that are specifically specified
// by the contexts.
//
// Note: to install all plugins for a specific product, plugin groups will
// need to be used.
plugins, err := DiscoverServerPlugins()
if err != nil {
errList = append(errList, err)
}

for i := range plugins {
err = InstallStandalonePlugin(plugins[i].Name, plugins[i].RecommendedVersion, plugins[i].Target)
if err != nil {
errList = append(errList, err)
}
}

return kerrors.NewAggregate(errList)
}

func DiscoverPluginsForContextType(contextType configtypes.ContextType) ([]discovery.Discovered, error) {
ctx, err := configlib.GetActiveContext(contextType)
if err != nil {
Expand Down
7 changes: 0 additions & 7 deletions pkg/pluginmanager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -756,13 +756,6 @@ func Test_SyncPlugins(t *testing.T) {
assertions.NotNil(p)
assertions.Equal(common.PluginStatusNotInstalled, p.Status)
}

// Sync all available plugins
err = SyncPlugins()
assertions.NotNil(err)
// There is an error for the kubernetes discovery since we don't have a cluster
// but other server plugins will be found, so we use those
assertions.Contains(err.Error(), `Failed to load Kubeconfig file from "config"`)
}

func Test_setAvailablePluginsStatus(t *testing.T) {
Expand Down
11 changes: 11 additions & 0 deletions pkg/pluginsupplier/plugin_supplier.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,17 @@ func FilterPluginsByActiveContextType(plugins []cli.PluginInfo) (result []cli.Pl
}

// IsPluginActive returns true if specified plugin is active
//
// Plugin being activated means it is available as Tanzu CLI commands.
// Some plugins are activated conditionally based on a the plugin descriptor.
// If plugin is not active, even if the plugin is installed, it is not
// available as the Tanzu CLI commands for users to use.
//
// For example: If PluginDescription.SupportedContextType is
// specified for the plugin, the plugin is only active
// if the SupportedContextType mentioned with PluginDescription is
// active in Tanzu CLI contexts.
// Reference: https://github.com/vmware-tanzu/tanzu-plugin-runtime/blob/main/plugin/types.go#L117
func IsPluginActive(pi *cli.PluginInfo) bool {
if len(pi.SupportedContextType) == 0 {
return true
Expand Down
13 changes: 6 additions & 7 deletions test/e2e/framework/framework_constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,12 @@ const (
CLIE2ETestEnvironment = "TANZU_CLI_E2E_TEST_ENVIRONMENT"

// General constants
True = "true"
Installed = "installed"
UpdateAvailable = "update available"
NotInstalled = "not installed"
RecommendInstall = "install recommended"
RecommendUpdate = "update recommended"
JSONOtuput = "-o json"
True = "true"
Installed = "installed"
UpdateAvailable = "update available"
NotInstalled = "not installed"
UpdateNeeded = "update needed"
JSONOtuput = "-o json"

// Context commands
CreateContextWithEndPoint = "%s context create --endpoint %s %s"
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/framework/plugin_lifecycle_operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func (po *pluginCmdOps) ListRecommendedPluginsFromActiveContext(installedOnly bo
for i := range plugins {
if plugins[i].Recommended != "" {
if installedOnly {
if plugins[i].Status == Installed || plugins[i].Status == UpdateAvailable || plugins[i].Status == RecommendUpdate {
if plugins[i].Status == Installed || plugins[i].Status == UpdateAvailable || plugins[i].Status == UpdateNeeded {
recommendedPlugins = append(recommendedPlugins, plugins[i])
}
} else {
Expand Down
6 changes: 3 additions & 3 deletions test/e2e/plugin_sync/k8s/plugin_sync_k8s_lifecycle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ var _ = f.CLICoreDescribe("[Tests:E2E][Feature:Plugin-sync-lifecycle]", func() {
recommendedInstalledPlugins, err = tf.PluginCmd.ListRecommendedPluginsFromActiveContext(true)
Expect(err).To(BeNil(), "should not get any error for plugin list")
for i := range recommendedInstalledPlugins {
Expect(recommendedInstalledPlugins[i].Status).To(Equal(f.RecommendUpdate), "all installed context-scoped plugin status should show 'update recommended'")
Expect(recommendedInstalledPlugins[i].Status).To(Equal(f.UpdateNeeded), "all installed context-scoped plugin status should show 'update needed'")
}

_, _, err = tf.PluginCmd.Sync()
Expand All @@ -207,7 +207,7 @@ var _ = f.CLICoreDescribe("[Tests:E2E][Feature:Plugin-sync-lifecycle]", func() {
Expect(err).To(BeNil(), "should not get any error for plugin list")

uninstalledPluginInfo := f.GetGivenPluginFromTheGivenPluginList(recommendedPluginsList, pluginToUninstall)
Expect(uninstalledPluginInfo.Status).To(Equal(f.RecommendInstall), "uninstalled plugin should be listed as 'install recommended'")
Expect(uninstalledPluginInfo.Status).To(Equal(f.NotInstalled), "uninstalled plugin should be listed as 'not installed'")
Expect(uninstalledPluginInfo.Recommended).To(Equal(pluginToUninstall.Version), "uninstalled plugin should also specify correct recommended column")

_, _, err = tf.PluginCmd.Sync()
Expand Down Expand Up @@ -317,7 +317,7 @@ var _ = f.CLICoreDescribe("[Tests:E2E][Feature:Plugin-sync-lifecycle]", func() {
const (
ContextActivated = "Successfully activated context '%s'"
PluginWillBeInstalled = "Installing following plugins recommended by the context '%s':"
PluginsTableHeaderRegExp = "NAME\\s+TARGET\\s+RECOMMENDED"
PluginsTableHeaderRegExp = "NAME\\s+TARGET\\s+INSTALLING"
PluginsRow = "%s\\s+%s\\s+%s"
PluginInstalledRegExp = "Installed plugin '%s:.+' with target '%s'|Reinitialized plugin '%s:.+' with target '%s'"
)
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/plugin_sync/tmc/plugin_sync_tmc_lifecycle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ var _ = f.CLICoreDescribe("[Tests:E2E][Feature:Plugin-Sync-TMC-lifecycle]", func
Expect(err).To(BeNil(), "should not get any error for plugin list")

uninstalledPluginInfo := f.GetGivenPluginFromTheGivenPluginList(recommendedPluginsList, pluginToUninstall)
Expect(uninstalledPluginInfo.Status).To(Equal(f.RecommendInstall), "uninstalled plugin should be listed as 'install recommended'")
Expect(uninstalledPluginInfo.Status).To(Equal(f.NotInstalled), "uninstalled plugin should be listed as 'not installed'")
Expect(uninstalledPluginInfo.Recommended).To(Equal(pluginToUninstall.Version), "uninstalled plugin should also specify correct recommended column")
} else {
// fail the test case if at least two plugins are not available in the mock response
Expand Down

0 comments on commit fb57491

Please sign in to comment.