Skip to content

Commit a64efd0

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #1743 from vrutkovs/minimum-two-etcd-endpoints
OCPBUGS-42083: Don't rollout revision until three etcd endpoints are listed
2 parents 3a58fd7 + 992114a commit a64efd0

File tree

3 files changed

+124
-10
lines changed

3 files changed

+124
-10
lines changed

‎pkg/operator/starter.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
267267
kubeInformersForNamespaces,
268268
kubeClient,
269269
startupmonitorreadiness.IsStartupMonitorEnabledFunction(configInformers.Config().V1().Infrastructures().Lister(), operatorClient),
270+
notOnSingleReplicaTopology,
270271
controllerContext.EventRecorder,
271272
)
272273

‎pkg/operator/targetconfigcontroller/targetconfigcontroller.go‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ type TargetConfigController struct {
4848
kubeClient kubernetes.Interface
4949
configMapLister corev1listers.ConfigMapLister
5050

51-
isStartupMonitorEnabledFn func() (bool, error)
51+
isStartupMonitorEnabledFn func() (bool, error)
52+
notOnSingleReplicaTopologyFn func() bool
5253
}
5354

5455
func NewTargetConfigController(
@@ -58,15 +59,17 @@ func NewTargetConfigController(
5859
kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces,
5960
kubeClient kubernetes.Interface,
6061
isStartupMonitorEnabledFn func() (bool, error),
62+
notOnSingleReplicaTopologyFn func() bool,
6163
eventRecorder events.Recorder,
6264
) factory.Controller {
6365
c := &TargetConfigController{
64-
targetImagePullSpec: targetImagePullSpec,
65-
operatorImagePullSpec: operatorImagePullSpec,
66-
operatorClient: operatorClient,
67-
kubeClient: kubeClient,
68-
configMapLister: kubeInformersForNamespaces.ConfigMapLister(),
69-
isStartupMonitorEnabledFn: isStartupMonitorEnabledFn,
66+
targetImagePullSpec: targetImagePullSpec,
67+
operatorImagePullSpec: operatorImagePullSpec,
68+
operatorClient: operatorClient,
69+
kubeClient: kubeClient,
70+
configMapLister: kubeInformersForNamespaces.ConfigMapLister(),
71+
isStartupMonitorEnabledFn: isStartupMonitorEnabledFn,
72+
notOnSingleReplicaTopologyFn: notOnSingleReplicaTopologyFn,
7073
}
7174

7275
return factory.New().WithInformers(
@@ -100,7 +103,8 @@ func (c TargetConfigController) sync(ctx context.Context, syncContext factory.Sy
100103
}
101104

102105
// block until config is observed and specific paths are present
103-
if err := isRequiredConfigPresent(operatorSpec.ObservedConfig.Raw); err != nil {
106+
isNotOnSingleReplicaTopology := c.notOnSingleReplicaTopologyFn()
107+
if err := isRequiredConfigPresent(operatorSpec.ObservedConfig.Raw, isNotOnSingleReplicaTopology); err != nil {
104108
syncContext.Recorder().Warning("ConfigMissing", err.Error())
105109
return err
106110
}
@@ -116,7 +120,7 @@ func (c TargetConfigController) sync(ctx context.Context, syncContext factory.Sy
116120
return nil
117121
}
118122

119-
func isRequiredConfigPresent(config []byte) error {
123+
func isRequiredConfigPresent(config []byte, isNotSingleNode bool) error {
120124
if len(config) == 0 {
121125
return fmt.Errorf("no observedConfig")
122126
}
@@ -148,6 +152,16 @@ func isRequiredConfigPresent(config []byte) error {
148152
if configValString, ok := configVal.(string); ok && len(configValString) == 0 {
149153
return fmt.Errorf("%v empty in config", strings.Join(requiredPath, "."))
150154
}
155+
156+
if len(requiredPath) == 2 && requiredPath[0] == "apiServerArguments" && requiredPath[1] == "etcd-servers" && isNotSingleNode {
157+
configValSlice, ok := configVal.([]interface{})
158+
if !ok {
159+
return fmt.Errorf("%v is not a slice", strings.Join(requiredPath, "."))
160+
}
161+
if len(configValSlice) < 3 {
162+
return fmt.Errorf("%v has less than three endpoints: %v", strings.Join(requiredPath, "."), configValSlice)
163+
}
164+
}
151165
}
152166
return nil
153167
}

‎pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go‎

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package targetconfigcontroller
22

33
import (
4+
"fmt"
45
"strings"
56
"testing"
67

@@ -110,7 +111,7 @@ func TestIsRequiredConfigPresent(t *testing.T) {
110111

111112
for _, test := range tests {
112113
t.Run(test.name, func(t *testing.T) {
113-
actual := isRequiredConfigPresent([]byte(test.config))
114+
actual := isRequiredConfigPresent([]byte(test.config), false)
114115
switch {
115116
case actual == nil && len(test.expectedError) == 0:
116117
case actual == nil && len(test.expectedError) != 0:
@@ -237,3 +238,101 @@ func TestManageTemplate(t *testing.T) {
237238
})
238239
}
239240
}
241+
242+
func TestIsRequiredConfigPresentEtcdEndpoints(t *testing.T) {
243+
configTemplate := `{
244+
"servingInfo": {
245+
"namedCertificates": [
246+
{
247+
"certFile": "/etc/kubernetes/static-pod-certs/secrets/localhost-serving-cert-certkey/tls.crt",
248+
"keyFile": "/etc/kubernetes/static-pod-certs/secrets/localhost-serving-cert-certkey/tls.key"
249+
}
250+
]
251+
},
252+
"admission": {"pluginConfig": { "network.openshift.io/RestrictedEndpointsAdmission": {}}},
253+
"apiServerArguments": {
254+
"etcd-servers": %s
255+
}
256+
}
257+
`
258+
tests := []struct {
259+
name string
260+
etcdServers string
261+
expectedError string
262+
isNotSingleNode bool
263+
}{
264+
{
265+
name: "nil-storage-urls",
266+
etcdServers: "null",
267+
expectedError: "apiServerArguments.etcd-servers null in config",
268+
},
269+
{
270+
name: "missing-storage-urls",
271+
etcdServers: "[]",
272+
expectedError: "apiServerArguments.etcd-servers empty in config",
273+
},
274+
{
275+
name: "empty-string-storage-urls",
276+
etcdServers: `""`,
277+
expectedError: "apiServerArguments.etcd-servers empty in config",
278+
},
279+
{
280+
name: "one-etcd-server",
281+
etcdServers: `[ "val" ]`,
282+
isNotSingleNode: true,
283+
expectedError: "apiServerArguments.etcd-servers has less than three endpoints",
284+
},
285+
{
286+
name: "one-etcd-server-sno",
287+
etcdServers: `[ "val" ]`,
288+
isNotSingleNode: false,
289+
},
290+
{
291+
name: "two-etcd-servers",
292+
etcdServers: `[ "val1", "val2" ]`,
293+
isNotSingleNode: true,
294+
expectedError: "apiServerArguments.etcd-servers has less than three endpoints",
295+
},
296+
{
297+
name: "two-etcd-servers-sno",
298+
etcdServers: `[ "val1", "val2" ]`,
299+
isNotSingleNode: false,
300+
},
301+
{
302+
name: "three-etcd-servers",
303+
etcdServers: `[ "val1", "val2", "val3" ]`,
304+
isNotSingleNode: true,
305+
},
306+
{
307+
name: "three-etcd-servers-sno",
308+
etcdServers: `[ "val1", "val2", "val3" ]`,
309+
isNotSingleNode: false,
310+
},
311+
{
312+
name: "four-etcd-servers",
313+
etcdServers: `[ "val1", "val2", "val3", "val4" ]`,
314+
isNotSingleNode: true,
315+
},
316+
{
317+
name: "four-etcd-servers-sno",
318+
etcdServers: `[ "val1", "val2", "val3", "val4" ]`,
319+
isNotSingleNode: false,
320+
},
321+
}
322+
323+
for _, test := range tests {
324+
t.Run(test.name, func(t *testing.T) {
325+
config := fmt.Sprintf(configTemplate, test.etcdServers)
326+
actual := isRequiredConfigPresent([]byte(config), test.isNotSingleNode)
327+
switch {
328+
case actual == nil && len(test.expectedError) == 0:
329+
case actual == nil && len(test.expectedError) != 0:
330+
t.Fatal(actual)
331+
case actual != nil && len(test.expectedError) == 0:
332+
t.Fatal(actual)
333+
case actual != nil && len(test.expectedError) != 0 && !strings.Contains(actual.Error(), test.expectedError):
334+
t.Fatal(actual)
335+
}
336+
})
337+
}
338+
}

0 commit comments

Comments
 (0)