Skip to content

Commit

Permalink
Allow specifying traffic distribution preference for the etcd client …
Browse files Browse the repository at this point in the history
…Service (#973)

* Allow specifying traffic distribution preference for the etcd client Service
  • Loading branch information
ialidzhikov authored Jan 29, 2025
1 parent 90cc1ab commit 0cfc04c
Show file tree
Hide file tree
Showing 12 changed files with 106 additions and 19 deletions.
5 changes: 5 additions & 0 deletions api/v1alpha1/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

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 @@ -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
Expand Down
7 changes: 7 additions & 0 deletions config/crd/bases/crd-druid.gardener.cloud_etcds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions config/samples/druid_v1alpha1_etcd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 * * *"
Expand Down
6 changes: 6 additions & 0 deletions config/samples/druid_v1alpha1_etcd_azurite.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 * * *"
Expand Down
6 changes: 6 additions & 0 deletions config/samples/druid_v1alpha1_etcd_fakegcs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 * * *"
Expand Down
6 changes: 6 additions & 0 deletions config/samples/druid_v1alpha1_etcd_localstack.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 * * *"
Expand Down
1 change: 1 addition & 0 deletions docs/api-reference/etcd-druid-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.<br />More info: https://kubernetes.io/docs/reference/networking/virtual-ips/#traffic-distribution | | Enum: [PreferClose] <br /> |


#### CompactionMode
Expand Down
8 changes: 8 additions & 0 deletions internal/component/clientservice/clientservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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{
Expand Down
55 changes: 36 additions & 19 deletions internal/component/clientservice/clientservice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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,
Expand All @@ -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())
Expand All @@ -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),
Expand All @@ -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())
Expand Down Expand Up @@ -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)
Expand All @@ -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()
}

Expand All @@ -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})

Expand Down Expand Up @@ -306,6 +322,7 @@ func matchClientService(g *WithT, etcd *druidv1alpha1.Etcd, actualSvc corev1.Ser
TargetPort: intstr.FromInt(int(backupPort)),
}),
),
"TrafficDistribution": Equal(expectedTrafficDistribution),
}),
}))
}
Expand Down
13 changes: 13 additions & 0 deletions test/utils/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 0cfc04c

Please sign in to comment.