From c6a6e34465c5ca1386d08f126d0855abd4671d56 Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Thu, 3 Oct 2024 22:51:16 -0400 Subject: [PATCH 1/2] 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. Signed-off-by: Marc Khouzam --- pkg/config/context.go | 90 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/pkg/config/context.go b/pkg/config/context.go index c9555310e..db6bb43a0 100644 --- a/pkg/config/context.go +++ b/pkg/config/context.go @@ -4,11 +4,26 @@ 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() @@ -16,6 +31,10 @@ func SyncContextsAndServers() error { 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)) +} From 9eab76362cc546071bea3038708126f7f2f517d0 Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Mon, 21 Oct 2024 16:06:54 -0400 Subject: [PATCH 2/2] 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 --- pkg/lastversion/last_version.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/pkg/lastversion/last_version.go b/pkg/lastversion/last_version.go index 8317b484d..a88bce623 100644 --- a/pkg/lastversion/last_version.go +++ b/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]) + } }