Skip to content

fix: preserve overview updates during concurrent config applies - #1938

Open
pooyanazad wants to merge 1 commit into
nginx:mainfrom
pooyanazad:fix/1927-buffer-nginx-config-context
Open

pooyanazad wants to merge 1 commit into
nginx:mainfrom
pooyanazad:fix/1927-buffer-nginx-config-context

Conversation

@pooyanazad

Copy link
Copy Markdown

Config context updates were dropped whenever another apply was in progress. Keep the latest context per instance and publish it when watchers are re-enabled, as long as it still matches the completed apply.

Add coverage for matching and stale buffered contexts.

Fixes #1927

Proposed changes

When two identical config applies overlap, the watcher can discard the
configuration context from the first apply. The second apply then sees the
same cached context, so neither apply publishes NginxConfigUpdateTopic and
UpdateOverview is never sent.

This change buffers the latest configuration context for each instance while
a config apply is in progress. When watchers are re-enabled, the buffered
context is published only if it matches the completed apply context. Stale
contexts are discarded.

Testing

  • go test -count=1 ./internal/watcher/...
  • go test -race -count=1 ./internal/watcher

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING document
  • I have run make install-tools and have attached any dependency changes to this pull request
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • If applicable, I have checked that any relevant tests pass after adding my changes
  • Not applicable, no documentation changes are required
  • Not applicable, this change is platform-independent

@pooyanazad
pooyanazad requested a review from a team as a code owner September 16, 2026 19:41
@github-actions

github-actions Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

✅ All required contributors have signed the F5 CLA for this PR. Thank you!
Posted by the CLA Assistant Lite bot.

@github-actions github-actions Bot added bug Something isn't working chore Pull requests for routine tasks labels Sep 16, 2026
@pooyanazad

Copy link
Copy Markdown
Author

I have hereby read the F5 CLA and agree to its terms

@codecov

codecov Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.97%. Comparing base (ac2827d) to head (2984cee).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1938      +/-   ##
==========================================
+ Coverage   88.94%   88.97%   +0.02%     
==========================================
  Files         111      111              
  Lines       12413    12422       +9     
==========================================
+ Hits        11041    11052      +11     
+ Misses       1370     1368       -2     
  Partials        2        2              
Files with missing lines Coverage Δ
internal/watcher/watcher_plugin.go 76.70% <100.00%> (+4.24%) ⬆️

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac2827d...2984cee. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pooyanazad
pooyanazad force-pushed the fix/1927-buffer-nginx-config-context branch from 8b71fa9 to d184b42 Compare September 18, 2026 12:40
Config context updates were dropped whenever another apply was in
progress. Keep the latest context per instance and publish it when
watchers are re-enabled, as long as it still matches the completed
apply.

Add coverage for matching and stale buffered contexts.

Fixes nginx#1927
@pooyanazad
pooyanazad force-pushed the fix/1927-buffer-nginx-config-context branch from d184b42 to f6e5182 Compare September 18, 2026 12:41
@pooyanazad

Copy link
Copy Markdown
Author

Fixed the lint issues from the previous CI run and squashed the fixes into the original commit. Ready for another CI run and review.

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

Labels

bug Something isn't working chore Pull requests for routine tasks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Concurrent config applies cause UpdateOverview to never be sent

1 participant