Skip to content

Commit 62d9bb5

Browse files
ElbeheryElbehery
committed
use etcd-endpoint CM voting members
1 parent 3f289fc commit 62d9bb5

File tree

4 files changed

+29
-87
lines changed

4 files changed

+29
-87
lines changed

‎pkg/operator/ceohelpers/unsupported_override_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@ package ceohelpers
22

33
import (
44
"fmt"
5+
"testing"
6+
57
"github.com/stretchr/testify/require"
68
"k8s.io/apimachinery/pkg/runtime"
7-
"testing"
89

910
operatorv1 "github.com/openshift/api/operator/v1"
1011
)

‎pkg/operator/periodicbackupcontroller/periodicbackupcontroller.go‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ import (
88
"strings"
99
"time"
1010

11-
"github.com/openshift/cluster-etcd-operator/pkg/etcdcli"
12-
"github.com/openshift/cluster-etcd-operator/pkg/operator/ceohelpers"
1311
"k8s.io/apimachinery/pkg/util/sets"
1412

1513
"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
@@ -79,7 +77,6 @@ type PeriodicBackupController struct {
7977
backupVarGetter backuphelpers.BackupVar
8078
featureGateAccessor featuregates.FeatureGateAccess
8179
kubeInformers v1helpers.KubeInformersForNamespaces
82-
etcdClient etcdcli.EtcdClient
8380
}
8481

8582
func NewPeriodicBackupController(
@@ -92,7 +89,6 @@ func NewPeriodicBackupController(
9289
accessor featuregates.FeatureGateAccess,
9390
backupVarGetter backuphelpers.BackupVar,
9491
backupsInformer factory.Informer,
95-
etcdClient etcdcli.EtcdClient,
9692
kubeInformers v1helpers.KubeInformersForNamespaces) factory.Controller {
9793

9894
c := &PeriodicBackupController{
@@ -103,7 +99,6 @@ func NewPeriodicBackupController(
10399
operatorImagePullSpec: operatorImagePullSpec,
104100
backupVarGetter: backupVarGetter,
105101
featureGateAccessor: accessor,
106-
etcdClient: etcdClient,
107102
kubeInformers: kubeInformers,
108103
}
109104

@@ -139,10 +134,6 @@ func (c *PeriodicBackupController) sync(ctx context.Context, syncContext factory
139134
if item.Name == defaultBackupCRName {
140135
defaultFound = true
141136

142-
err = ensureVotingNodesLabeled(ctx, c.kubeClient, c.etcdClient)
143-
if err != nil {
144-
return fmt.Errorf("PeriodicBackupController could not label voting master nodes: %w", err)
145-
}
146137
currentEtcdBackupDS, err := c.kubeClient.AppsV1().DaemonSets(operatorclient.TargetNamespace).Get(ctx, backupServerDaemonSet, v1.GetOptions{})
147138
if err != nil && !apierrors.IsNotFound(err) {
148139
return fmt.Errorf("PeriodicBackupController could not retrieve [defaultBackupDeployment]: %w", err)
@@ -153,6 +144,11 @@ func (c *PeriodicBackupController) sync(ctx context.Context, syncContext factory
153144
return fmt.Errorf("PeriodicBackupController failed to list etcd-endpoints config-map: %w", err)
154145
}
155146

147+
err = ensureVotingNodesLabeled(ctx, c.kubeClient, endpoints)
148+
if err != nil {
149+
return fmt.Errorf("PeriodicBackupController could not label voting master nodes: %w", err)
150+
}
151+
156152
desiredEtcdBackupDS := createBackupServerDaemonSet(item, endpoints)
157153
if etcdBackupServerDSDiffers(desiredEtcdBackupDS.Spec, currentEtcdBackupDS.Spec) {
158154
_, opStatus, _, _ := c.operatorClient.GetOperatorState()
@@ -535,19 +531,12 @@ func etcdBackupServerDSDiffers(l, r appv1.DaemonSetSpec) bool {
535531
return false
536532
}
537533

538-
func ensureVotingNodesLabeled(ctx context.Context, client kubernetes.Interface, etcdClient etcdcli.EtcdClient) error {
539-
members, err := etcdClient.VotingMemberList(ctx)
540-
if err != nil {
541-
return fmt.Errorf("failed to list voting members: %w", err)
542-
}
543-
544-
votingMemberIPs := sets.NewString()
545-
for _, m := range members {
546-
memberIP, mErr := ceohelpers.MemberToNodeInternalIP(m)
547-
if mErr != nil {
548-
return mErr
549-
}
550-
votingMemberIPs.Insert(memberIP)
534+
func ensureVotingNodesLabeled(ctx context.Context, client kubernetes.Interface, etcdEndpoints []string) error {
535+
votingIPs := sets.NewString()
536+
for _, ep := range etcdEndpoints {
537+
splits := strings.Split(ep, ":")
538+
host := splits[0]
539+
votingIPs.Insert(host)
551540
}
552541

553542
masterNodes, err := client.CoreV1().Nodes().List(ctx, v1.ListOptions{
@@ -560,7 +549,7 @@ func ensureVotingNodesLabeled(ctx context.Context, client kubernetes.Interface,
560549
for _, node := range masterNodes.Items {
561550
for _, addr := range node.Status.Addresses {
562551
if addr.Type == corev1.NodeInternalIP {
563-
if votingMemberIPs.Has(addr.Address) {
552+
if votingIPs.Has(addr.Address) {
564553
// update node's labels
565554
node.Labels[votingNodeSelector] = ""
566555
_, err = client.CoreV1().Nodes().Update(ctx, &node, v1.UpdateOptions{})

‎pkg/operator/periodicbackupcontroller/periodicbackupcontroller_test.go‎

Lines changed: 15 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@ import (
88
"testing"
99
"time"
1010

11-
"github.com/openshift/cluster-etcd-operator/pkg/etcdcli"
12-
"go.etcd.io/etcd/api/v3/etcdserverpb"
13-
1411
testing2 "k8s.io/utils/clock/testing"
1512

1613
"github.com/openshift/library-go/pkg/controller/factory"
@@ -96,14 +93,6 @@ func TestSyncLoopWithDefaultBackupCR(t *testing.T) {
9693
&operatorv1.StaticPodOperatorSpec{OperatorSpec: operatorv1.OperatorSpec{ManagementState: operatorv1.Managed}},
9794
&operatorv1.StaticPodOperatorStatus{}, nil, nil)
9895

99-
defaultEtcdMembers := []*etcdserverpb.Member{
100-
u.FakeEtcdMemberWithoutServer(0),
101-
u.FakeEtcdMemberWithoutServer(1),
102-
u.FakeEtcdMemberWithoutServer(2),
103-
}
104-
fakeEtcdClient, err := etcdcli.NewFakeEtcdClient(defaultEtcdMembers, etcdcli.WithFakeClusterHealth(&etcdcli.FakeMemberHealth{Healthy: 3, Unhealthy: 0}))
105-
require.NoError(t, err)
106-
10796
controller := PeriodicBackupController{
10897
operatorClient: fakeOperatorClient,
10998
podLister: fakeKubeInformerForNamespace.InformersFor(operatorclient.TargetNamespace).Core().V1().Pods().Lister(),
@@ -112,7 +101,6 @@ func TestSyncLoopWithDefaultBackupCR(t *testing.T) {
112101
operatorImagePullSpec: "pullspec-image",
113102
backupVarGetter: backuphelpers.NewDisabledBackupConfig(),
114103
featureGateAccessor: backupFeatureGateAccessor,
115-
etcdClient: fakeEtcdClient,
116104
kubeInformers: fakeKubeInformerForNamespace,
117105
}
118106

@@ -129,7 +117,7 @@ func TestSyncLoopWithDefaultBackupCR(t *testing.T) {
129117
fakeKubeInformerForNamespace.Start(stopChan)
130118

131119
expDisabledBackupVar := " args:\n - --enabled=false"
132-
err = controller.sync(context.TODO(), syncCtx)
120+
err := controller.sync(context.TODO(), syncCtx)
133121
require.NoError(t, err)
134122
require.Equal(t, expDisabledBackupVar, controller.backupVarGetter.ArgString())
135123

@@ -193,14 +181,6 @@ func TestSyncLoopWithDefaultBackupCRWithoutRetentionSpec(t *testing.T) {
193181
&operatorv1.StaticPodOperatorSpec{OperatorSpec: operatorv1.OperatorSpec{ManagementState: operatorv1.Managed}},
194182
&operatorv1.StaticPodOperatorStatus{}, nil, nil)
195183

196-
defaultEtcdMembers := []*etcdserverpb.Member{
197-
u.FakeEtcdMemberWithoutServer(0),
198-
u.FakeEtcdMemberWithoutServer(1),
199-
u.FakeEtcdMemberWithoutServer(2),
200-
}
201-
fakeEtcdClient, err := etcdcli.NewFakeEtcdClient(defaultEtcdMembers, etcdcli.WithFakeClusterHealth(&etcdcli.FakeMemberHealth{Healthy: 3, Unhealthy: 0}))
202-
require.NoError(t, err)
203-
204184
controller := PeriodicBackupController{
205185
operatorClient: fakeOperatorClient,
206186
podLister: fakeKubeInformerForNamespace.InformersFor(operatorclient.TargetNamespace).Core().V1().Pods().Lister(),
@@ -209,7 +189,6 @@ func TestSyncLoopWithDefaultBackupCRWithoutRetentionSpec(t *testing.T) {
209189
operatorImagePullSpec: "pullspec-image",
210190
backupVarGetter: backuphelpers.NewDisabledBackupConfig(),
211191
featureGateAccessor: backupFeatureGateAccessor,
212-
etcdClient: fakeEtcdClient,
213192
kubeInformers: fakeKubeInformerForNamespace,
214193
}
215194

@@ -226,7 +205,7 @@ func TestSyncLoopWithDefaultBackupCRWithoutRetentionSpec(t *testing.T) {
226205
fakeKubeInformerForNamespace.Start(stopChan)
227206

228207
expDisabledBackupVar := " args:\n - --enabled=false"
229-
err = controller.sync(context.TODO(), syncCtx)
208+
err := controller.sync(context.TODO(), syncCtx)
230209
require.NoError(t, err)
231210
require.Equal(t, expDisabledBackupVar, controller.backupVarGetter.ArgString())
232211

@@ -290,14 +269,6 @@ func TestSyncLoopWithDefaultBackupCRWithoutScheduleSpec(t *testing.T) {
290269
&operatorv1.StaticPodOperatorSpec{OperatorSpec: operatorv1.OperatorSpec{ManagementState: operatorv1.Managed}},
291270
&operatorv1.StaticPodOperatorStatus{}, nil, nil)
292271

293-
defaultEtcdMembers := []*etcdserverpb.Member{
294-
u.FakeEtcdMemberWithoutServer(0),
295-
u.FakeEtcdMemberWithoutServer(1),
296-
u.FakeEtcdMemberWithoutServer(2),
297-
}
298-
fakeEtcdClient, err := etcdcli.NewFakeEtcdClient(defaultEtcdMembers, etcdcli.WithFakeClusterHealth(&etcdcli.FakeMemberHealth{Healthy: 3, Unhealthy: 0}))
299-
require.NoError(t, err)
300-
301272
controller := PeriodicBackupController{
302273
operatorClient: fakeOperatorClient,
303274
podLister: fakeKubeInformerForNamespace.InformersFor(operatorclient.TargetNamespace).Core().V1().Pods().Lister(),
@@ -306,7 +277,6 @@ func TestSyncLoopWithDefaultBackupCRWithoutScheduleSpec(t *testing.T) {
306277
operatorImagePullSpec: "pullspec-image",
307278
backupVarGetter: backuphelpers.NewDisabledBackupConfig(),
308279
featureGateAccessor: backupFeatureGateAccessor,
309-
etcdClient: fakeEtcdClient,
310280
kubeInformers: fakeKubeInformerForNamespace,
311281
}
312282

@@ -323,7 +293,7 @@ func TestSyncLoopWithDefaultBackupCRWithoutScheduleSpec(t *testing.T) {
323293
fakeKubeInformerForNamespace.Start(stopChan)
324294

325295
expDisabledBackupVar := " args:\n - --enabled=false"
326-
err = controller.sync(context.TODO(), syncCtx)
296+
err := controller.sync(context.TODO(), syncCtx)
327297
require.NoError(t, err)
328298
require.Equal(t, expDisabledBackupVar, controller.backupVarGetter.ArgString())
329299

@@ -388,14 +358,6 @@ func TestSyncLoopWithDefaultBackupCREditSpec(t *testing.T) {
388358
&operatorv1.StaticPodOperatorSpec{OperatorSpec: operatorv1.OperatorSpec{ManagementState: operatorv1.Managed}},
389359
&operatorv1.StaticPodOperatorStatus{}, nil, nil)
390360

391-
defaultEtcdMembers := []*etcdserverpb.Member{
392-
u.FakeEtcdMemberWithoutServer(0),
393-
u.FakeEtcdMemberWithoutServer(1),
394-
u.FakeEtcdMemberWithoutServer(2),
395-
}
396-
fakeEtcdClient, err := etcdcli.NewFakeEtcdClient(defaultEtcdMembers, etcdcli.WithFakeClusterHealth(&etcdcli.FakeMemberHealth{Healthy: 3, Unhealthy: 0}))
397-
require.NoError(t, err)
398-
399361
controller := PeriodicBackupController{
400362
operatorClient: fakeOperatorClient,
401363
podLister: fakeKubeInformerForNamespace.InformersFor(operatorclient.TargetNamespace).Core().V1().Pods().Lister(),
@@ -404,7 +366,6 @@ func TestSyncLoopWithDefaultBackupCREditSpec(t *testing.T) {
404366
operatorImagePullSpec: "pullspec-image",
405367
backupVarGetter: backuphelpers.NewDisabledBackupConfig(),
406368
featureGateAccessor: backupFeatureGateAccessor,
407-
etcdClient: fakeEtcdClient,
408369
kubeInformers: fakeKubeInformerForNamespace,
409370
}
410371

@@ -421,7 +382,7 @@ func TestSyncLoopWithDefaultBackupCREditSpec(t *testing.T) {
421382
fakeKubeInformerForNamespace.Start(stopChan)
422383

423384
expDisabledBackupVar := " args:\n - --enabled=false"
424-
err = controller.sync(context.TODO(), syncCtx)
385+
err := controller.sync(context.TODO(), syncCtx)
425386
require.NoError(t, err)
426387
require.Equal(t, expDisabledBackupVar, controller.backupVarGetter.ArgString())
427388

@@ -512,14 +473,6 @@ func TestSyncLoopFailsDegradesOperatorWithDefaultBackupCR(t *testing.T) {
512473
backups.Items = append(backups.Items, backup)
513474
operatorFake := fake.NewClientset([]runtime.Object{&backups}...)
514475

515-
defaultEtcdMembers := []*etcdserverpb.Member{
516-
u.FakeEtcdMemberWithoutServer(0),
517-
u.FakeEtcdMemberWithoutServer(1),
518-
u.FakeEtcdMemberWithoutServer(2),
519-
}
520-
fakeEtcdClient, err := etcdcli.NewFakeEtcdClient(defaultEtcdMembers, etcdcli.WithFakeClusterHealth(&etcdcli.FakeMemberHealth{Healthy: 3, Unhealthy: 0}))
521-
require.NoError(t, err)
522-
523476
controller := PeriodicBackupController{
524477
operatorClient: fakeOperatorClient,
525478
podLister: fakeKubeInformerForNamespace.InformersFor(operatorclient.TargetNamespace).Core().V1().Pods().Lister(),
@@ -528,7 +481,6 @@ func TestSyncLoopFailsDegradesOperatorWithDefaultBackupCR(t *testing.T) {
528481
operatorImagePullSpec: "pullspec-image",
529482
backupVarGetter: backuphelpers.NewDisabledBackupConfig(),
530483
featureGateAccessor: backupFeatureGateAccessor,
531-
etcdClient: fakeEtcdClient,
532484
kubeInformers: fakeKubeInformerForNamespace,
533485
}
534486

@@ -545,7 +497,7 @@ func TestSyncLoopFailsDegradesOperatorWithDefaultBackupCR(t *testing.T) {
545497
fakeKubeInformerForNamespace.Start(stopChan)
546498

547499
expDisabledBackupVar := " args:\n - --enabled=false"
548-
err = controller.sync(context.TODO(), syncCtx)
500+
err := controller.sync(context.TODO(), syncCtx)
549501
require.NoError(t, err)
550502
require.Equal(t, expDisabledBackupVar, controller.backupVarGetter.ArgString())
551503
requireOperatorStatus(t, fakeOperatorClient, false)
@@ -852,23 +804,24 @@ func extractEtcdBackupServerArgVal(t testing.TB, argName string, args []string)
852804

853805
func TestEnsureVotingNodesLabeled(t *testing.T) {
854806
// arrange
855-
defaultEtcdMembers := []*etcdserverpb.Member{
856-
u.FakeEtcdMemberWithoutServer(0),
857-
u.FakeEtcdMemberWithoutServer(1),
858-
u.FakeEtcdMemberWithoutServer(2),
859-
}
860-
fakeEtcdClient, err := etcdcli.NewFakeEtcdClient(defaultEtcdMembers, etcdcli.WithFakeClusterHealth(&etcdcli.FakeMemberHealth{Healthy: 3, Unhealthy: 0}))
861-
require.NoError(t, err)
862-
863807
allClusterNodes := defaultClusterNodes()
864808
var objects []runtime.Object
865809
for _, n := range allClusterNodes {
866810
objects = append(objects, n)
867811
}
812+
813+
endpointCM := u.EndpointsConfigMap(
814+
u.WithEndpoint(1, "10.0.0.1"+etcdClientPort),
815+
u.WithEndpoint(2, "10.0.0.2"+etcdClientPort),
816+
u.WithEndpoint(3, "10.0.0.3"+etcdClientPort),
817+
)
818+
objects = append(objects, endpointCM)
868819
client := k8sfakeclient.NewClientset(objects...)
869820

870821
// act
871-
err = ensureVotingNodesLabeled(context.TODO(), client, fakeEtcdClient)
822+
endpoints, err := getEtcdEndpoints(context.TODO(), client)
823+
require.NoError(t, err)
824+
err = ensureVotingNodesLabeled(context.TODO(), client, endpoints)
872825
require.NoError(t, err)
873826

874827
// assert

‎pkg/operator/starter.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,6 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
493493
featureGateAccessor,
494494
backuphelpers.NewDisabledBackupConfig(),
495495
configBackupInformer,
496-
etcdClient,
497496
kubeInformersForNamespaces)
498497

499498
backupController := backupcontroller.NewBackupController(

0 commit comments

Comments
 (0)