diff --git a/pkg/operator/starter.go b/pkg/operator/starter.go index 761368084d..ae41cb56b8 100644 --- a/pkg/operator/starter.go +++ b/pkg/operator/starter.go @@ -267,6 +267,7 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle kubeInformersForNamespaces, kubeClient, startupmonitorreadiness.IsStartupMonitorEnabledFunction(configInformers.Config().V1().Infrastructures().Lister(), operatorClient), + notOnSingleReplicaTopology, controllerContext.EventRecorder, ) diff --git a/pkg/operator/targetconfigcontroller/targetconfigcontroller.go b/pkg/operator/targetconfigcontroller/targetconfigcontroller.go index d2b2fcae47..d8745fd10f 100644 --- a/pkg/operator/targetconfigcontroller/targetconfigcontroller.go +++ b/pkg/operator/targetconfigcontroller/targetconfigcontroller.go @@ -48,7 +48,8 @@ type TargetConfigController struct { kubeClient kubernetes.Interface configMapLister corev1listers.ConfigMapLister - isStartupMonitorEnabledFn func() (bool, error) + isStartupMonitorEnabledFn func() (bool, error) + notOnSingleReplicaTopologyFn func() bool } func NewTargetConfigController( @@ -58,15 +59,17 @@ func NewTargetConfigController( kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces, kubeClient kubernetes.Interface, isStartupMonitorEnabledFn func() (bool, error), + notOnSingleReplicaTopologyFn func() bool, eventRecorder events.Recorder, ) factory.Controller { c := &TargetConfigController{ - targetImagePullSpec: targetImagePullSpec, - operatorImagePullSpec: operatorImagePullSpec, - operatorClient: operatorClient, - kubeClient: kubeClient, - configMapLister: kubeInformersForNamespaces.ConfigMapLister(), - isStartupMonitorEnabledFn: isStartupMonitorEnabledFn, + targetImagePullSpec: targetImagePullSpec, + operatorImagePullSpec: operatorImagePullSpec, + operatorClient: operatorClient, + kubeClient: kubeClient, + configMapLister: kubeInformersForNamespaces.ConfigMapLister(), + isStartupMonitorEnabledFn: isStartupMonitorEnabledFn, + notOnSingleReplicaTopologyFn: notOnSingleReplicaTopologyFn, } return factory.New().WithInformers( @@ -100,7 +103,8 @@ func (c TargetConfigController) sync(ctx context.Context, syncContext factory.Sy } // block until config is observed and specific paths are present - if err := isRequiredConfigPresent(operatorSpec.ObservedConfig.Raw); err != nil { + isNotOnSingleReplicaTopology := c.notOnSingleReplicaTopologyFn() + if err := isRequiredConfigPresent(operatorSpec.ObservedConfig.Raw, isNotOnSingleReplicaTopology); err != nil { syncContext.Recorder().Warning("ConfigMissing", err.Error()) return err } @@ -116,7 +120,7 @@ func (c TargetConfigController) sync(ctx context.Context, syncContext factory.Sy return nil } -func isRequiredConfigPresent(config []byte) error { +func isRequiredConfigPresent(config []byte, isNotSingleNode bool) error { if len(config) == 0 { return fmt.Errorf("no observedConfig") } @@ -148,6 +152,16 @@ func isRequiredConfigPresent(config []byte) error { if configValString, ok := configVal.(string); ok && len(configValString) == 0 { return fmt.Errorf("%v empty in config", strings.Join(requiredPath, ".")) } + + if len(requiredPath) == 2 && requiredPath[0] == "apiServerArguments" && requiredPath[1] == "etcd-servers" && isNotSingleNode { + configValSlice, ok := configVal.([]interface{}) + if !ok { + return fmt.Errorf("%v is not a slice", strings.Join(requiredPath, ".")) + } + if len(configValSlice) < 3 { + return fmt.Errorf("%v has less than three endpoints: %v", strings.Join(requiredPath, "."), configValSlice) + } + } } return nil } diff --git a/pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go b/pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go index 1d136467ca..1f4d9f7c4f 100644 --- a/pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go +++ b/pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go @@ -1,6 +1,7 @@ package targetconfigcontroller import ( + "fmt" "strings" "testing" @@ -110,7 +111,7 @@ func TestIsRequiredConfigPresent(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - actual := isRequiredConfigPresent([]byte(test.config)) + actual := isRequiredConfigPresent([]byte(test.config), false) switch { case actual == nil && len(test.expectedError) == 0: case actual == nil && len(test.expectedError) != 0: @@ -237,3 +238,101 @@ func TestManageTemplate(t *testing.T) { }) } } + +func TestIsRequiredConfigPresentEtcdEndpoints(t *testing.T) { + configTemplate := `{ + "servingInfo": { + "namedCertificates": [ + { + "certFile": "/etc/kubernetes/static-pod-certs/secrets/localhost-serving-cert-certkey/tls.crt", + "keyFile": "/etc/kubernetes/static-pod-certs/secrets/localhost-serving-cert-certkey/tls.key" + } + ] + }, + "admission": {"pluginConfig": { "network.openshift.io/RestrictedEndpointsAdmission": {}}}, + "apiServerArguments": { + "etcd-servers": %s + } + } + ` + tests := []struct { + name string + etcdServers string + expectedError string + isNotSingleNode bool + }{ + { + name: "nil-storage-urls", + etcdServers: "null", + expectedError: "apiServerArguments.etcd-servers null in config", + }, + { + name: "missing-storage-urls", + etcdServers: "[]", + expectedError: "apiServerArguments.etcd-servers empty in config", + }, + { + name: "empty-string-storage-urls", + etcdServers: `""`, + expectedError: "apiServerArguments.etcd-servers empty in config", + }, + { + name: "one-etcd-server", + etcdServers: `[ "val" ]`, + isNotSingleNode: true, + expectedError: "apiServerArguments.etcd-servers has less than three endpoints", + }, + { + name: "one-etcd-server-sno", + etcdServers: `[ "val" ]`, + isNotSingleNode: false, + }, + { + name: "two-etcd-servers", + etcdServers: `[ "val1", "val2" ]`, + isNotSingleNode: true, + expectedError: "apiServerArguments.etcd-servers has less than three endpoints", + }, + { + name: "two-etcd-servers-sno", + etcdServers: `[ "val1", "val2" ]`, + isNotSingleNode: false, + }, + { + name: "three-etcd-servers", + etcdServers: `[ "val1", "val2", "val3" ]`, + isNotSingleNode: true, + }, + { + name: "three-etcd-servers-sno", + etcdServers: `[ "val1", "val2", "val3" ]`, + isNotSingleNode: false, + }, + { + name: "four-etcd-servers", + etcdServers: `[ "val1", "val2", "val3", "val4" ]`, + isNotSingleNode: true, + }, + { + name: "four-etcd-servers-sno", + etcdServers: `[ "val1", "val2", "val3", "val4" ]`, + isNotSingleNode: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + config := fmt.Sprintf(configTemplate, test.etcdServers) + actual := isRequiredConfigPresent([]byte(config), test.isNotSingleNode) + switch { + case actual == nil && len(test.expectedError) == 0: + case actual == nil && len(test.expectedError) != 0: + t.Fatal(actual) + case actual != nil && len(test.expectedError) == 0: + t.Fatal(actual) + case actual != nil && len(test.expectedError) != 0 && !strings.Contains(actual.Error(), test.expectedError): + t.Fatal(actual) + } + }) + } +}