Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tenant cluster loadbalancer service is hardcoded to ClusterIP type #64

Closed
agradouski opened this issue Jan 11, 2022 · 9 comments · Fixed by #80
Closed

Tenant cluster loadbalancer service is hardcoded to ClusterIP type #64

agradouski opened this issue Jan 11, 2022 · 9 comments · Fixed by #80
Assignees

Comments

@agradouski
Copy link
Contributor

With ClusterIP type a tenant cluster will only be accessible within the same cluster, and cannot be exposed to external workloads.

Example load balancer service:

kvcluster-lb      ClusterIP   10.108.169.113   <none>        6443/TCP    4m56s

Currently, there's no way to configure the loadbalancer service, as the spec is hard coded.

@agradouski agradouski changed the title Tenant cluster loadbalancer service is currently hardcoded to ClusterIP type Tenant cluster loadbalancer service is hardcoded to ClusterIP type Jan 11, 2022
@agradouski
Copy link
Contributor Author

For now, to address the immediate issue, we can just expose the service type as a field on KubevirtCluster resource, but a discussion needs to be held on:

  • do we want to expose load balancer service spec as a whole in KubevirtCluster?
  • will a simple service be sufficient for tenant cluster load balancing, or a more advanced implementation for load balancer is warranted?

@wangxin311
Copy link
Contributor

wangxin311 commented Jan 14, 2022

My two cents:

do we want to expose load balancer service spec as a whole in KubevirtCluster?

: I think create a loadbalancer service as a whole is the easiest way to solve the problem,
if the loadbalancer is available on the cluster, and Kubernetes will return both ClusterIP and LoadBalancerIP for the service, and return IP function in loadbalance.go can just return LoadBalancerIP, otherwise, Kubernetes will only return ClusterIP, and LoadbalancerIP will be shown as Pending, in this case, return IP function in loadbalance.go will return ClusterIP like what we're doing today, but of cause the workload cluster is restricted access within the cluster.

example logic

	if len(loadBalancer.Status.LoadBalancer.Ingress) == 0 {
		if len(loadBalancer.Spec.ClusterIP) == 0 {
			return "", fmt.Errorf("the load balancer service is not ready yet")
		}
		return loadBalancer.Spec.ClusterIP, nil
	}
	return loadBalancer.Status.LoadBalancer.Ingress[0].IP, nil

will a simple service be sufficient for tenant cluster load balancing, or a more advanced implementation for load balancer is warranted?

IMO, simply Loadbalancer service is enough, administrator should responsible to manage/deploy Loadbalancer as separately.

@kewang-equinix
Copy link
Contributor

There is one more issue we need address in order to support separate mgmt and workload cluster is to create VM service for each VirtualMachine and expose service as Loadbalancer and then return LB external IP instead of VM IP address to kubevirt cluster api, because mgmt cluster need to SSH into each VM to validate VM state to see if they're ready to join Kubernetes cluster but VM IP only visible within cluster, otherwise, capk-controller-manager may continuously complain "Waiting for underlying VM to bootstrap..."

example yaml for VMI service should looks like this

apiVersion: v1
kind: Service
metadata:
  name: kvcluster121-control-plane-667lr-svc
spec:
  externalTrafficPolicy: Cluster
  ports:
  - name: ssh
    port: 22
    protocol: TCP
    targetPort: 22
  selector:
    name: kvcluster121-control-plane-667lr
  type: LoadBalancer

@agradouski
Copy link
Contributor Author

@kewang-equinix that's a good point.
however, even with current logic we don't strictly require SSH access for each VM, in fact it's optional (based on whether ssh keys for VMs were generated in the first place), see:

	if externalMachine.SupportsCheckingIsBootstrapped() && !conditions.IsTrue(ctx.KubevirtMachine, infrav1.BootstrapExecSucceededCondition) {

having a load-balancer service for each tenant cluster VM is quite demanding in terms of underlying resources, and we might consider removing SSH requirement for the nodes running in external clusters ... or removing SSH-validation path altogether in the long run regardless of where a tenant cluster is provisioned. the assumption being if a VMI is in ready condition, and has an IP, it is ready for the cluster.

@agradouski
Copy link
Contributor Author

for now, let's limit the scope of this issue to tenant cluster loadbalancer only,
separate issue(s) to be created for tenant cluster VMs networking

@agradouski
Copy link
Contributor Author

@agradouski is actively working on this.

@agradouski agradouski self-assigned this Jan 27, 2022
@wangxin311
Copy link
Contributor

@agradouski I agree with you on enable LB service for each VM is too much in most of scenario, and also I support the idea of disable SSH validation as it doesn't provide too much benefit as per currently implementation.
However in some other user case, end user may need access VM to do debugging in case something wrong, VM only accessible within infra Kubernetes cluster as of now but administrator may not comfortable to share infra cluster access to end user, so we probably should provide an configurable option to end-user if they want enable LB service for VM.
I believe for other cloud provider platform like AWS, Openstack, etc, user is allowed to assign public/floating IP for VM, I think we should support that as well, we can track that as a separate task/feature.

@rmohr
Copy link
Contributor

rmohr commented Jan 31, 2022

@wangxin311 I think kubevirt offers a few wasy on accessing VMs via ssh independent of the cloud provider. Trying to summarize a few common patterns, to see if they may meet your demands:

  • cloud-provider-like access to all VMs via a jumphost/bastion: I created for another need some time ago jumppod. One LB or node-port for all VMs would then be sufficient (perfect for heavy work, like copying big blops via scp, ...)
  • maintenance access with virtctl ssh which uses port-forward via the k8s apiserver, no extra IP or port exposure necessary (perfect for debugging and small maintenance tasks).

In combination with automatic ssh key propagation to VMs like implemented by @davidvossel in kubevirt/kubevirt#4195, I believe that kubevirt is pretty flexible here and that it can be handled orthogonal to the cloud provider.

@kewang-equinix I believe that in principle that could also be levereged for ssh validation if the management part is in another cluster in the future. What do you think?

@Arvinderpal
Copy link

FYI, loosely related to this LB discussion, there is long standing issue for supporting LB providers as first-class citizens in CAPI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants