Skip to content

[fix] stop ListRoutes refreshing the instance cache - #644

Merged
glennpratt merged 2 commits into
mainfrom
gpratt-fix-route-controller-instance-refresh
Oct 15, 2026
Merged

glennpratt merged 2 commits into
mainfrom
gpratt-fix-route-controller-instance-refresh

Conversation

@glennpratt

@glennpratt glennpratt commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor
  • Read the instance cache without checking its TTL, so route reconciles no longer re-list account instances and VPC IPs about every 20s.
  • Copy the cache under its read lock, fixing a data race with refreshes.
  • A node missing from the cache triggers CreateRoute, which refreshes an expired cache via LookupLinode; periodic node controller lookups keep the cache fresh.

Combined with #642, here's what I see in a quick back to back from metrics:

Calls/min v0.9.6 #642 only #644 only Both Δ
ListInstances 2.99 2.99 2.99 0.14 −95%
ListVPCIPAddresses 3.85 3.84 3.84 1.00 −74%
ListVPCIPv6Addresses 2.99 2.99 2.99 0.14 −95%
Total 9.83 9.81 9.81 1.28 −87%

General:

  • Have you removed all sensitive information, including but not limited to access keys and passwords?
  • Have you checked to ensure there aren't other open or closed Pull Requests for the same bug/feature/question?

Pull Request Guidelines:

  1. Does your submission pass tests?
  2. Have you added tests?
  3. Are you addressing a single feature in this PR?
  4. Are your commits atomic, addressing one change per commit?
  5. Are you following the conventions of the language?
  6. Have you saved your large formatting changes for a different PR, so we can focus on your work?
  7. Have you explained your rationale for why this feature is needed?
  8. Have you linked your PR to an open issue

* Read the instance cache without checking its TTL, so route reconciles
  no longer re-list account instances and VPC IPs about every 20s.
* Copy the cache under its read lock, fixing a data race with refreshes.
* A node missing from the cache triggers CreateRoute, which refreshes an
  expired cache via LookupLinode; periodic node controller lookups keep
  the cache fresh.
@github-actions github-actions Bot added the bugfix for any bug fixes in the changelog. label Oct 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

Static review found no blocking issues in cache synchronization or refresh behavior; tests were not executed.

0 open findings

What changed in this PR

Reduces Linode API calls during route reconciliation by reusing cached instances, complementing #642’s lookup reductions.

Changes:

  • Loads instances only when the cache has never been populated.
  • Copies cached instances under a read lock to prevent races.
  • Adds cache behavior tests and initializes the route test’s node cache.
File Description
cloud/​linode/​services/​instances.go Adds initialization-only loading and synchronized cache reads.
cloud/​linode/​services/​instances_test.go Tests initialization, expired-cache reuse, failures, and concurrency.
cloud/​linode/​route_controller.go Uses cached instances without TTL-triggered refreshes.
cloud/​linode/​route_controller_test.go Initializes and restores the registered node cache.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov

codecov Bot commented Oct 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.64%. Comparing base (3953ce9) to head (72d9b94).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #644      +/-   ##
==========================================
+ Coverage   82.59%   82.64%   +0.05%     
==========================================
  Files          19       19              
  Lines        2608     2616       +8     
==========================================
+ Hits         2154     2162       +8     
  Misses        454      454              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

@AshleyDumaine AshleyDumaine left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice 🔥

@glennpratt
glennpratt enabled auto-merge (squash) October 15, 2026 19:13
@glennpratt
glennpratt merged commit 6fcdcfa into main Oct 15, 2026
17 of 18 checks passed
@glennpratt
glennpratt deleted the gpratt-fix-route-controller-instance-refresh branch October 15, 2026 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix for any bug fixes in the changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants