Skip to content

Conversation

runakash
Copy link
Member

Issue #, if available: Fixes #20

Description of changes: Adding the listener to the Service Export Controller to track EndpointSlice events. EndpointSlice events are triggered by Service or Deployment changes. Refactor ServiceExport to handle the deletion scenario when the Service corresponding to ServiceExport is deleted. Also, updating the e2e test to accommodate the scale-up scenario. Improvements in unit tests.

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

…ts. EndpointSlice events are triggered by Service or Deployment changes. Refactor ServiceExport to handle the deletion scenario when the Service corresponding to ServiceExport is deleted. Also, updating the e2e test to accommodate the scale-up scenario. Improvements in unit tests.
@codecov-commenter
Copy link

codecov-commenter commented Nov 18, 2021

Codecov Report

Attention: Patch coverage is 31.57895% with 91 lines in your changes missing coverage. Please review.

Project coverage is 73.52%. Comparing base (94c4488) to head (c5f7b73).
Report is 161 commits behind head on main.

Files with missing lines Patch % Lines
pkg/controllers/serviceexport_controller.go 33.07% 74 Missing and 11 partials ⚠️
pkg/model/plan.go 0.00% 6 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #88      +/-   ##
==========================================
- Coverage   76.30%   73.52%   -2.78%     
==========================================
  Files          14       14              
  Lines        1224     1292      +68     
==========================================
+ Hits          934      950      +16     
- Misses        226      274      +48     
- Partials       64       68       +4     
Files with missing lines Coverage Δ
pkg/model/plan.go 77.77% <0.00%> (-22.23%) ⬇️
pkg/controllers/serviceexport_controller.go 40.00% <33.07%> (-6.38%) ⬇️

Continue to review full report in Codecov by Sentry.

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

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

astaticvoid
astaticvoid previously approved these changes Nov 19, 2021
if cmService != nil {
if err := r.CloudMap.DeleteEndpoints(ctx, cmService.Namespace, cmService.Name, cmService.Endpoints); err != nil {
r.Log.Error(err, "error while deleting endpoints from Cloud Map",
"namespace", cmService.Namespace, "name", cmService.Name)
return ctrl.Result{}, err
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess in the future we'll return something other than empty struct.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now our strategy seems to be just log error, and move on. We have to probably figure out fault-tolerance strategy before GA.

@runakash runakash merged commit a23d0c7 into aws:main Nov 19, 2021
@runakash runakash deleted the sync-service-changes branch November 19, 2021 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ServiceExport controller should watch changes on Service resources
4 participants