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.

Sign up for GitHub

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

Jump to bottom

LoadBalancer cache causes ReRoute changed by Admin API to not respect new downstream path #383

Closed
kingdamian42 opened this issue Jun 5, 2018 · 6 comments · Fixed by #421
Closed

LoadBalancer cache causes ReRoute changed by Admin API to not respect new downstream path #383

kingdamian42 opened this issue Jun 5, 2018 · 6 comments · Fixed by #421
Labels
bug Identified as a potential bug medium effort Likely a few days of development effort merged Issue has been merged to dev and is waiting for the next release

Comments

@kingdamian42
Copy link

Expected Behavior / New Feature

After changing the configuration via the Admin API, requests should be rerouted to the new configuration routes and ignore the cache.

Actual Behavior / Motivation for New Feature

After changing the configuration via the Admin API any endpoints that were previously requested will still forward to the old configuration (cached config?).

Ex:
A) localhost:5000/fooValue returns "FOO"
B) localhost:5000/barValue returns "BAR"
C) localhost:5000/fooValue/123 returns "FOO123"
D) localhost:5000/barValue/123 returns "BAR123"

Test only requests A and B. Load a new configuration pointing BAR to FOO.

Now requests to B remain "BAR" rather than changing to "FOO" as it should. However request D now properly returns "FOO123" since the request/response was not cached.

Steps to Reproduce the Problem

Specifications

  • Version:
  • Platform:
  • Subsystem:
@TomPallister
Copy link
Member

@kingdamian42 thanks for your interest in the project. Would you be able to paste your configuration json and your startup / program classes.

This sounds like a bug to me but I don’t have time to investigate until next week!

@TomPallister TomPallister added the question Initially seen a question could become a new feature or bug or closed ;) label Jun 6, 2018
@TomPallister
Copy link
Member

@kingdamian42 please can you paste your configuration json and your startup / program classes so I can investigate this further?

TomPallister pushed a commit that referenced this issue Jun 20, 2018
#383 added failing test for this issue
77b992f
@TomPallister TomPallister added bug Identified as a potential bug medium effort Likely a few days of development effort and removed question Initially seen a question could become a new feature or bug or closed ;) labels Jun 20, 2018
@TomPallister
Copy link
Member

@kingdamian42 I have got a failing test for this issue, it is a bug thanks for letting me know!! I will fix asap.

TomPallister pushed a commit that referenced this issue Jun 20, 2018
#383 identified issue was with cached load balancer for a given upstr…
067fc44 
…eam path template based on the key we use, have modified this to include more data, I guess this might be an issue again for other things so I will have a think about it
@TomPallister TomPallister changed the title Cache Not Clearing LoadBalancer cache causes ReRoute changed by Admin API to not respect new downstream path Jun 20, 2018
@TomPallister TomPallister mentioned this issue Jun 20, 2018
Merged
TomPallister added a commit that referenced this issue Jun 20, 2018
@TomPallister
#383 fixed failing tests after key change
2f9bf0d
TomPallister added a commit that referenced this issue Jun 21, 2018
@TomPallister
Feature/fix admin api caching wrong re routes (#421)
3eb9b4d 
* #383 added failing test for this issue

* #383 identified issue was with cached load balancer for a given upstream path template based on the key we use, have modified this to include more data, I guess this might be an issue again for other things so I will have a think about it

* #383 fixed failing tests after key change

* Seems to be an issue with coveralls new package not being on nuget...try same version as their nuget package

* bash the old manual tests json back in
@TomPallister TomPallister added the merged Issue has been merged to dev and is waiting for the next release label Jun 21, 2018
@TomPallister TomPallister reopened this Jun 21, 2018
@TomPallister
Copy link
Member

Released in 7.0.6

@kingdamian42
Copy link
Author

My apologies for not responding with my configuration, I'm sure it would have saved a some time and research on your end. My post was made in a bit of a rush and then as we all do I got sidetracked down another path.

Thankyou for addressing this even with the limited amount of information provided!

@TomPallister
Copy link
Member

@kingdamian42 no worries! Everyone is busy 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Identified as a potential bug medium effort Likely a few days of development effort merged Issue has been merged to dev and is waiting for the next release
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Feature/fix admin api caching wrong re routes ThreeMammals/Ocelot
2 participants
@kingdamian42 @TomPallister