Skip to content

Commit

Permalink
log: debug manifest cache (#9602)
Browse files Browse the repository at this point in the history 
* log: debug manifest cache

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>

fix silly refactor

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>

* fix tests

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>

* shorter function signatures

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>

* less duplication

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>
  • Loading branch information
@crenshaw-dev
crenshaw-dev authored Jun 16, 2022
1 parent 5c4e38a commit c478617
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 5 deletions.
43 changes: 38 additions & 5 deletions reposerver/cache/cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,24 +63,35 @@ func AddCacheFlagsToCmd(cmd *cobra.Command, opts ...func(client *redis.Client))
}

func appSourceKey(appSrc *appv1.ApplicationSource) uint32 {
return hash.FNVa(appSourceKeyJSON(appSrc))
}

func appSourceKeyJSON(appSrc *appv1.ApplicationSource) string {
appSrc = appSrc.DeepCopy()
if !appSrc.IsHelm() {
appSrc.RepoURL = "" // superseded by commitSHA
appSrc.TargetRevision = "" // superseded by commitSHA
}
appSrcStr, _ := json.Marshal(appSrc)
return hash.FNVa(string(appSrcStr))
return string(appSrcStr)
}

func clusterRuntimeInfoKey(info ClusterRuntimeInfo) uint32 {
if info == nil {
return 0
}
key := clusterRuntimeInfoKeyUnhashed(info)
return hash.FNVa(key)
}

// clusterRuntimeInfoKeyUnhashed gets the cluster runtime info for a cache key, but does not hash the info. Does not
// check if info is nil, the caller must do that.
func clusterRuntimeInfoKeyUnhashed(info ClusterRuntimeInfo) string {
apiVersions := info.GetApiVersions()
sort.Slice(apiVersions, func(i, j int) bool {
return apiVersions[i] < apiVersions[j]
})
return hash.FNVa(info.GetKubeVersion() + "|" + strings.Join(apiVersions, ","))
return info.GetKubeVersion() + "|" + strings.Join(apiVersions, ",")
}

func listApps(repoURL, revision string) string {
Expand Down Expand Up @@ -139,11 +150,32 @@ func (c *Cache) GetGitReferences(repo string, references *[]*plumbing.Reference)
}

func manifestCacheKey(revision string, appSrc *appv1.ApplicationSource, namespace string, trackingMethod string, appLabelKey string, appName string, info ClusterRuntimeInfo) string {
trackingKey := trackingKey(appLabelKey, trackingMethod)
return fmt.Sprintf("mfst|%s|%s|%s|%s|%d", trackingKey, appName, revision, namespace, appSourceKey(appSrc)+clusterRuntimeInfoKey(info))
}

func trackingKey(appLabelKey string, trackingMethod string) string {
trackingKey := appLabelKey
if text.FirstNonEmpty(trackingMethod, string(argo.TrackingMethodLabel)) != string(argo.TrackingMethodLabel) {
trackingKey = trackingMethod + ":" + trackingKey
}
return fmt.Sprintf("mfst|%s|%s|%s|%s|%d", trackingKey, appName, revision, namespace, appSourceKey(appSrc)+clusterRuntimeInfoKey(info))
return trackingKey
}

// LogDebugManifestCacheKeyFields logs all the information included in a manifest cache key. It's intended to be run
// before every manifest cache operation to help debug cache misses.
func LogDebugManifestCacheKeyFields(message string, reason string, revision string, appSrc *appv1.ApplicationSource, clusterInfo ClusterRuntimeInfo, namespace string, trackingMethod string, appLabelKey string, appName string) {
if log.IsLevelEnabled(log.DebugLevel) {
log.WithFields(log.Fields{
"revision": revision,
"appSrc": appSourceKeyJSON(appSrc),
"namespace": namespace,
"trackingKey": trackingKey(appLabelKey, trackingMethod),
"appName": appName,
"clusterInfo": clusterRuntimeInfoKeyUnhashed(clusterInfo),
"reason": reason,
}).Debug(message)
}
}

func (c *Cache) GetManifests(revision string, appSrc *appv1.ApplicationSource, clusterInfo ClusterRuntimeInfo, namespace string, trackingMethod string, appLabelKey string, appName string, res *CachedManifestResponse) error {
Expand All @@ -162,6 +194,8 @@ func (c *Cache) GetManifests(revision string, appSrc *appv1.ApplicationSource, c
if hash != res.CacheEntryHash || res.ManifestResponse == nil && res.MostRecentError == "" {
log.Warnf("Manifest hash did not match expected value or cached manifests response is empty, treating as a cache miss: %s", appName)

LogDebugManifestCacheKeyFields("deleting manifests cache", "manifest hash did not match or cached response is empty", revision, appSrc, clusterInfo, namespace, trackingMethod, appLabelKey, appName)

err = c.DeleteManifests(revision, appSrc, clusterInfo, namespace, trackingMethod, appLabelKey, appName)
if err != nil {
return fmt.Errorf("Unable to delete manifest after hash mismatch, %v", err)
Expand All @@ -178,7 +212,6 @@ func (c *Cache) GetManifests(revision string, appSrc *appv1.ApplicationSource, c
}

func (c *Cache) SetManifests(revision string, appSrc *appv1.ApplicationSource, clusterInfo ClusterRuntimeInfo, namespace string, trackingMethod string, appLabelKey string, appName string, res *CachedManifestResponse) error {

// Generate and apply the cache entry hash, before writing
if res != nil {
res = res.shallowCopy()
Expand All @@ -192,7 +225,7 @@ func (c *Cache) SetManifests(revision string, appSrc *appv1.ApplicationSource, c
return c.cache.SetItem(manifestCacheKey(revision, appSrc, namespace, trackingMethod, appLabelKey, appName, clusterInfo), res, c.repoCacheExpiration, res == nil)
}

func (c *Cache) DeleteManifests(revision string, appSrc *appv1.ApplicationSource, clusterInfo ClusterRuntimeInfo, namespace string, trackingMethod string, appLabelKey string, appName string) error {
func (c *Cache) DeleteManifests(revision string, appSrc *appv1.ApplicationSource, clusterInfo ClusterRuntimeInfo, namespace, trackingMethod, appLabelKey, appName string) error {
return c.cache.SetItem(manifestCacheKey(revision, appSrc, namespace, trackingMethod, appLabelKey, appName, clusterInfo), "", c.repoCacheExpiration, true)
}

Expand Down
14 changes: 14 additions & 0 deletions reposerver/repository/repository.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,7 @@ func (s *Service) runManifestGenAsync(ctx context.Context, repoRoot, commitSHA,
if err != nil {
// If manifest generation error caching is enabled
if s.initConstants.PauseGenerationAfterFailedGenerationAttempts > 0 {
cache.LogDebugManifestCacheKeyFields("getting manifests cache", "GenerateManifests error", cacheKey, q.ApplicationSource, q, q.Namespace, q.TrackingMethod, q.AppLabelKey, q.AppName)

// Retrieve a new copy (if available) of the cached response: this ensures we are updating the latest copy of the cache,
// rather than a copy of the cache that occurred before (a potentially lengthy) manifest generation.
Expand All @@ -480,6 +481,8 @@ func (s *Service) runManifestGenAsync(ctx context.Context, repoRoot, commitSHA,
innerRes.FirstFailureTimestamp = s.now().Unix()
}

cache.LogDebugManifestCacheKeyFields("setting manifests cache", "GenerateManifests error", cacheKey, q.ApplicationSource, q, q.Namespace, q.TrackingMethod, q.AppLabelKey, q.AppName)

// Update the cache to include failure information
innerRes.NumberOfConsecutiveFailures++
innerRes.MostRecentError = err.Error()
Expand All @@ -494,6 +497,9 @@ func (s *Service) runManifestGenAsync(ctx context.Context, repoRoot, commitSHA,
ch.errCh <- err
return
}

cache.LogDebugManifestCacheKeyFields("setting manifests cache", "fresh GenerateManifests response", cacheKey, q.ApplicationSource, q, q.Namespace, q.TrackingMethod, q.AppLabelKey, q.AppName)

// Otherwise, no error occurred, so ensure the manifest generation error data in the cache entry is reset before we cache the value
manifestGenCacheEntry := cache.CachedManifestResponse{
ManifestResponse: manifestGenResult,
Expand All @@ -518,6 +524,8 @@ func (s *Service) runManifestGenAsync(ctx context.Context, repoRoot, commitSHA,
// and returns true otherwise.
// If true is returned, either the second or third parameter (but not both) will contain a value from the cache (a ManifestResponse, or error, respectively)
func (s *Service) getManifestCacheEntry(cacheKey string, q *apiclient.ManifestRequest, firstInvocation bool) (bool, *apiclient.ManifestResponse, error) {
cache.LogDebugManifestCacheKeyFields("getting manifests cache", "GenerateManifest API call", cacheKey, q.ApplicationSource, q, q.Namespace, q.TrackingMethod, q.AppLabelKey, q.AppName)

res := cache.CachedManifestResponse{}
err := s.cache.GetManifests(cacheKey, q.ApplicationSource, q, q.Namespace, q.TrackingMethod, q.AppLabelKey, q.AppName, &res)
if err == nil {
Expand All @@ -537,6 +545,8 @@ func (s *Service) getManifestCacheEntry(cacheKey string, q *apiclient.ManifestRe

// After X minutes, reset the cache and retry the operation (e.g. perhaps the error is ephemeral and has passed)
if elapsedTimeInMinutes >= s.initConstants.PauseGenerationOnFailureForMinutes {
cache.LogDebugManifestCacheKeyFields("deleting manifests cache", "manifest hash did not match or cached response is empty", cacheKey, q.ApplicationSource, q, q.Namespace, q.TrackingMethod, q.AppLabelKey, q.AppName)

// We can now try again, so reset the cache state and run the operation below
err = s.cache.DeleteManifests(cacheKey, q.ApplicationSource, q, q.Namespace, q.TrackingMethod, q.AppLabelKey, q.AppName)
if err != nil {
Expand All @@ -551,6 +561,8 @@ func (s *Service) getManifestCacheEntry(cacheKey string, q *apiclient.ManifestRe
if s.initConstants.PauseGenerationOnFailureForRequests > 0 && res.NumberOfCachedResponsesReturned > 0 {

if res.NumberOfCachedResponsesReturned >= s.initConstants.PauseGenerationOnFailureForRequests {
cache.LogDebugManifestCacheKeyFields("deleting manifests cache", "reset after paused generation count", cacheKey, q.ApplicationSource, q, q.Namespace, q.TrackingMethod, q.AppLabelKey, q.AppName)

// We can now try again, so reset the error cache state and run the operation below
err = s.cache.DeleteManifests(cacheKey, q.ApplicationSource, q, q.Namespace, q.TrackingMethod, q.AppLabelKey, q.AppName)
if err != nil {
Expand All @@ -567,6 +579,8 @@ func (s *Service) getManifestCacheEntry(cacheKey string, q *apiclient.ManifestRe
cachedErrorResponse := fmt.Errorf(cachedManifestGenerationPrefix+": %s", res.MostRecentError)

if firstInvocation {
cache.LogDebugManifestCacheKeyFields("setting manifests cache", "update error count", cacheKey, q.ApplicationSource, q, q.Namespace, q.TrackingMethod, q.AppLabelKey, q.AppName)

// Increment the number of returned cached responses and push that new value to the cache
// (if we have not already done so previously in this function)
res.NumberOfCachedResponsesReturned++
Expand Down
6 changes: 6 additions & 0 deletions util/webhook/webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,10 +282,16 @@ func (a *ArgoCDWebhookHandler) storePreviouslyCachedManifests(app *v1alpha1.Appl
if err != nil {
return err
}

cache.LogDebugManifestCacheKeyFields("getting manifests cache", "webhook app revision changed", change.shaBefore, &app.Spec.Source, &clusterInfo, app.Spec.Destination.Namespace, trackingMethod, appInstanceLabelKey, app.Name)

var cachedManifests cache.CachedManifestResponse
if err := a.repoCache.GetManifests(change.shaBefore, &app.Spec.Source, &clusterInfo, app.Spec.Destination.Namespace, trackingMethod, appInstanceLabelKey, app.Name, &cachedManifests); err == nil {
return err
}

cache.LogDebugManifestCacheKeyFields("setting manifests cache", "webhook app revision changed", change.shaAfter, &app.Spec.Source, &clusterInfo, app.Spec.Destination.Namespace, trackingMethod, appInstanceLabelKey, app.Name)

if err = a.repoCache.SetManifests(change.shaAfter, &app.Spec.Source, &clusterInfo, app.Spec.Destination.Namespace, trackingMethod, appInstanceLabelKey, app.Name, &cachedManifests); err != nil {
return err
}
Expand Down

0 comments on commit c478617

Please sign in to comment.