Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cloudfoundry): add configurable cache expiry for clients #5508

Merged
merged 4 commits into from
Oct 11, 2021

Conversation

mattgogerly
Copy link
Member

@mattgogerly mattgogerly commented Sep 2, 2021

Currently Clouddriver maintains an in-memory cache of GUIDs to Server Groups. If caching is handled by a specific pod (e.g. if cache sharding is enabled, or HA is enabled) and a different pod handles an operation that populates the cache, then this cache will never be updated.

If an application is deleted outside of Spinnaker and then redeployed with the same name, the app GUID will change. This results in Clouddriver trying to perform actions against the old GUID which no longer exists.

I've added a default expires-after-write for the Applications cache, but have made this configurable alongside an expires-after-access rule. Whilst I was at it I did the same for the Routes cache.

Also threw in some minor cleanup in files whilst I was there, making things final, using lambdas etc.

…ent and make Routes cache expiration configurable
@mattgogerly mattgogerly marked this pull request as draft September 2, 2021 20:07
@mattgogerly mattgogerly changed the title fix(cloudfoundry): add configurable cache expiry for Applications client and make Routes cache expiration configurable fix(cloudfoundry): add configurable cache expiry for clients Sep 2, 2021
@mattgogerly mattgogerly marked this pull request as ready for review September 21, 2021 09:59
@mattgogerly
Copy link
Member Author

@zachsmith1 Not sure how best to demonstrate the issue as it's more a logical issue than a code one. Any ideas let me know :)

@mattgogerly mattgogerly deleted the cf-cache-expiry branch September 30, 2021 12:11
@mattgogerly mattgogerly restored the cf-cache-expiry branch October 1, 2021 08:48
@mattgogerly mattgogerly reopened this Oct 1, 2021
@zachsmith1 zachsmith1 added ready to merge Approved and ready for a merge backport-candidate Add to PRs to designate release branch patch candidates. labels Oct 11, 2021
@mergify mergify bot added the auto merged Merged automatically by a bot label Oct 11, 2021
@mergify mergify bot merged commit d12b5f0 into spinnaker:master Oct 11, 2021
@mattgogerly mattgogerly deleted the cf-cache-expiry branch October 11, 2021 20:58
@link108
Copy link
Member

link108 commented Oct 12, 2021

@Mergifyio backport release-1.27.x

mergify bot pushed a commit that referenced this pull request Oct 12, 2021
* fix(cloudfoundry): add configurable cache expiry for Applications client and make Routes cache expiration configurable

* fix(cloudfoundry): merge conflict

(cherry picked from commit d12b5f0)

# Conflicts:
#	clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/Applications.java
@mergify
Copy link
Contributor

mergify bot commented Oct 12, 2021

Command backport release-1.27.x: success

Backports have been created

link108 added a commit that referenced this pull request Oct 12, 2021
…#5508) (#5555)

Co-authored-by: Matt <6519811+mattgogerly@users.noreply.github.com>
Co-authored-by: Cameron Motevasselani <cameron@armory.io>
Co-authored-by: Cameron Motevasselani <cmotevasselani@gmail.com>
@link108 link108 removed the backport-candidate Add to PRs to designate release branch patch candidates. label Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto merged Merged automatically by a bot ready to merge Approved and ready for a merge target-release/1.28
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants