Skip to content

Conversation

@cp319391
Copy link

Summary

This PR downgrades the log level for DiffFromCache errors caused by expected Redis cache misses (specifically cache: key is missing) from ERROR to WARN, while preserving ERROR logs for genuine Redis failures.

The diff behaviour and control flow are unchanged; Argo CD continues to fall back to a non-cache diff when the cache is unavailable.

Motivation

In Redis HA / Sentinel setups, cache misses are expected during events such as:

  • Redis restarts
  • Sentinel master failovers
  • Replica resynchronisation

In these scenarios, Argo CD currently logs DiffFromCache failures at ERROR level even though:

  • The cache is best-effort and rebuildable
  • Applications recover automatically
  • No user action is required

This results in repeated error-level log noise and false alerts in otherwise healthy clusters.

What this PR changes

  • Downgrades the log level to WARN when the error message contains cache: key is missing
  • Retains ERROR level logging for all other Redis/cache errors (e.g. connection refused, timeouts, auth failures)
  • Does not change application behaviour, reconcile logic, or diff results

Why this is safe

  • Cache misses already trigger a fallback to live diff
  • No functional or behavioural changes are introduced
  • Only the log severity is adjusted for a known benign case

Scope

  • No CLI changes
  • No UI changes
  • No API changes
  • No documentation changes required

Related issues

Fixes: #5068

Update diff.go change log level error to warn

Signed-off-by: CPunia <[email protected]>
@cp319391 cp319391 requested a review from a team as a code owner January 30, 2026 06:53
@bunnyshell
Copy link

bunnyshell bot commented Jan 30, 2026

❗ Preview Environment deployment failed on Bunnyshell

See: Environment Details | Pipeline Logs

Available commands (reply to this comment):

  • 🚀 /bns:deploy to redeploy the environment
  • /bns:delete to remove the environment

fix errors

Signed-off-by: CPunia <[email protected]>
import strings fix error

Signed-off-by: CPunia <[email protected]>
arrange imports together

Signed-off-by: CPunia <[email protected]>
Signed-off-by: CPunia <[email protected]>
Signed-off-by: CPunia <[email protected]>
fixing classic gofumpt vs CI moment :)

Signed-off-by: CPunia <[email protected]>
err := c.stateCache.GetAppManagedResources(appName, &cachedDiff)
if err != nil {
log.Errorf("DiffFromCache error: error getting managed resources for app %s: %s", appName, err)
if strings.Contains(err.Error(), "cache: key is missing") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using a hard-coded value that is vulnerable to future changes, wouldn't it be better to check that the error is ErrCacheMiss?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error log on missing cache key

2 participants