Added clusterIP service type for provider server

controllers/storagecluster/provider_server.go

@@ -146,11 +146,16 @@ func (o *ocsProviderServer) createDeployment(r *StorageClusterReconciler, instan
 
 func (o *ocsProviderServer) createService(r *StorageClusterReconciler, instance *ocsv1.StorageCluster) (reconcile.Result, error) {
 
-	if instance.Spec.ProviderAPIServerServiceType != "" && instance.Spec.ProviderAPIServerServiceType != corev1.ServiceTypeNodePort &&
-		instance.Spec.ProviderAPIServerServiceType != corev1.ServiceTypeLoadBalancer {
-		err := fmt.Errorf("providerAPIServer only supports service of type %s and %s", corev1.ServiceTypeNodePort, corev1.ServiceTypeLoadBalancer)
-		r.Log.Error(err, "Failed to create/update service, Requested ServiceType is", "ServiceType", instance.Spec.ProviderAPIServerServiceType)
-		return reconcile.Result{}, err
+	if instance.Spec.ProviderAPIServerServiceType != "" {
+		switch instance.Spec.ProviderAPIServerServiceType {
+		case corev1.ServiceTypeClusterIP, corev1.ServiceTypeLoadBalancer, corev1.ServiceTypeNodePort:
+		default:
+			err := fmt.Errorf("providerAPIServer only supports service of type %s, %s and %s",
+				corev1.ServiceTypeNodePort, corev1.ServiceTypeLoadBalancer, corev1.ServiceTypeClusterIP)
+			r.Log.Error(err, "Failed to create/update service, Requested ServiceType is", "ServiceType", instance.Spec.ProviderAPIServerServiceType)
+			return reconcile.Result{}, err
+		}
+
 	}
 
 	desiredService := GetProviderAPIServerService(instance)
@@ -186,7 +191,8 @@ func (o *ocsProviderServer) createService(r *StorageClusterReconciler, instance
 
 	r.Log.Info("Service create/update succeeded")
 
-	if instance.Spec.ProviderAPIServerServiceType == corev1.ServiceTypeLoadBalancer {
+	switch instance.Spec.ProviderAPIServerServiceType {
+	case corev1.ServiceTypeLoadBalancer:
 		endpoint := o.getLoadBalancerServiceEndpoint(actualService)
 
 		if endpoint == "" {
@@ -196,7 +202,11 @@ func (o *ocsProviderServer) createService(r *StorageClusterReconciler, instance
 		}
 
 		instance.Status.StorageProviderEndpoint = fmt.Sprintf("%s:%d", endpoint, ocsProviderServicePort)
-	} else {
+
+	case corev1.ServiceTypeClusterIP:
+		instance.Status.StorageProviderEndpoint = fmt.Sprintf("%s:%d", actualService.Spec.ClusterIP, ocsProviderServicePort)
+
+	default: // Nodeport is the default ServiceType for the provider server
 		nodeAddresses, err := o.getWorkerNodesInternalIPAddresses(r)
 		if err != nil {
 			return reconcile.Result{}, err
@@ -384,7 +394,13 @@ func GetProviderAPIServerService(instance *ocsv1.StorageCluster) *corev1.Service
 			},
 			Ports: []corev1.ServicePort{
 				{
-					NodePort:   ocsProviderServiceNodePort,
+					NodePort: func() int32 {
+						// ClusterIP service doesn't need nodePort
+						if instance.Spec.ProviderAPIServerServiceType == corev1.ServiceTypeClusterIP {
+							return 0
+						}
+						return ocsProviderServiceNodePort
+					}(),
 					Port:       ocsProviderServicePort,
 					TargetPort: intstr.FromString("ocs-provider"),
 				},

controllers/storagecluster/provider_server_test.go

@@ -149,10 +149,68 @@ func TestOcsProviderServerEnsureCreated(t *testing.T) {
 		assert.NoError(t, r.Client.Get(context.TODO(), client.ObjectKeyFromObject(secret), secret))
 	})
 
-	t.Run("Ensure that Service is not created when AllowRemoteStorageConsumers is enabled and ProviderAPIServerServiceType is set to any other value than NodePort or LoadBalancer", func(t *testing.T) {
+	t.Run("Ensure that Deployment,Service,Secret is created when AllowRemoteStorageConsumers and ProviderAPIServerServiceType set to ClusterIP", func(t *testing.T) {
 
 		r, instance := createSetupForOcsProviderTest(t, true, corev1.ServiceTypeClusterIP)
 
+		obj := &ocsProviderServer{}
+		res, err := obj.ensureCreated(r, instance)
+		assert.NoError(t, err)
+		assert.False(t, res.IsZero())
+
+		// storagecluster controller waits for svc status to fetch the IP and it requeue
+		// as we are using a fake client and it does not fill the status automatically.
+		// update the required status field of the svc to overcome the failure and requeue.
+		service := &corev1.Service{
+			ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName},
+		}
+		err = r.Update(context.TODO(), service)
+		assert.NoError(t, err)
+
+		// call ensureCreated again after filling the status of svc, It will fail on deployment now
+		res, err = obj.ensureCreated(r, instance)
+		assert.NoError(t, err)
+		assert.False(t, res.IsZero())
+
+		// storagecluster controller waits for deployment status to fetch the replica count and it requeue
+		// as we are using a fake client and it does not fill the status automatically.
+		// update the required status field of the deployment to overcome the failure and requeue.
+		deployment := &appsv1.Deployment{
+			ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName},
+		}
+		deployment.Status.AvailableReplicas = 1
+		err = r.Update(context.TODO(), deployment)
+		assert.NoError(t, err)
+
+		// call ensureCreated again after filling the status of deployment, It will pass now
+		res, err = obj.ensureCreated(r, instance)
+		assert.NoError(t, err)
+		assert.True(t, res.IsZero())
+
+		deployment = &appsv1.Deployment{
+			ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName},
+		}
+		assert.NoError(t, r.Client.Get(context.TODO(), client.ObjectKeyFromObject(deployment), deployment))
+		expectedDeployment := GetProviderAPIServerDeploymentForTest(instance)
+		assert.Equal(t, deployment.Spec, expectedDeployment.Spec)
+
+		service = &corev1.Service{
+			ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName},
+		}
+		assert.NoError(t, r.Client.Get(context.TODO(), client.ObjectKeyFromObject(service), service))
+		expectedService := GetClusterIPProviderAPIServerServiceForTest(instance)
+		assert.Equal(t, expectedService.Spec, service.Spec)
+
+		secret := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName},
+		}
+		assert.NoError(t, r.Client.Get(context.TODO(), client.ObjectKeyFromObject(secret), secret))
+	})
+
+	t.Run("Ensure that Service is not created when AllowRemoteStorageConsumers is enabled and ProviderAPIServerServiceType is set to any other value than NodePort, ClusterIP or LoadBalancer", func(t *testing.T) {
+
+		r, instance := createSetupForOcsProviderTest(t, true, corev1.ServiceTypeExternalName)
+
 		obj := &ocsProviderServer{}
 		_, err := obj.ensureCreated(r, instance)
 		assert.Errorf(t, err, "only supports service of type")
@@ -391,3 +449,28 @@ func GetLoadBalancerProviderAPIServerServiceForTest(instance *ocsv1.StorageClust
 		},
 	}
 }
+
+func GetClusterIPProviderAPIServerServiceForTest(instance *ocsv1.StorageCluster) *corev1.Service {
+
+	return &corev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      ocsProviderServerName,
+			Namespace: instance.Namespace,
+			Annotations: map[string]string{
+				"service.beta.openshift.io/serving-cert-secret-name": ocsProviderCertSecretName,
+			},
+		},
+		Spec: corev1.ServiceSpec{
+			Selector: map[string]string{
+				"app": "ocsProviderApiServer",
+			},
+			Ports: []corev1.ServicePort{
+				{
+					Port:       ocsProviderServicePort,
+					TargetPort: intstr.FromString("ocs-provider"),
+				},
+			},
+			Type: corev1.ServiceTypeClusterIP,
+		},
+	}
+}