Adding the disable-load-balancer flag that replace the type of service from LoadBalancer to ClusterIP

[liranmauda]

Adding the disable-load-balancer flag that replaces the type of service from LoadBalancer to ClusterIP

Functionality

• Adding the disable-load-balancer flag to the options
• Add the disable-load-balancer flag functionality in the install command

It will be respected only in pod start/restart, and will not be reconciled after

Testing

• Run noobaa install --disable-load-balancer and see that the service type is ClusterIP

[dannyzaken]

I think this should be a disabling flag, e.g. DisableLoadBalancerService, instead of using an enabling flag.

the problem with using an enabling flag is that after upgrading, existing CRs will not have this flag set and it will change the service to ClusterIP.
of course, if we want ClusterIP to be the default for new systems we will need to pass DisableLoadBalancerService: true in noobaa CR from CLI or ocs-operator

[liranmauda]

Yes, I was thinking the same after doing this PR.
And I think we should not default it.

I will change it.

another point is what should we do with the ExternalDNS?
@dannyzaken WDYT?

[liranmauda]

Also, in second thought regarding Phase4, I think we should handle it also, hance, we let it be changed after installation.
WDYT?

[baum]

Hey @liranmauda, looks good! 🙄

There is also code logic that might require tweaking in phase3 - CheckServiceStatus() assumes NodePort in service ports, however this field is missing in ClusterIP:

   clusterIP: 10.101.217.181
   ports:
   - name: mgmt
     port: 1780
     protocol: TCP
     targetPort: 8080
   ...

For dynamic LoadBalancer <-> ClusterIP transition (are we going to support that ?), those NodePorts should be removed from the ports array during LoadBalancer -> ClusterIP transition, in addition to Service type change. This logic probably belongs to phase2 SetDesiredServiceMgmt() and SetDesiredServiceS3()

WDYT?

[liranmauda]

@baum
Regarding the ports, yes we should probably also remove them, but as far as I know, the ports are getting ignored on ClusterIP, Am I correct?

[baum]

Not sure.

In OCP cluster, using kubectl patch, no NodePort removal is required:

NAME   TYPE           CLUSTER-IP      EXTERNAL-IP                                                              PORT(S)                                                    AGE
➜  noobaa-operator git:(master) k patch svc -n baum-5-9-0 s3  -p '{"spec": {"type": "ClusterIP" }}'
service/s3 patched
➜  noobaa-operator git:(master) k get svc -n baum-5-9-0 s3
NAME   TYPE        CLUSTER-IP      EXTERNAL-IP   PORT(S)                            AGE
s3     ClusterIP   172.30.99.106   <none>        80/TCP,443/TCP,8444/TCP,7004/TCP   3d22h

However in my dev cluster, I get an error:

➜  noobaa-operator git:(master) k patch svc  s3  -p '{"spec": {"type": "ClusterIP" }}'
The Service "s3" is invalid:
* spec.ports[0].nodePort: Forbidden: may not be used when `type` is 'ClusterIP'
* spec.ports[1].nodePort: Forbidden: may not be used when `type` is 'ClusterIP'
* spec.ports[2].nodePort: Forbidden: may not be used when `type` is 'ClusterIP'
* spec.ports[3].nodePort: Forbidden: may not be used when `type` is 'ClusterIP'```

[liranmauda]

@baum
So I guess we should move that into the code.

[liranmauda]

According to https://pkg.go.dev/k8s.io/api/core/v1#ServicePort.NodePort

This field will be wiped when updating a Service to no longer need it (e.g. changing type
from NodePort to ClusterIP).

[baum]

👍