diff --git a/controllers/storagecluster/provider_server.go b/controllers/storagecluster/provider_server.go index 90686e2d2d..3b21f99bfe 100644 --- a/controllers/storagecluster/provider_server.go +++ b/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"), }, diff --git a/controllers/storagecluster/provider_server_test.go b/controllers/storagecluster/provider_server_test.go index 054dc38cbd..c3807b3cdb 100644 --- a/controllers/storagecluster/provider_server_test.go +++ b/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, + }, + } +}