Implements review comments

Author: appiepollo14

api/operator/v1alpha1/certmanager_types.go

@@ -82,10 +82,11 @@ type DeploymentConfig struct {
 	// +optional
 	OverrideResources CertManagerResourceRequirements `json:"overrideResources,omitempty"`
 
-	// Replicas defines the number of replicas to run for an operand
+	// OverrideReplicas defines the number of replicas to run for an operand
 	// +kubebuilder:validation:Optional
+	// +kubebuilder:validation:Minimum=1
 	// +optional
-	OverrideReplicas int32 `json:"overrideReplicas,omitempty"`
+	OverrideReplicas *int32 `json:"overrideReplicas,omitempty"`
 
 	// +kubebuilder:validation:Optional
 	// +optional

pkg/controller/deployment/deployment_helper.go

@@ -216,10 +216,10 @@ func getOverridePodLabelsFor(certmanagerinformer certmanagerinformer.CertManager
 
 // getOverrideReplicasFor is a helper function that returns the OverrideReplicas provided
 // in the operator spec based on the deployment name.
-func getOverrideReplicasFor(certmanagerinformer certmanagerinformer.CertManagerInformer, deploymentName string) (int32, error) {
+func getOverrideReplicasFor(certmanagerinformer certmanagerinformer.CertManagerInformer, deploymentName string) (*int32, error) {
 	certmanager, err := certmanagerinformer.Lister().Get("cluster")
 	if err != nil {
-		return 0, fmt.Errorf("failed to get certmanager %q due to %v", "cluster", err)
+		return nil, fmt.Errorf("failed to get certmanager %q due to %v", "cluster", err)
 	}
 
 	switch deploymentName {
@@ -236,9 +236,9 @@ func getOverrideReplicasFor(certmanagerinformer certmanagerinformer.CertManagerI
 			return certmanager.Spec.CAInjectorConfig.OverrideReplicas, nil
 		}
 	default:
-		return 0, fmt.Errorf("unsupported deployment name %q provided", deploymentName)
+		return nil, fmt.Errorf("unsupported deployment name %q provided", deploymentName)
 	}
-	return 0, nil
+	return nil, nil
 }
 
 // getOverrideResourcesFor is a helper function that returns the OverrideResources provided

pkg/controller/deployment/deployment_helper_test.go

@@ -946,11 +946,13 @@ func TestGetOverrideSchedulingFor(t *testing.T) {
 }
 
 func TestGetOverrideReplicasFor(t *testing.T) {
+	ptr := func(i int32) *int32 { return &i }
+
 	tests := []struct {
 		name                     string
 		certManagerObj           v1alpha1.CertManager
 		deploymentName           string
-		expectedOverrideReplicas int32
+		expectedOverrideReplicas *int32
 	}{
 		{
 			name: "get override replicas of cert manager controller config",
@@ -960,12 +962,12 @@ func TestGetOverrideReplicasFor(t *testing.T) {
 				},
 				Spec: v1alpha1.CertManagerSpec{
 					ControllerConfig: &v1alpha1.DeploymentConfig{
-						OverrideReplicas: 2,
+						OverrideReplicas: ptr(2),
 					},
 				},
 			},
 			deploymentName:           certmanagerControllerDeployment,
-			expectedOverrideReplicas: 2,
+			expectedOverrideReplicas: ptr(2),
 		},
 		{
 			name: "get override scheduling of cert manager webhook config",
@@ -975,12 +977,12 @@ func TestGetOverrideReplicasFor(t *testing.T) {
 				},
 				Spec: v1alpha1.CertManagerSpec{
 					WebhookConfig: &v1alpha1.DeploymentConfig{
-						OverrideReplicas: 0,
+						OverrideReplicas: ptr(0),
 					},
 				},
 			},
 			deploymentName:           certmanagerWebhookDeployment,
-			expectedOverrideReplicas: 0,
+			expectedOverrideReplicas: ptr(0),
 		},
 		{
 			name: "get override scheduling of cert manager cainjector config",
@@ -990,12 +992,25 @@ func TestGetOverrideReplicasFor(t *testing.T) {
 				},
 				Spec: v1alpha1.CertManagerSpec{
 					CAInjectorConfig: &v1alpha1.DeploymentConfig{
-						OverrideReplicas: 2,
+						OverrideReplicas: ptr(2),
 					},
 				},
 			},
 			deploymentName:           certmanagerCAinjectorDeployment,
-			expectedOverrideReplicas: 2,
+			expectedOverrideReplicas: ptr(2),
+		},
+		{
+			name: "no override replicas configured",
+			certManagerObj: v1alpha1.CertManager{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "cluster",
+				},
+				Spec: v1alpha1.CertManagerSpec{
+					// no configs set
+				},
+			},
+			deploymentName:           certmanagerControllerDeployment,
+			expectedOverrideReplicas: nil,
 		},
 	}
 

pkg/controller/deployment/deployment_overrides.go

@@ -12,7 +12,6 @@ import (
 	"k8s.io/utils/ptr"
 
 	operatorv1 "github.com/openshift/api/operator/v1"
-	v1 "github.com/openshift/api/operator/v1"
 	"github.com/operator-framework/operator-lib/proxy"
 
 	"github.com/openshift/cert-manager-operator/api/operator/v1alpha1"
@@ -59,9 +58,9 @@ type overrideEnvFunc func(certmanagerinformer.CertManagerInformer, string) ([]co
 type overrideLabelsFunc func(certmanagerinformer.CertManagerInformer, string) (map[string]string, error)
 
 // overrideReplicasFunc defines a function signature that is accepted by
-// withContainerReplicasOverrideHook(). This function returns the override
+// withDeploymentReplicasOverrideHook(). This function returns the override
 // replicas provided to cert-manager-operator spec.
-type overrideReplicasFunc func(certmanagerinformer.CertManagerInformer, string) (int32, error)
+type overrideReplicasFunc func(certmanagerinformer.CertManagerInformer, string) (*int32, error)
 
 // overrideResourcesFunc defines a function signature that is accepted by
 // withContainerResourcesOverrideHook(). This function returns the override
@@ -143,18 +142,18 @@ func withContainerResourcesOverrideHook(certmanagerinformer certmanagerinformer.
 	}
 }
 
-// withContainerReplicasOverrideHook overrides the deployment replicas with those provided by
+// withDeploymentReplicasOverrideHook overrides the deployment replicas with those provided by
 // the overrideReplicasFunc function.
-func withContainerReplicasOverrideHook(certmanagerinformer certmanagerinformer.CertManagerInformer, deploymentName string, fn overrideReplicasFunc) func(operatorSpec *v1.OperatorSpec, deployment *appsv1.Deployment) error {
-	return func(operatorSpec *v1.OperatorSpec, deployment *appsv1.Deployment) error {
+func withDeploymentReplicasOverrideHook(certmanagerinformer certmanagerinformer.CertManagerInformer, deploymentName string, fn overrideReplicasFunc) func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error {
+	return func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error {
 		overrideReplicas, err := fn(certmanagerinformer, deploymentName)
 		if err != nil {
 			return err
 		}
 
-		if overrideReplicas != 0 {
+		if overrideReplicas != nil {
 			if deployment.Name == deploymentName {
-				deployment.Spec.Replicas = &overrideReplicas
+				deployment.Spec.Replicas = overrideReplicas
 			}
 		}
 		return nil

pkg/controller/deployment/generic_deployment_controller.go

@@ -50,7 +50,7 @@ func newGenericDeploymentController(
 		withContainerArgsValidateHook(certManagerOperatorInformers.Operator().V1alpha1().CertManagers(), deployment.Name),
 		withContainerEnvOverrideHook(certManagerOperatorInformers.Operator().V1alpha1().CertManagers(), deployment.Name, getOverrideEnvFor),
 		withContainerEnvValidateHook(certManagerOperatorInformers.Operator().V1alpha1().CertManagers(), deployment.Name),
-		withContainerReplicasOverrideHook(certManagerOperatorInformers.Operator().V1alpha1().CertManagers(), deployment.Name, getOverrideReplicasFor),
+		withDeploymentReplicasOverrideHook(certManagerOperatorInformers.Operator().V1alpha1().CertManagers(), deployment.Name, getOverrideReplicasFor),
 		withContainerResourcesOverrideHook(certManagerOperatorInformers.Operator().V1alpha1().CertManagers(), deployment.Name, getOverrideResourcesFor),
 		withContainerResourcesValidateHook(certManagerOperatorInformers.Operator().V1alpha1().CertManagers(), deployment.Name),
 		withPodSchedulingOverrideHook(certManagerOperatorInformers.Operator().V1alpha1().CertManagers(), deployment.Name, getOverrideSchedulingFor),