diff --git a/pkg/operator/csi/csidrivercontrollerservicecontroller/helpers.go b/pkg/operator/csi/csidrivercontrollerservicecontroller/helpers.go index 985949a50d..a0328ceae4 100644 --- a/pkg/operator/csi/csidrivercontrollerservicecontroller/helpers.go +++ b/pkg/operator/csi/csidrivercontrollerservicecontroller/helpers.go @@ -12,9 +12,7 @@ import ( appsv1 "k8s.io/api/apps/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/labels" corev1 "k8s.io/client-go/informers/core/v1" - corev1listers "k8s.io/client-go/listers/core/v1" configv1 "github.com/openshift/api/config/v1" opv1 "github.com/openshift/api/operator/v1" @@ -141,22 +139,20 @@ func WithSecretHashAnnotationHook( } } -// WithReplicasHook sets the deployment.Spec.Replicas field according to the number -// of available nodes. If the number of available nodes is bigger than one, then the -// number of replicas will be two. The number of nodes is determined by the node -// selector specified in the field deployment.Spec.Templates.NodeSelector. -// When node ports or hostNetwork are used, maxSurge=0 should be set in the +// WithReplicasHook sets the deployment.Spec.Replicas field according to the ControlPlaneTopology +// mode. If the topology is set to 'HighlyAvailable' then the number of replicas will be two. +// Else it will be one. When node ports or hostNetwork are used, maxSurge=0 should be set in the // Deployment RollingUpdate strategy to prevent the new pod from getting stuck // waiting for a node with free ports. -func WithReplicasHook(nodeLister corev1listers.NodeLister) dc.DeploymentHookFunc { +func WithReplicasHook(configInformer configinformers.SharedInformerFactory) dc.DeploymentHookFunc { return func(_ *opv1.OperatorSpec, deployment *appsv1.Deployment) error { - nodeSelector := deployment.Spec.Template.Spec.NodeSelector - nodes, err := nodeLister.List(labels.SelectorFromSet(nodeSelector)) + infra, err := configInformer.Config().V1().Infrastructures().Lister().Get(infraConfigName) if err != nil { return err } + replicas := int32(1) - if len(nodes) > 1 { + if infra.Status.ControlPlaneTopology == configv1.HighlyAvailableTopologyMode { replicas = int32(2) } deployment.Spec.Replicas = &replicas diff --git a/pkg/operator/csi/csidrivercontrollerservicecontroller/helpers_test.go b/pkg/operator/csi/csidrivercontrollerservicecontroller/helpers_test.go index 20f00588e0..021ec30415 100644 --- a/pkg/operator/csi/csidrivercontrollerservicecontroller/helpers_test.go +++ b/pkg/operator/csi/csidrivercontrollerservicecontroller/helpers_test.go @@ -15,8 +15,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" - coreinformers "k8s.io/client-go/informers" - fakecore "k8s.io/client-go/kubernetes/fake" "github.com/google/go-cmp/cmp" "sigs.k8s.io/yaml" @@ -159,135 +157,60 @@ func TestWithObservedProxyDeploymentHook(t *testing.T) { } func TestWithReplicasHook(t *testing.T) { - var ( - argsLevel2 = 2 - masterNodeLabels = map[string]string{"node-role.kubernetes.io/master": ""} - workerNodeLabels = map[string]string{"node-role.kubernetes.io/worker": ""} - ) + argsLevel2 := 2 testCases := []struct { name string - initialDriver *fakeDriverInstance - initialNodes []*v1.Node + infra *configv1.Infrastructure initialDeployment *appsv1.Deployment expectedDeployment *appsv1.Deployment - expectError bool }{ { - name: "three-node control-plane", - initialDriver: makeFakeDriverInstance(), - initialNodes: []*v1.Node{ - makeNode("A", masterNodeLabels), - makeNode("B", masterNodeLabels), - makeNode("C", masterNodeLabels), - }, + name: "highly available topology", + infra: makeInfraWithCPTopology(configv1.HighlyAvailableTopologyMode), initialDeployment: makeDeployment( defaultClusterID, argsLevel2, defaultImages(), withDeploymentReplicas(1), - withDeploymentGeneration(1, 0)), - expectedDeployment: makeDeployment( - defaultClusterID, - argsLevel2, - defaultImages(), - withDeploymentReplicas(2), - withDeploymentGeneration(1, 0)), - expectError: false, - }, - { - name: "three-node control-plane with one worker node", - initialDriver: makeFakeDriverInstance(), - initialNodes: []*v1.Node{ - makeNode("A", masterNodeLabels), - makeNode("B", masterNodeLabels), - makeNode("C", masterNodeLabels), - makeNode("D", workerNodeLabels), - }, - initialDeployment: makeDeployment( - defaultClusterID, - argsLevel2, - defaultImages(), - withDeploymentReplicas(1), - withDeploymentGeneration(1, 0)), - expectedDeployment: makeDeployment( - defaultClusterID, - argsLevel2, - defaultImages(), - withDeploymentReplicas(2), - withDeploymentGeneration(1, 0)), - expectError: false, - }, - { - name: "two-node control-plane", - initialDriver: makeFakeDriverInstance(), - initialNodes: []*v1.Node{ - makeNode("A", masterNodeLabels), - makeNode("B", masterNodeLabels), - }, - initialDeployment: makeDeployment( - defaultClusterID, - argsLevel2, - defaultImages(), - withDeploymentReplicas(1), - withDeploymentGeneration(1, 0)), + withDeploymentGeneration(1, 0), + ), expectedDeployment: makeDeployment( defaultClusterID, argsLevel2, defaultImages(), withDeploymentReplicas(2), - withDeploymentGeneration(1, 0)), - expectError: false, + withDeploymentGeneration(1, 0), + ), }, { - name: "single-node control-plane with one worker node", - initialDriver: makeFakeDriverInstance(), - initialNodes: []*v1.Node{ - makeNode("A", masterNodeLabels), - makeNode("B", workerNodeLabels), - }, - initialDeployment: makeDeployment( - defaultClusterID, + name: "single replica topology", + infra: makeInfraWithCPTopology(configv1.SingleReplicaTopologyMode), + initialDeployment: makeDeployment(defaultClusterID, argsLevel2, defaultImages(), withDeploymentReplicas(1), - withDeploymentGeneration(1, 0)), - expectedDeployment: makeDeployment( - defaultClusterID, + withDeploymentGeneration(1, 0), + ), + expectedDeployment: makeDeployment(defaultClusterID, argsLevel2, defaultImages(), withDeploymentReplicas(1), - withDeploymentGeneration(1, 0)), - expectError: false, + withDeploymentGeneration(1, 0), + ), }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - var initialObjects []runtime.Object - - // Add deployment to slice of objects to be added to client and informer - initialObjects = append(initialObjects, tc.initialDeployment) - - // Do the same with nodes - for _, node := range tc.initialNodes { - initialObjects = append(initialObjects, node) - } - - // Create fake client and informer - coreClient := fakecore.NewSimpleClientset(initialObjects...) - coreInformerFactory := coreinformers.NewSharedInformerFactory(coreClient, 0 /*no resync */) - - // Fill the fake informer with the initial deployment and nodes - coreInformerFactory.Apps().V1().Deployments().Informer().GetIndexer().Add(tc.initialDeployment) - for _, node := range initialObjects { - coreInformerFactory.Core().V1().Nodes().Informer().GetIndexer().Add(node) - } - - fn := WithReplicasHook(coreInformerFactory.Core().V1().Nodes().Lister()) - err := fn(&tc.initialDriver.Spec, tc.initialDeployment) - if err != nil && !tc.expectError { - t.Errorf("Expected no error running hook function, got: %v", err) + initialInfras := []runtime.Object{tc.infra} + configClient := fakeconfig.NewSimpleClientset(initialInfras...) + configInformerFactory := configinformers.NewSharedInformerFactory(configClient, 0) + configInformerFactory.Config().V1().Infrastructures().Informer().GetIndexer().Add(initialInfras[0]) + fn := WithReplicasHook(configInformerFactory) + err := fn(nil, tc.initialDeployment) + if err != nil { + t.Errorf("Unexpected error: %v", err) } if !equality.Semantic.DeepEqual(tc.initialDeployment, tc.expectedDeployment) { t.Errorf("Unexpected Deployment content:\n%s", cmp.Diff(tc.initialDeployment, tc.expectedDeployment))