Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specifying traffic distribution preference for the etcd client Service #973

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions api/v1alpha1/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,10 @@ 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
ialidzhikov marked this conversation as resolved.
Show resolved Hide resolved
TrafficDistribution *string `json:"trafficDistribution,omitempty"`
ialidzhikov marked this conversation as resolved.
Show resolved Hide resolved
}

// 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,11 @@ 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
type: string
type: object
clientUrlTls:
description: ClientUrlTLS contains the ca, server TLS and client
Expand Down
5 changes: 5 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,11 @@ 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
type: string
type: object
clientUrlTls:
description: ClientUrlTLS contains the ca, server TLS and client
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 | | |


#### 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