Skip to content

Conversation

fredjywang
Copy link
Contributor

@fredjywang fredjywang commented Aug 8, 2022

Related to Issue #22: Support for headless services

Description of changes:

  • Added logic in ServiceImportController for detecting ServiceType of importing Service and typing ServiceImport object accordingly (either Headless or ClusterSetIP)
  • Added unit tests for new functions related to new logic

Note: This change currently only discovers A/AAAA records; support for SRV records will be added after cluster annotations are added to EndpointSlices with Source-Cluster label PR (#179). Additionally, the logic currently assumes that all Endpoints have the same ServiceType, and therefore fetches the ServiceType from the first endpoint in the Service. This may be changed in the conflict resolution story.

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

@codecov-commenter
Copy link

codecov-commenter commented Aug 8, 2022

Codecov Report

Attention: Patch coverage is 95.45455% with 1 line in your changes missing coverage. Please review.

Project coverage is 72.84%. Comparing base (8460148) to head (1a20940).
Report is 161 commits behind head on main.

Files with missing lines Patch % Lines
...kg/controllers/multicluster/cloudmap_controller.go 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #180      +/-   ##
==========================================
+ Coverage   72.65%   72.84%   +0.19%     
==========================================
  Files          15       15              
  Lines        1558     1569      +11     
==========================================
+ Hits         1132     1143      +11     
  Misses        342      342              
  Partials       84       84              
Files with missing lines Coverage Δ
pkg/controllers/multicluster/utils.go 96.47% <100.00%> (+0.24%) ⬆️
...kg/controllers/multicluster/cloudmap_controller.go 45.23% <80.00%> (ø)

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 8460148...1a20940. Read the comment docs.

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

@@ -174,6 +174,22 @@ func CreateDerivedServiceStruct(svcImport *multiclusterv1alpha1.ServiceImport, i
svcPorts = append(svcPorts, PortToServicePort(*svcPort))
}

// if svcImport is Headless type, specify ClusterIP field to "None"
if svcImport.Spec.Type == multiclusterv1alpha1.Headless {
return &v1.Service{
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicate code. Create the struct with the shared values, then conditionally set the differences.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also missing unit test coverage.

@fredjywang fredjywang requested a review from astaticvoid August 9, 2022 17:18
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.

Lgtm, thanks for the improvements.

@fredjywang fredjywang merged commit 84f43fc into aws:main Aug 10, 2022
runakash pushed a commit to runakash/aws-cloud-map-mcs-controller-for-k8s that referenced this pull request Jun 20, 2025
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.

4 participants