From 748d359494d426569f7ea4eefbefe362a2231d56 Mon Sep 17 00:00:00 2001 From: Chandra Pamuluri Date: Thu, 14 Mar 2024 12:21:53 -0500 Subject: [PATCH] Enhance plugin installation UX PR updates the plugin installation UX, - As part of plugin installation in-progress message, it adds total plugins needs to installed and the number of plugins being installed The in-progress message will disappear once the installation completes. Next plugin in-progress will be shown. - No log messages will be showed once the plugin installation completes. - This UX applicable to below use case: plugin upgrade plugin install (target specific and local source) plugin sync context use --- pkg/command/context.go | 5 +- pkg/command/plugin.go | 2 + pkg/pluginmanager/essentials.go | 1 + pkg/pluginmanager/manager.go | 115 ++++++++++++++++++++++++-------- 4 files changed, 95 insertions(+), 28 deletions(-) diff --git a/pkg/command/context.go b/pkg/command/context.go index ff9a8953c..52d0acbc0 100644 --- a/pkg/command/context.go +++ b/pkg/command/context.go @@ -323,18 +323,21 @@ func syncContextPlugins(cmd *cobra.Command, contextType configtypes.ContextType, errList := make([]error, 0) log.Infof("Installing the following plugins recommended by context '%s':", ctxName) + pluginmanager.SetTotalPluginsToInstall(len(pluginsNeedToBeInstalled)) displayToBeInstalledPluginsAsTable(plugins, cmd.ErrOrStderr()) for i := range pluginsNeedToBeInstalled { err = pluginmanager.InstallStandalonePlugin(pluginsNeedToBeInstalled[i].Name, pluginsNeedToBeInstalled[i].RecommendedVersion, pluginsNeedToBeInstalled[i].Target) if err != nil { errList = append(errList, err) + } else { + pluginmanager.IncrementPluginsInstalledCount(1) } } err = kerrors.NewAggregate(errList) if err == nil { log.Success("Successfully installed all recommended plugins.") } - + pluginmanager.ResetPluginInstallationCounts() return err } diff --git a/pkg/command/plugin.go b/pkg/command/plugin.go index 8b4305b79..983ff6887 100644 --- a/pkg/command/plugin.go +++ b/pkg/command/plugin.go @@ -243,6 +243,7 @@ func newInstallPluginCmd() *cobra.Command { return installPluginsForPluginGroup(cmd, args) } + pluginmanager.SetTotalPluginsToInstall(1) // Invoke install plugin from local source if local files are provided if local != "" { if len(args) == 0 { @@ -340,6 +341,7 @@ func newUpgradePluginCmd() *cobra.Command { // With the Central Repository feature we can simply request to install // the recommendedVersion. + pluginmanager.SetTotalPluginsToInstall(1) err = pluginmanager.UpgradePlugin(pluginName, cli.VersionLatest, getTarget()) if err != nil { return err diff --git a/pkg/pluginmanager/essentials.go b/pkg/pluginmanager/essentials.go index 9a009cef0..b959e49a9 100644 --- a/pkg/pluginmanager/essentials.go +++ b/pkg/pluginmanager/essentials.go @@ -69,6 +69,7 @@ func installPluginsFromEssentialPluginGroup(name, version string) (string, error if err != nil { return "", fmt.Errorf("failed to install plugins from group: %w", err) } + log.Successf("successfully installed all plugins from group '%s'", groupWithVersion) // If the installation is successful, return the group with version. return groupWithVersion, nil diff --git a/pkg/pluginmanager/manager.go b/pkg/pluginmanager/manager.go index 8bbf18744..e8e4fc998 100644 --- a/pkg/pluginmanager/manager.go +++ b/pkg/pluginmanager/manager.go @@ -11,6 +11,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strings" "github.com/Masterminds/semver" @@ -56,7 +57,32 @@ const ( errorNoActiveContexForGivenContextType = "there is no active context for the given context type `%v`" ) +var totalPluginsToInstall = 0 +var pluginsInstalledCount = 0 var execCommand = exec.Command +var spinner component.OutputWriterSpinner + +func init() { + // Initialize global spinner + spinner = component.NewOutputWriterSpinner(component.WithOutputStream(os.Stderr)) + runtime.SetFinalizer(spinner, func(s component.OutputWriterSpinner) { + if s != nil { + s.StopSpinner() + } + }) +} + +func StopSpinner() { + if spinner != nil { + spinner.StopSpinner() + } +} + +// ResetPluginInstallationCounts resets the total number of plugins to install and the number of plugins installed +func ResetPluginInstallationCounts() { + SetTotalPluginsToInstall(0) + SetPluginsInstalledCount(0) +} type DeletePluginOptions struct { Target configtypes.Target @@ -472,9 +498,30 @@ func InitializePlugin(plugin *cli.PluginInfo) error { // InstallStandalonePlugin installs a plugin by name, version and target as a standalone plugin. func InstallStandalonePlugin(pluginName, version string, target configtypes.Target) error { + defer StopSpinner() return installPlugin(pluginName, version, target, "") } +// SetTotalPluginsToInstall sets the total number of plugins to install +func SetTotalPluginsToInstall(total int) { + totalPluginsToInstall = total +} + +// SetPluginsInstalledCount sets the number of plugins installed already +func SetPluginsInstalledCount(count int) { + pluginsInstalledCount = count +} + +// GetPluginsInstalledCount returns the number of plugins installed +func GetPluginsInstalledCount() int { + return pluginsInstalledCount +} + +// IncrementPluginsInstalledCount increments the number of plugins installed +func IncrementPluginsInstalledCount(count int) { + pluginsInstalledCount += count +} + // installs a plugin by name, version and target. // If the contextName is not empty, it implies the plugin is a context-scope plugin, otherwise // we are installing a standalone plugin. @@ -560,7 +607,7 @@ func installPlugin(pluginName, version string, target configtypes.Target, contex if len(matchedPlugins) == 1 { return installOrUpgradePlugin(&matchedPlugins[0], matchedPlugins[0].RecommendedVersion, false) } - + // there can be only one plugin with the same name and target, so we can safely return the first one for i := range matchedPlugins { if matchedPlugins[i].Target == target { return installOrUpgradePlugin(&matchedPlugins[i], matchedPlugins[i].RecommendedVersion, false) @@ -592,31 +639,36 @@ func InstallPluginsFromGroup(pluginName, groupIDAndVersion string, options ...Pl // from the database groupIDAndVersion = fmt.Sprintf("%s-%s/%s:%s", pg.Vendor, pg.Publisher, pg.Name, pg.RecommendedVersion) log.Infof("Installing plugins from plugin group '%s'", groupIDAndVersion) - return InstallPluginsFromGivenPluginGroup(pluginName, groupIDAndVersion, pg) } // InstallPluginsFromGivenPluginGroup installs either the specified plugin or all plugins from given plugin group plugins. func InstallPluginsFromGivenPluginGroup(pluginName, groupIDAndVersion string, pg *plugininventory.PluginGroup) (string, error) { numErrors := 0 - numInstalled := 0 mandatoryPluginsExist := false pluginExist := false + + pluginsToInstall := make([]*plugininventory.PluginGroupPluginEntry, 0) for _, plugin := range pg.Versions[pg.RecommendedVersion] { if pluginName == cli.AllPlugins || pluginName == plugin.Name { pluginExist = true if plugin.Mandatory { mandatoryPluginsExist = true - err := InstallStandalonePlugin(plugin.Name, plugin.Version, plugin.Target) - if err != nil { - numErrors++ - log.Warningf("unable to install plugin '%s': %v", plugin.Name, err.Error()) - } else { - numInstalled++ - } + pluginsToInstall = append(pluginsToInstall, plugin) // Add mandatory plugin to the slice } } } + SetTotalPluginsToInstall(len(pluginsToInstall)) + SetPluginsInstalledCount(0) + defer StopSpinner() + for _, plugin := range pluginsToInstall { + err := InstallStandalonePlugin(plugin.Name, plugin.Version, plugin.Target) + if err != nil { + numErrors++ + } else { + IncrementPluginsInstalledCount(1) + } + } if !pluginExist { return groupIDAndVersion, fmt.Errorf("plugin '%s' is not part of the group '%s'", pluginName, groupIDAndVersion) @@ -633,9 +685,10 @@ func InstallPluginsFromGivenPluginGroup(pluginName, groupIDAndVersion string, pg return groupIDAndVersion, fmt.Errorf("could not install %d plugin(s) from group '%s'", numErrors, groupIDAndVersion) } - if numInstalled == 0 { + if GetPluginsInstalledCount() == 0 { return groupIDAndVersion, fmt.Errorf("plugin '%s' is not part of the group '%s'", pluginName, groupIDAndVersion) } + ResetPluginInstallationCounts() return groupIDAndVersion, nil } @@ -704,7 +757,7 @@ func getPluginInstallationMessage(p *discovery.Discovered, version string, isPlu installingMsg = fmt.Sprintf("Installing plugin '%v:%v' %v(from cache)", p.Name, version, withTarget) installedMsg = fmt.Sprintf("Installed plugin '%v:%v' %v(from cache)", p.Name, version, withTarget) } else { - installingMsg = fmt.Sprintf("Plugin '%v:%v' %vis already installed. Reinitializing...", p.Name, version, withTarget) + installingMsg = fmt.Sprintf("Already installed : Plugin '%v:%v' %v", p.Name, version, withTarget) installedMsg = fmt.Sprintf("Reinitialized plugin '%v:%v' %v", p.Name, version, withTarget) } } else { @@ -734,25 +787,27 @@ func installOrUpgradePlugin(p *discovery.Discovered, version string, installTest } // Log message based on different installation conditions - installingMsg, installedMsg, errMsg := getPluginInstallationMessage(p, version, plugin != nil, isPluginAlreadyInstalled) + installingMsg, _, errMsg := getPluginInstallationMessage(p, version, plugin != nil, isPluginAlreadyInstalled) - var spinner component.OutputWriterSpinner + installingMsg = fmt.Sprintf("[%v/%v] %v", pluginsInstalledCount, totalPluginsToInstall, installingMsg) + errMsg = fmt.Sprintf("[%v/%v] %v", pluginsInstalledCount, totalPluginsToInstall, errMsg) + numPluginsInstalled := fmt.Sprintf("%d plugins installed out of %d", pluginsInstalledCount, totalPluginsToInstall) + numPluginsInstalledWithLog := fmt.Sprintf("%s%s", log.GetLogTypeIndicator(log.LogTypeERROR), numPluginsInstalled) + errMsg = fmt.Sprintf("%s\n%s", errMsg, numPluginsInstalledWithLog) // Initialize the spinner if the spinner is allowed - if component.IsTTYEnabled() { - // Initialize the spinner - spinner = component.NewOutputWriterSpinner(component.WithOutputStream(os.Stderr), - component.WithSpinnerText(installingMsg), - component.WithSpinnerStarted()) + if component.IsTTYEnabled() && spinner != nil { + spinner.SetText(installingMsg) spinner.SetFinalText(errMsg, log.LogTypeERROR) - defer spinner.StopSpinner() + spinner.StartSpinner() } else { log.Info(installingMsg) } pluginErr := verifyInstallAndInitializePlugin(plugin, p, version, installTestPlugin) if pluginErr == nil && spinner != nil { - spinner.SetFinalText(installedMsg, log.LogTypeINFO) + // unset the final text to avoid the spinner from showing the final text + spinner.SetFinalText("", "") } return pluginErr } @@ -1104,7 +1159,6 @@ func InstallPluginsFromLocalSource(pluginName, version string, target configtype if err != nil { return errors.Wrap(err, "unable to discover plugins") } - var errList []error var matchedPlugins []discovery.Discovered @@ -1128,20 +1182,27 @@ func InstallPluginsFromLocalSource(pluginName, version string, target configtype return errors.Errorf("unable to find plugin '%v' matching version '%v'", pluginName, version) } + defer StopSpinner() if len(matchedPlugins) == 1 { return installOrUpgradePlugin(&matchedPlugins[0], version, installTestPlugin) } + var pluginsToInstall []*discovery.Discovered for i := range matchedPlugins { // Install all plugins otherwise include all matching plugins if pluginName == cli.AllPlugins || matchedPlugins[i].Target == target { - err = installOrUpgradePlugin(&matchedPlugins[i], version, installTestPlugin) - if err != nil { - errList = append(errList, err) - } + plugin := matchedPlugins[i] + pluginsToInstall = append(pluginsToInstall, &plugin) + } + } + SetTotalPluginsToInstall(len(pluginsToInstall)) + SetPluginsInstalledCount(0) + for _, plugin := range pluginsToInstall { + err = installOrUpgradePlugin(plugin, version, installTestPlugin) + if err != nil { + errList = append(errList, err) } } - err = kerrors.NewAggregate(errList) if err != nil { return err