Skip to content

Commit

Permalink
[MINOR] feat(operator): Add support to pass annotations to coordinato…
Browse files Browse the repository at this point in the history
…r nodeport/headless service (#2187)

### What changes were proposed in this pull request?

Add support to pass a list of annotations per coordinator service (node port/headless).  

### Why are the changes needed?
Some tools use service annotations to create DNS records and currently is not supported.

### Does this PR introduce _any_ user-facing change?
Introduce 2 new optional fields in CRD: 
1. headlessServiceAnnotations
2. nodePortServiceAnnotations

### How was this patch tested?
UT + manually
  • Loading branch information
shlomitubul authored Oct 21, 2024
1 parent f9b4c0e commit 28fd03a
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,14 @@ type CoordinatorConfig struct {
// HTTPNodePort defines http port of node port service used for coordinators' external access.
// +optional
HTTPNodePort []int32 `json:"httpNodePort,omitempty"`

// NodePortServiceAnnotations is a list of annotations for the NodePort service.
// +optional
NodePortServiceAnnotations []map[string]string `json:"nodePortServiceAnnotations,omitempty"`

// HeadlessServiceAnnotations is a list of annotations for the headless service.
// +optional
HeadlessServiceAnnotations []map[string]string `json:"headlessServiceAnnotations,omitempty"`
}

// ShuffleServerConfig records configuration used to generate workload of shuffle servers.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -1786,6 +1786,14 @@ spec:
description: ExcludeNodesFilePath indicates exclude nodes file
path in coordinators' containers.
type: string
headlessServiceAnnotations:
description: HeadlessServiceAnnotations is a list of annotations
for the headless service.
items:
additionalProperties:
type: string
type: object
type: array
hostNetwork:
default: true
description: HostNetwork indicates whether we need to enable host
Expand Down Expand Up @@ -1827,6 +1835,14 @@ spec:
description: LogHostPath represents host path used to save logs
of shuffle servers.
type: string
nodePortServiceAnnotations:
description: NodePortServiceAnnotations is a list of annotations
for the NodePort service.
items:
additionalProperties:
type: string
type: object
type: array
nodeSelector:
additionalProperties:
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,19 @@ func GenerateHeadlessSvc(rss *unifflev1alpha1.RemoteShuffleService, index int) *
name := GenerateNameByIndex(rss, index)
serviceName := appendHeadless(name)

annotations := map[string]string{}

if len(rss.Spec.Coordinator.HeadlessServiceAnnotations) > index {
for key, value := range rss.Spec.Coordinator.HeadlessServiceAnnotations[index] {
annotations[key] = value
}
}

svc := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: serviceName,
Namespace: rss.Namespace,
Name: serviceName,
Namespace: rss.Namespace,
Annotations: annotations,
},
Spec: corev1.ServiceSpec{
ClusterIP: corev1.ClusterIPNone,
Expand Down Expand Up @@ -140,10 +149,20 @@ func GenerateHeadlessSvc(rss *unifflev1alpha1.RemoteShuffleService, index int) *
// this function is skipped.
func GenerateSvc(rss *unifflev1alpha1.RemoteShuffleService, index int) *corev1.Service {
name := GenerateNameByIndex(rss, index)

annotations := map[string]string{}

if len(rss.Spec.Coordinator.NodePortServiceAnnotations) > index {
for key, value := range rss.Spec.Coordinator.NodePortServiceAnnotations[index] {
annotations[key] = value
}
}

svc := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: rss.Namespace,
Name: name,
Namespace: rss.Namespace,
Annotations: annotations,
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeNodePort,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,24 @@ var (
"key1": "value1",
"key2": "value2",
}

testSvcAnnotationsList = []map[string]string{
{
"annotation1": "value1",
},
{
"annotation2": "value2",
},
}
)

func buildRssWithSvcAnnotations() *uniffleapi.RemoteShuffleService {
rss := utils.BuildRSSWithDefaultValue()
rss.Spec.Coordinator.NodePortServiceAnnotations = testSvcAnnotationsList
rss.Spec.Coordinator.HeadlessServiceAnnotations = testSvcAnnotationsList
return rss
}

func buildRssWithLabels() *uniffleapi.RemoteShuffleService {
rss := utils.BuildRSSWithDefaultValue()
rss.Spec.Coordinator.Labels = testLabels
Expand Down Expand Up @@ -546,6 +562,35 @@ func TestGenerateSvcForCoordinator(t *testing.T) {
}
}

func TestGenerateSvcWithAnnotationsForCoordinator(t *testing.T) {
for _, tt := range []struct {
name string
rss *uniffleapi.RemoteShuffleService
expectedAnnotations []map[string]string
}{
{
name: "nodeport and headless services with annotations",
rss: buildRssWithSvcAnnotations(),
expectedAnnotations: []map[string]string{
{"annotation1": "value1"},
{"annotation1": "value1"},
{"annotation2": "value2"},
{"annotation2": "value2"}},
},
} {
t.Run(tt.name, func(tc *testing.T) {
_, _, services, _ := GenerateCoordinators(tt.rss)

for i, svc := range services {
match := reflect.DeepEqual(tt.expectedAnnotations[i], svc.Annotations)
if !match {
tc.Errorf("unexpected annotations: %v, expected: %v", svc.Annotations, tt.expectedAnnotations[i])
}
}
})
}
}

func TestGenerateAddresses(t *testing.T) {
assertion := assert.New(t)
rss := buildRssWithLabels()
Expand Down

0 comments on commit 28fd03a

Please sign in to comment.