From 0cfc04c164ec53ea9ecfb32102cea58d768264dd Mon Sep 17 00:00:00 2001
From: Ismail Alidzhikov <9372594+ialidzhikov@users.noreply.github.com>
Date: Wed, 29 Jan 2025 12:27:03 +0200
Subject: [PATCH] Allow specifying traffic distribution preference for the etcd
client Service (#973)
* Allow specifying traffic distribution preference for the etcd client Service
---
api/v1alpha1/etcd.go | 5 ++
api/v1alpha1/zz_generated.deepcopy.go | 5 ++
.../crd-druid.gardener.cloud_etcds.yaml | 7 +++
.../bases/crd-druid.gardener.cloud_etcds.yaml | 7 +++
config/samples/druid_v1alpha1_etcd.yaml | 6 ++
.../samples/druid_v1alpha1_etcd_azurite.yaml | 6 ++
.../samples/druid_v1alpha1_etcd_fakegcs.yaml | 6 ++
.../druid_v1alpha1_etcd_localstack.yaml | 6 ++
docs/api-reference/etcd-druid-api.md | 1 +
.../component/clientservice/clientservice.go | 8 +++
.../clientservice/clientservice_test.go | 55 ++++++++++++-------
test/utils/etcd.go | 13 +++++
12 files changed, 106 insertions(+), 19 deletions(-)
diff --git a/api/v1alpha1/etcd.go b/api/v1alpha1/etcd.go
index 8d311e95e..a5beb8bd9 100644
--- a/api/v1alpha1/etcd.go
+++ b/api/v1alpha1/etcd.go
@@ -243,6 +243,11 @@ type ClientService struct {
// Labels specify the labels that should be added to the client service
// +optional
Labels map[string]string `json:"labels,omitempty"`
+ // TrafficDistribution defines the traffic distribution preference that should be added to the client service.
+ // More info: https://kubernetes.io/docs/reference/networking/virtual-ips/#traffic-distribution
+ // +optional
+ // +kubebuilder:validation:Enum=PreferClose
+ TrafficDistribution *string `json:"trafficDistribution,omitempty"`
}
// SharedConfig defines parameters shared and used by Etcd as well as backup-restore sidecar.
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
index bb057b96b..2c690ebe3 100644
--- a/api/v1alpha1/zz_generated.deepcopy.go
+++ b/api/v1alpha1/zz_generated.deepcopy.go
@@ -131,6 +131,11 @@ func (in *ClientService) DeepCopyInto(out *ClientService) {
(*out)[key] = val
}
}
+ if in.TrafficDistribution != nil {
+ in, out := &in.TrafficDistribution, &out.TrafficDistribution
+ *out = new(string)
+ **out = **in
+ }
}
// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClientService.
diff --git a/charts/druid/charts/crds/templates/crd-druid.gardener.cloud_etcds.yaml b/charts/druid/charts/crds/templates/crd-druid.gardener.cloud_etcds.yaml
index 11cbb5b03..b0be5b864 100644
--- a/charts/druid/charts/crds/templates/crd-druid.gardener.cloud_etcds.yaml
+++ b/charts/druid/charts/crds/templates/crd-druid.gardener.cloud_etcds.yaml
@@ -404,6 +404,13 @@ spec:
description: Labels specify the labels that should be added
to the client service
type: object
+ trafficDistribution:
+ description: |-
+ TrafficDistribution defines the traffic distribution preference that should be added to the client service.
+ More info: https://kubernetes.io/docs/reference/networking/virtual-ips/#traffic-distribution
+ enum:
+ - PreferClose
+ type: string
type: object
clientUrlTls:
description: ClientUrlTLS contains the ca, server TLS and client
diff --git a/config/crd/bases/crd-druid.gardener.cloud_etcds.yaml b/config/crd/bases/crd-druid.gardener.cloud_etcds.yaml
index 11cbb5b03..b0be5b864 100644
--- a/config/crd/bases/crd-druid.gardener.cloud_etcds.yaml
+++ b/config/crd/bases/crd-druid.gardener.cloud_etcds.yaml
@@ -404,6 +404,13 @@ spec:
description: Labels specify the labels that should be added
to the client service
type: object
+ trafficDistribution:
+ description: |-
+ TrafficDistribution defines the traffic distribution preference that should be added to the client service.
+ More info: https://kubernetes.io/docs/reference/networking/virtual-ips/#traffic-distribution
+ enum:
+ - PreferClose
+ type: string
type: object
clientUrlTls:
description: ClientUrlTLS contains the ca, server TLS and client
diff --git a/config/samples/druid_v1alpha1_etcd.yaml b/config/samples/druid_v1alpha1_etcd.yaml
index e945283ee..b6837df30 100644
--- a/config/samples/druid_v1alpha1_etcd.yaml
+++ b/config/samples/druid_v1alpha1_etcd.yaml
@@ -36,6 +36,12 @@ spec:
serverPort: 2380
quota: 8Gi
# heartbeatDuration: 10s
+ # clientService:
+ # annotations:
+ # key: value
+ # labels:
+ # key: value
+ # trafficDistribution: PreferClose
backup:
port: 8080
fullSnapshotSchedule: "0 */24 * * *"
diff --git a/config/samples/druid_v1alpha1_etcd_azurite.yaml b/config/samples/druid_v1alpha1_etcd_azurite.yaml
index 20437f830..589b689dc 100644
--- a/config/samples/druid_v1alpha1_etcd_azurite.yaml
+++ b/config/samples/druid_v1alpha1_etcd_azurite.yaml
@@ -36,6 +36,12 @@ spec:
serverPort: 2380
quota: 8Gi
# heartbeatDuration: 10s
+ # clientService:
+ # annotations:
+ # key: value
+ # labels:
+ # key: value
+ # trafficDistribution: PreferClose
backup:
port: 8080
fullSnapshotSchedule: "0 */24 * * *"
diff --git a/config/samples/druid_v1alpha1_etcd_fakegcs.yaml b/config/samples/druid_v1alpha1_etcd_fakegcs.yaml
index 2508f897d..431abdaaf 100644
--- a/config/samples/druid_v1alpha1_etcd_fakegcs.yaml
+++ b/config/samples/druid_v1alpha1_etcd_fakegcs.yaml
@@ -36,6 +36,12 @@ spec:
serverPort: 2380
quota: 8Gi
# heartbeatDuration: 10s
+ # clientService:
+ # annotations:
+ # key: value
+ # labels:
+ # key: value
+ # trafficDistribution: PreferClose
backup:
port: 8080
fullSnapshotSchedule: "0 */24 * * *"
diff --git a/config/samples/druid_v1alpha1_etcd_localstack.yaml b/config/samples/druid_v1alpha1_etcd_localstack.yaml
index 4f51553ee..2174576a0 100644
--- a/config/samples/druid_v1alpha1_etcd_localstack.yaml
+++ b/config/samples/druid_v1alpha1_etcd_localstack.yaml
@@ -36,6 +36,12 @@ spec:
serverPort: 2380
quota: 8Gi
# heartbeatDuration: 10s
+ # clientService:
+ # annotations:
+ # key: value
+ # labels:
+ # key: value
+ # trafficDistribution: PreferClose
backup:
port: 8080
fullSnapshotSchedule: "0 */24 * * *"
diff --git a/docs/api-reference/etcd-druid-api.md b/docs/api-reference/etcd-druid-api.md
index 022b0affa..d90379445 100644
--- a/docs/api-reference/etcd-druid-api.md
+++ b/docs/api-reference/etcd-druid-api.md
@@ -61,6 +61,7 @@ _Appears in:_
| --- | --- | --- | --- |
| `annotations` _object (keys:string, values:string)_ | Annotations specify the annotations that should be added to the client service | | |
| `labels` _object (keys:string, values:string)_ | Labels specify the labels that should be added to the client service | | |
+| `trafficDistribution` _string_ | TrafficDistribution defines the traffic distribution preference that should be added to the client service.
More info: https://kubernetes.io/docs/reference/networking/virtual-ips/#traffic-distribution | | Enum: [PreferClose]
|
#### CompactionMode
diff --git a/internal/component/clientservice/clientservice.go b/internal/component/clientservice/clientservice.go
index ccb95bdad..a2d126ad7 100644
--- a/internal/component/clientservice/clientservice.go
+++ b/internal/component/clientservice/clientservice.go
@@ -115,6 +115,7 @@ func buildResource(etcd *druidv1alpha1.Etcd, svc *corev1.Service) {
// Therefore, only using default labels as label selector can cause issues as we have already seen in https://github.com/gardener/etcd-druid/issues/914
svc.Spec.Selector = utils.MergeMaps(druidv1alpha1.GetDefaultLabels(etcd.ObjectMeta), map[string]string{druidv1alpha1.LabelComponentKey: common.ComponentNameStatefulSet})
svc.Spec.Ports = getPorts(etcd)
+ svc.Spec.TrafficDistribution = getTrafficDistribution(etcd)
}
func getObjectKey(obj metav1.ObjectMeta) client.ObjectKey {
@@ -144,6 +145,13 @@ func getAnnotations(etcd *druidv1alpha1.Etcd) map[string]string {
return nil
}
+func getTrafficDistribution(etcd *druidv1alpha1.Etcd) *string {
+ if etcd.Spec.Etcd.ClientService != nil {
+ return etcd.Spec.Etcd.ClientService.TrafficDistribution
+ }
+ return nil
+}
+
func emptyClientService(objectKey client.ObjectKey) *corev1.Service {
return &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
diff --git a/internal/component/clientservice/clientservice_test.go b/internal/component/clientservice/clientservice_test.go
index ed05d1522..59ca8a109 100644
--- a/internal/component/clientservice/clientservice_test.go
+++ b/internal/component/clientservice/clientservice_test.go
@@ -84,12 +84,13 @@ func TestGetExistingResourceNames(t *testing.T) {
// ----------------------------------- Sync -----------------------------------
func TestSyncWhenNoServiceExists(t *testing.T) {
testCases := []struct {
- name string
- clientPort *int32
- backupPort *int32
- peerPort *int32
- createErr *apierrors.StatusError
- expectedErr *druiderr.DruidError
+ name string
+ clientPort *int32
+ backupPort *int32
+ peerPort *int32
+ trafficDistribution *string
+ createErr *apierrors.StatusError
+ expectedErr *druiderr.DruidError
}{
{
name: "create client service with default ports",
@@ -100,6 +101,10 @@ func TestSyncWhenNoServiceExists(t *testing.T) {
backupPort: ptr.To[int32](3333),
peerPort: ptr.To[int32](4444),
},
+ {
+ name: "create client service with traffic distribution",
+ trafficDistribution: ptr.To("PreferClose"),
+ },
{
name: "create fails when there is a create error",
createErr: testutils.TestAPIInternalErr,
@@ -115,7 +120,7 @@ func TestSyncWhenNoServiceExists(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
- etcd := buildEtcd(tc.clientPort, tc.peerPort, tc.backupPort)
+ etcd := buildEtcd(tc.clientPort, tc.peerPort, tc.backupPort, tc.trafficDistribution)
cl := testutils.CreateTestFakeClientForObjects(nil, tc.createErr, nil, nil, nil, client.ObjectKey{Name: druidv1alpha1.GetClientServiceName(etcd.ObjectMeta), Namespace: etcd.Namespace})
operator := New(cl)
opCtx := component.NewOperatorContext(context.Background(), logr.Discard(), uuid.NewString())
@@ -134,24 +139,30 @@ func TestSyncWhenNoServiceExists(t *testing.T) {
func TestSyncWhenServiceExists(t *testing.T) {
const (
- originalClientPort int32 = 2379
- originalServerPort int32 = 2380
- originalBackupPort int32 = 8080
+ originalClientPort int32 = 2379
+ originalServerPort int32 = 2380
+ originalBackupPort int32 = 8080
+ originalTrafficDistribution string = "PreferClose"
)
- existingEtcd := buildEtcd(ptr.To(originalClientPort), ptr.To(originalServerPort), ptr.To(originalBackupPort))
+ existingEtcd := buildEtcd(ptr.To(originalClientPort), ptr.To(originalServerPort), ptr.To(originalBackupPort), ptr.To(originalTrafficDistribution))
testCases := []struct {
- name string
- clientPort *int32
- backupPort *int32
- peerPort *int32
- patchErr *apierrors.StatusError
- expectedError *druiderr.DruidError
+ name string
+ clientPort *int32
+ backupPort *int32
+ peerPort *int32
+ trafficDistribution *string
+ patchErr *apierrors.StatusError
+ expectedError *druiderr.DruidError
}{
{
name: "update peer service with new server port",
clientPort: ptr.To[int32](2222),
peerPort: ptr.To[int32](3333),
},
+ {
+ name: "update client service with new traffic distribution",
+ trafficDistribution: ptr.To("Foo"),
+ },
{
name: "update fails when there is a patch error",
clientPort: ptr.To[int32](2222),
@@ -173,7 +184,7 @@ func TestSyncWhenServiceExists(t *testing.T) {
// ********************* test sync with updated ports *********************
operator := New(cl)
opCtx := component.NewOperatorContext(context.Background(), logr.Discard(), uuid.NewString())
- updatedEtcd := buildEtcd(tc.clientPort, tc.peerPort, tc.backupPort)
+ updatedEtcd := buildEtcd(tc.clientPort, tc.peerPort, tc.backupPort, tc.trafficDistribution)
syncErr := operator.Sync(opCtx, updatedEtcd)
latestClientSvc, getErr := getLatestClientService(cl, updatedEtcd)
g.Expect(latestClientSvc).ToNot(BeNil())
@@ -247,7 +258,7 @@ func TestTriggerDelete(t *testing.T) {
}
// ---------------------------- Helper Functions -----------------------------
-func buildEtcd(clientPort, peerPort, backupPort *int32) *druidv1alpha1.Etcd {
+func buildEtcd(clientPort, peerPort, backupPort *int32, trafficDistribution *string) *druidv1alpha1.Etcd {
etcdBuilder := testutils.EtcdBuilderWithDefaults(testutils.TestEtcdName, testutils.TestNamespace)
if clientPort != nil {
etcdBuilder.WithEtcdClientPort(clientPort)
@@ -258,6 +269,9 @@ func buildEtcd(clientPort, peerPort, backupPort *int32) *druidv1alpha1.Etcd {
if backupPort != nil {
etcdBuilder.WithBackupPort(backupPort)
}
+ if trafficDistribution != nil {
+ etcdBuilder.WithEtcdClientServiceTrafficDistribution(trafficDistribution)
+ }
return etcdBuilder.Build()
}
@@ -268,9 +282,11 @@ func matchClientService(g *WithT, etcd *druidv1alpha1.Etcd, actualSvc corev1.Ser
etcdObjMeta := etcd.ObjectMeta
expectedLabels := druidv1alpha1.GetDefaultLabels(etcdObjMeta)
var expectedAnnotations map[string]string
+ var expectedTrafficDistribution *string
if etcd.Spec.Etcd.ClientService != nil {
expectedAnnotations = etcd.Spec.Etcd.ClientService.Annotations
expectedLabels = utils.MergeMaps(etcd.Spec.Etcd.ClientService.Labels, druidv1alpha1.GetDefaultLabels(etcdObjMeta))
+ expectedTrafficDistribution = etcd.Spec.Etcd.ClientService.TrafficDistribution
}
expectedLabelSelector := utils.MergeMaps(druidv1alpha1.GetDefaultLabels(etcdObjMeta), map[string]string{druidv1alpha1.LabelComponentKey: common.ComponentNameStatefulSet})
@@ -306,6 +322,7 @@ func matchClientService(g *WithT, etcd *druidv1alpha1.Etcd, actualSvc corev1.Ser
TargetPort: intstr.FromInt(int(backupPort)),
}),
),
+ "TrafficDistribution": Equal(expectedTrafficDistribution),
}),
}))
}
diff --git a/test/utils/etcd.go b/test/utils/etcd.go
index b2c2658f6..179301b21 100644
--- a/test/utils/etcd.go
+++ b/test/utils/etcd.go
@@ -278,6 +278,19 @@ func (eb *EtcdBuilder) WithEtcdClientServiceAnnotations(annotations map[string]s
return eb
}
+func (eb *EtcdBuilder) WithEtcdClientServiceTrafficDistribution(trafficDistribution *string) *EtcdBuilder {
+ if eb == nil || eb.etcd == nil {
+ return nil
+ }
+
+ if eb.etcd.Spec.Etcd.ClientService == nil {
+ eb.etcd.Spec.Etcd.ClientService = &druidv1alpha1.ClientService{}
+ }
+
+ eb.etcd.Spec.Etcd.ClientService.TrafficDistribution = trafficDistribution
+ return eb
+}
+
func (eb *EtcdBuilder) WithEtcdServerPort(serverPort *int32) *EtcdBuilder {
if eb == nil || eb.etcd == nil {
return nil