From 72dce8da6213308912acb2b6b7ec87d178b540ef Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Tue, 28 Jan 2025 19:36:38 -0500 Subject: [PATCH] Use a different command tree file To allow older CLIs (< 1.5.3) to continue using the old command-tree format for telemetry, this commit uses a new command_tree_v2.yaml file for the new format. Switching back and forth from new to old CLIs will no longer mess with the same plugin tree cache and will keep telemetry in a better shape for older CLIs. This change also means we do not need to reset the command_tree.yaml file using a global initializer, so this commit remove all chaneges related to that. Signed-off-by: Marc Khouzam --- pkg/globalinit/catalog_cache_init.go | 2 +- .../plugin_discovery_source_update.go | 2 +- pkg/globalinit/telemetry_cache_reset.go | 37 ------ pkg/lastversion/last_version.go | 51 +-------- pkg/lastversion/last_version_test.go | 107 +----------------- pkg/plugincmdtree/plugins_cache.go | 3 +- pkg/plugincmdtree/plugins_cache_test.go | 2 +- 7 files changed, 12 insertions(+), 192 deletions(-) delete mode 100644 pkg/globalinit/telemetry_cache_reset.go diff --git a/pkg/globalinit/catalog_cache_init.go b/pkg/globalinit/catalog_cache_init.go index d6b28ebf5..5e4a9e45a 100644 --- a/pkg/globalinit/catalog_cache_init.go +++ b/pkg/globalinit/catalog_cache_init.go @@ -28,7 +28,7 @@ func init() { func triggerForPreCommandRemapping() bool { // If the last executed CLI version is < 1.3.0, we need to refresh the plugin catalog. - return lastversion.IsLessThan(lastversion.Version1_3_0) + return lastversion.GetLastExecutedCLIVersion() == lastversion.OlderThan1_3_0 } // refreshPluginCatalog reads the info from each installed plugin diff --git a/pkg/globalinit/plugin_discovery_source_update.go b/pkg/globalinit/plugin_discovery_source_update.go index cbbbc8fc0..1a62372f0 100644 --- a/pkg/globalinit/plugin_discovery_source_update.go +++ b/pkg/globalinit/plugin_discovery_source_update.go @@ -22,7 +22,7 @@ func init() { func triggerForPluginDiscoverySourceUpdater() bool { // If the last executed CLI version is < 1.3.0, we need to update the discovery image source - return lastversion.IsLessThan(lastversion.Version1_3_0) + return lastversion.GetLastExecutedCLIVersion() == lastversion.OlderThan1_3_0 } // updatePluginDiscoverySource updates the plugin discovery source to point CLI to diff --git a/pkg/globalinit/telemetry_cache_reset.go b/pkg/globalinit/telemetry_cache_reset.go deleted file mode 100644 index b408143ae..000000000 --- a/pkg/globalinit/telemetry_cache_reset.go +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2025 VMware, Inc. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -package globalinit - -import ( - "io" - - "github.com/vmware-tanzu/tanzu-cli/pkg/lastversion" - "github.com/vmware-tanzu/tanzu-cli/pkg/plugincmdtree" - "github.com/vmware-tanzu/tanzu-plugin-runtime/log" -) - -// This global initializer checks if the last executed CLI version is < 1.5.3. -// If so it removes the plugin command-tree cache file used by telemetry. -// This allows a fresh cache to be gradually built using the latest command tree format. -// Note that this initializer does not build the cache, it only removes the existing cache file. - -func init() { - RegisterInitializer("Plugin Command-Tree Reset", triggerForPluginCmdTreeReset, resetPluginCmdTree) -} - -func triggerForPluginCmdTreeReset() bool { - // If the last executed CLI version is < 1.5.3, we need to remove the command-tree cache file. - return lastversion.IsLessThan(lastversion.Version1_5_3) -} - -// resetPluginCmdTree deletes the plugin command-tree cache. -func resetPluginCmdTree(_ io.Writer) error { - pct, err := plugincmdtree.NewCache() - if err != nil { - return err - } - - log.V(7).Infof("Clearing telemetry plugin command-tree cache for all plugins...") - return pct.DeleteTree() -} diff --git a/pkg/lastversion/last_version.go b/pkg/lastversion/last_version.go index ea416328b..a88bce623 100644 --- a/pkg/lastversion/last_version.go +++ b/pkg/lastversion/last_version.go @@ -8,8 +8,6 @@ package lastversion import ( "strings" - "github.com/Masterminds/semver" - "github.com/vmware-tanzu/tanzu-cli/pkg/buildinfo" "github.com/vmware-tanzu/tanzu-cli/pkg/constants" "github.com/vmware-tanzu/tanzu-cli/pkg/datastore" @@ -19,14 +17,7 @@ import ( // lastExecutedCLIVersionKey is the key used to store the last executed CLI version in the datastore. const ( lastExecutedCLIVersionKey = "lastExecutedCLIVersion" - olderThan1_3_0 = "olderThan1_3_0" -) - -var ( - // Version1_3_0 is the semver version 1.3.0 - Version1_3_0 = semver.MustParse("1.3.0") - // Version1_5_3 is the semver version 1.5.3 - Version1_5_3 = semver.MustParse("1.5.3") + OlderThan1_3_0 = "olderThan1_3_0" ) // lastExecutedCLIVersion is a struct used to store the last executed CLI version in the datastore. @@ -35,42 +26,10 @@ type lastExecutedCLIVersion struct { Version string `json:"version" yaml:"version"` } -// IsLessThan checks if the last executed CLI version is less (older) than the specified version. -// Any version >= 1.3.0 can be specified as an argument, such as the constants [Version1_3_0], [Version1_5_3]. -func IsLessThan(otherVersion *semver.Version) bool { - if otherVersion.LessThan(Version1_3_0) { - // This should NOT happen. - // The specified otherVersion should never be less than 1.3.0 - // for any last executed CLI version < 1.3.0, all we know is that the - // version is older than 1.3.0, we don't know the actual version. - // Therefore the caller of this function should never pass a version - // less than 1.3.0. - return false - } - - lastExecutedVersion := getLastExecutedCLIVersion() - - // Special case for last executed version < 1.3.0 - if lastExecutedVersion == olderThan1_3_0 { - // If the last executed version is < 1.3.0, then it is older than any - // other version that we can compare against. - return true - } - - lastExecutedVersionSemver, err := semver.NewVersion(lastExecutedVersion) - if err != nil { - // This should not happen. It means that the last executed version - // stored in the datastore is invalid. - return false - } - - return lastExecutedVersionSemver.LessThan(otherVersion) -} - -// getLastExecutedCLIVersion gets the last executed CLI version from the datastore. -// If the last executed version is < 1.3.0, it returns OlderThan1_3_0, otherwise +// GetLastExecutedCLIVersion gets the last executed CLI version from the datastore. +// If the last executed version is < 1.3.0, it returns an empty string, otherwise // it returns the last executed version. -func getLastExecutedCLIVersion() string { +func GetLastExecutedCLIVersion() string { if config.IsFeatureActivated(constants.FeatureContextCommand) { // If this feature flag is present, then we know that the // last version executed was < 1.3.0. We cannot know which version @@ -78,7 +37,7 @@ func getLastExecutedCLIVersion() string { // to set the last executed version. In this case, we return an empty string. // Don't return an empty string as it would be the same as the value returned // if the datastore is removed. Instead, return a constant value. - return olderThan1_3_0 + return OlderThan1_3_0 } var lastVersion lastExecutedCLIVersion diff --git a/pkg/lastversion/last_version_test.go b/pkg/lastversion/last_version_test.go index 9ecb7c98e..36f5f6876 100644 --- a/pkg/lastversion/last_version_test.go +++ b/pkg/lastversion/last_version_test.go @@ -8,7 +8,6 @@ import ( "strings" "testing" - "github.com/Masterminds/semver" "github.com/stretchr/testify/assert" "github.com/vmware-tanzu/tanzu-cli/pkg/buildinfo" @@ -42,7 +41,7 @@ func TestGetLastExecutedCLIVersion(t *testing.T) { { test: "last version older than 1.3.0 ", lastVersion: "1.2.0", - expectedVersion: olderThan1_3_0, + expectedVersion: OlderThan1_3_0, }, } @@ -80,7 +79,7 @@ func TestGetLastExecutedCLIVersion(t *testing.T) { _ = datastore.SetDataStoreValue(lastExecutedCLIVersionKey, lastExecutedCLIVersion{Version: spec.lastVersion}) // Get the last executed version and verify - lastVersion := getLastExecutedCLIVersion() + lastVersion := GetLastExecutedCLIVersion() assert.Equal(spec.expectedVersion, lastVersion) // Clean up @@ -165,105 +164,3 @@ func TestSetLastExecutedCLIVersion(t *testing.T) { }) } } - -func TestIsLessThan(t *testing.T) { - tests := []struct { - test string - lastVersion string - lessThanVersion string - expectedResult bool - }{ - { - test: "last version older than 1.3.0 compared to 1.5.0", - lastVersion: olderThan1_3_0, - lessThanVersion: "1.5.0", - expectedResult: true, - }, - { - test: "last version older than 1.3.0 compared to 2.0.0", - lastVersion: olderThan1_3_0, - lessThanVersion: "2.0.0", - expectedResult: true, - }, - { - // Not supported so returns false - test: "last version older than 1.3.0 compared to 1.2.0", - lastVersion: olderThan1_3_0, - lessThanVersion: "1.2.0", - expectedResult: false, - }, - { - test: "last version 1.5.3 compared to 1.3.0", - lastVersion: "1.5.3", - lessThanVersion: "1.3.0", - expectedResult: false, - }, - { - test: "last version 1.5.3 compared to 1.5.0", - lastVersion: "1.5.3", - lessThanVersion: "1.5.0", - expectedResult: false, - }, - { - test: "last version 1.5.3 compared to 1.5.4", - lastVersion: "1.5.3", - lessThanVersion: "1.5.4", - expectedResult: true, - }, - { - test: "last version 1.5.3 compared to 1.6.0", - lastVersion: "1.5.3", - lessThanVersion: "1.6.0", - expectedResult: true, - }, - { - test: "last version 1.5.3 compared to 2.0.0", - lastVersion: "1.5.3", - lessThanVersion: "2.0.0", - expectedResult: true, - }, - } - - assert := assert.New(t) - - // Setup a temporary configuration - configFile, err := os.CreateTemp("", "config") - assert.Nil(err) - err = os.Setenv("TANZU_CONFIG", configFile.Name()) - assert.Nil(err) - defer os.RemoveAll(configFile.Name()) - defer os.Unsetenv("TANZU_CONFIG") - - configFileNG, err := os.CreateTemp("", "config_ng") - assert.Nil(err) - err = os.Setenv("TANZU_CONFIG_NEXT_GEN", configFileNG.Name()) - assert.Nil(err) - defer os.RemoveAll(configFileNG.Name()) - defer os.Unsetenv("TANZU_CONFIG_NEXT_GEN") - - tmpDataStoreFile, _ := os.CreateTemp("", "data-store.yaml") - defer os.RemoveAll(tmpDataStoreFile.Name()) - os.Setenv("TEST_CUSTOM_DATA_STORE_FILE", tmpDataStoreFile.Name()) - defer os.Unsetenv("TEST_CUSTOM_DATA_STORE_FILE") - - for _, spec := range tests { - t.Run(spec.test, func(t *testing.T) { - if spec.lastVersion == olderThan1_3_0 { - // We need to set the 'features.global.context-target-v2' feature flag - // to indicate that the last version executed was < 1.3.0. - parts := strings.Split(constants.FeatureContextCommand, ".") - _ = config.SetFeature(parts[1], parts[2], "true") - } else { - // Set the last executed version in the datastore - _ = datastore.SetDataStoreValue(lastExecutedCLIVersionKey, lastExecutedCLIVersion{Version: spec.lastVersion}) - } - - // Compare the versions - assert.Equal(spec.expectedResult, IsLessThan(semver.MustParse(spec.lessThanVersion))) - - // Clean up - parts := strings.Split(constants.FeatureContextCommand, ".") - _ = config.DeleteFeature(parts[1], parts[2]) - }) - } -} diff --git a/pkg/plugincmdtree/plugins_cache.go b/pkg/plugincmdtree/plugins_cache.go index fd61e2656..146c7331b 100644 --- a/pkg/plugincmdtree/plugins_cache.go +++ b/pkg/plugincmdtree/plugins_cache.go @@ -38,7 +38,7 @@ func getPluginsCommandTreeCacheDir() string { return filepath.Join(common.DefaultCacheDir, pluginsCommandTreeDir) } func GetPluginsCommandTreeCachePath() string { - return filepath.Join(getPluginsCommandTreeCacheDir(), "command_tree.yaml") + return filepath.Join(getPluginsCommandTreeCacheDir(), "command_tree_v2.yaml") } func getPluginsDocsCachePath() string { @@ -133,6 +133,7 @@ func (c *cacheImpl) savePluginCommandTree() error { return nil } +//nolint:gocyclo // This function is complex func (c *cacheImpl) constructPluginCommandTree(rootCmd *cobra.Command, plugin *cli.PluginInfo) (*CommandNode, error) { if err := c.pluginDocsGenerator(plugin); err != nil { return nil, errors.Wrapf(err, "failed to generate docs for the plugin %q", plugin.Name) diff --git a/pkg/plugincmdtree/plugins_cache_test.go b/pkg/plugincmdtree/plugins_cache_test.go index b15637edd..ebcfd591d 100644 --- a/pkg/plugincmdtree/plugins_cache_test.go +++ b/pkg/plugincmdtree/plugins_cache_test.go @@ -494,7 +494,7 @@ func setupDummyPlugin(t *testing.T, dirName, pluginName, pluginAlias string) { assert.NoError(t, err) defer pluginExeFile.Close() - fmt.Fprint(pluginExeFile, fmt.Sprintf(samplePluginToGenerateAliasWithHelpCommand, pluginName, pluginAlias)) + fmt.Fprintf(pluginExeFile, samplePluginToGenerateAliasWithHelpCommand, pluginName, pluginAlias) } func validatePluginCommandTree(t *testing.T, gotPluginCommandTree *pluginCommandTree, pluginInstallationPath, expectedPluginCommandTreeYaml string) {