Skip to content
New issue

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

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

Already on GitHub? # to your account

Check cluster ID during service export #255

Merged
merged 3 commits into from
Dec 8, 2022

Conversation

bansal19
Copy link
Contributor

@bansal19 bansal19 commented Dec 6, 2022

Issue #, if available: #254

Description of changes:
When exporting a service from Cluster A, we call DiscoverInstances on an entire clusterset in order to determine if there are any changes for the service we wish to export. If there are any changes, all endpoints in that service will be wiped by the endpoints being exported by Cluster A for this service. In order to mitigate this, we must check for the current cluster's endpoints only when calling DiscoverInstances to Cloud Map.

To make sure we obtain the correct endpoints, we will need to invalidate the cache.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

pkg/cloudmap/client.go Outdated Show resolved Hide resolved
pkg/cloudmap/client.go Outdated Show resolved Hide resolved
pkg/cloudmap/client.go Outdated Show resolved Hide resolved
pkg/cloudmap/client_test.go Outdated Show resolved Hide resolved
@bansal19 bansal19 requested a review from CurtisThe December 8, 2022 20:57
@codecov-commenter
Copy link

codecov-commenter commented Dec 8, 2022

Codecov Report

Merging #255 (b094033) into main (ba90126) will decrease coverage by 0.27%.
The diff coverage is 94.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #255      +/-   ##
==========================================
- Coverage   70.86%   70.59%   -0.28%     
==========================================
  Files          17       17              
  Lines        1713     1724      +11     
==========================================
+ Hits         1214     1217       +3     
- Misses        417      423       +6     
- Partials       82       84       +2     
Impacted Files Coverage Δ
...ntrollers/multicluster/serviceexport_controller.go 37.37% <90.90%> (-1.67%) ⬇️
pkg/model/types.go 66.47% <100.00%> (+1.22%) ⬆️

Continue to review full report at Codecov.

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

@bansal19 bansal19 requested a review from jaywasd December 8, 2022 20:57
Copy link
Contributor

@astaticvoid astaticvoid left a comment

Choose a reason for hiding this comment

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

Thanks for the fix and improvements :)

@astaticvoid astaticvoid linked an issue Dec 8, 2022 that may be closed by this pull request
@bansal19 bansal19 merged commit d68c768 into aws:main Dec 8, 2022
@bansal19 bansal19 deleted the beshard/service-export-bug-fix branch December 8, 2022 23:23
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Service's endpoints being replaced during export
6 participants