Improve startup speed (#821)

* Avoid unnecessary synching of Context and Servers

Before this commit, the CLI would always perform a two-way sync of the
Contexts of the config-ng.yaml file and the Servers of the config.yaml
file, even if there was no reason to sync, i.e., if the Contexts
and Servers were already in sync.  This sync was being done for every
single command of the CLI.

This caused a write to the config files for each contexts configured
and one more for the current context.  With many contexts configured,
this unnecessary sync would make the CLI startup noticeably slower.

This commit verifies if the sync is required by computing and storing a
SHA for the "context" section and one for the "server" section, and
checking if the SHAs have changed on each CLI command and if so, only
then doing the sync. Note that Tanzu contexts are kept out of the
SHA computation since they are not synced.

* Write to datastore and config files only if needed

The logic to store the last execute CLI would always write to the
datastore and to the config files, which should be avoided.

This commit first checks if the update is required or not.

Signed-off-by: Marc Khouzam <[email protected]>

pkg/config/context.go

@@ -4,18 +4,37 @@
 package config
 
 import (
+	"crypto/sha256"
+	"encoding/hex"
+
 	"github.com/pkg/errors"
+	"gopkg.in/yaml.v2"
 
+	"github.com/vmware-tanzu/tanzu-cli/pkg/datastore"
 	"github.com/vmware-tanzu/tanzu-plugin-runtime/config"
+	"github.com/vmware-tanzu/tanzu-plugin-runtime/config/types"
 )
 
+type contextAndServerSha struct {
+	ContextsSha        string
+	CurrentContextsSha string
+	ServersSha         string
+	CurrentServerSha   string
+}
+
+const lastContextServerShasKey = "lastContextServerShas"
+
 // SyncContextsAndServers populate or sync contexts and servers
 func SyncContextsAndServers() error {
 	cfg, err := config.GetClientConfig()
 	if err != nil {
 		return errors.Wrap(err, "failed to get client config")
 	}
 
+	if !shouldSyncContextsAndServers(cfg) {
+		return nil
+	}
+
 	config.PopulateContexts(cfg)
 
 	// Now write the context to the configuration file.  This will also create any missing server for its corresponding context
@@ -34,5 +53,76 @@ func SyncContextsAndServers() error {
 			return errors.Wrap(err, "failed to set active context")
 		}
 	}
+
+	// After the sync, save the shas using the updated config
+	// so that next time, we avoid synching if the shas are the same
+	cfg, err = config.GetClientConfig()
+	if err == nil {
+		_ = saveContextAndServerShas(cfg)
+	}
 	return nil
 }
+
+func shouldSyncContextsAndServers(cfg *types.ClientConfig) bool {
+	var prevShas contextAndServerSha
+	err := datastore.GetDataStoreValue(lastContextServerShasKey, &prevShas)
+	if err != nil {
+		return true
+	}
+
+	newShas := computeShasForSync(cfg)
+	if newShas == nil || prevShas != *newShas {
+		return true
+	}
+	return false
+}
+
+//nolint:staticcheck // Deprecated
+func computeShasForSync(cfg *types.ClientConfig) *contextAndServerSha {
+	// tanzu contexts are not synced so we don't include them in the sha
+	// because even if they change, we don't need to sync them
+	var nonTanzuContexts []*types.Context
+	for i := range cfg.KnownContexts {
+		if cfg.KnownContexts[i].ContextType != types.ContextTypeTanzu {
+			nonTanzuContexts = append(nonTanzuContexts, cfg.KnownContexts[i])
+		}
+	}
+	ctxBytes, err := yaml.Marshal(nonTanzuContexts)
+	if err != nil {
+		return nil
+	}
+
+	// tanzu servers are not synced so we don't include them in the sha
+	// because even if they change, we don't need to sync them.
+	// Note that tanzu servers should normally not be in the known servers list
+	// but could happen for some older plugins or CLI versions which used to sync them.
+	var nonTanzuServers []*types.Server
+	for i := range cfg.KnownServers {
+		if cfg.KnownServers[i].Type != types.ServerType(types.ContextTypeTanzu) {
+			nonTanzuServers = append(nonTanzuServers, cfg.KnownServers[i])
+		}
+	}
+	serverBytes, err := yaml.Marshal(nonTanzuServers)
+	if err != nil {
+		return nil
+	}
+
+	return &contextAndServerSha{
+		ContextsSha: hashString(string(ctxBytes)),
+		// Only the k8s current context is synched
+		CurrentContextsSha: hashString(cfg.CurrentContext[types.ContextTypeK8s]),
+		ServersSha:         hashString(string(serverBytes)),
+		CurrentServerSha:   hashString(cfg.CurrentServer),
+	}
+}
+
+func saveContextAndServerShas(cfg *types.ClientConfig) error {
+	newShas := computeShasForSync(cfg)
+	return datastore.SetDataStoreValue(lastContextServerShasKey, newShas)
+}
+
+func hashString(str string) string {
+	h := sha256.New()
+	h.Write([]byte(str))
+	return hex.EncodeToString(h.Sum(nil))
+}

pkg/lastversion/last_version.go

@@ -47,10 +47,17 @@ func GetLastExecutedCLIVersion() string {
 
 // SetLastExecutedCLIVersion sets the last executed CLI version in the datastore.
 func SetLastExecutedCLIVersion() {
-	_ = datastore.SetDataStoreValue(lastExecutedCLIVersionKey, lastExecutedCLIVersion{Version: buildinfo.Version})
+	var prevLastVersion lastExecutedCLIVersion
+	_ = datastore.GetDataStoreValue(lastExecutedCLIVersionKey, &prevLastVersion)
+	if prevLastVersion.Version != buildinfo.Version {
+		// Only update the last executed version if it is different from the one already stored.
+		_ = datastore.SetDataStoreValue(lastExecutedCLIVersionKey, lastExecutedCLIVersion{Version: buildinfo.Version})
+	}
 
 	// Just in case the 'features.global.context-target-v2' feature flag is still set
 	// because the last version executed was < 1.3.0, we must remove it.
 	parts := strings.Split(constants.FeatureContextCommand, ".")
-	_ = config.DeleteFeature(parts[1], parts[2])
+	if enabled, err := config.IsFeatureEnabled(parts[1], parts[2]); err == nil && enabled {
+		_ = config.DeleteFeature(parts[1], parts[2])
+	}
 }