skypilot-org / skypilot

SkyPilot: Run AI and batch jobs on any infra (Kubernetes or 12+ clouds). Get unified execution, cost savings, and high GPU availability via a simple interface.
https://skypilot.readthedocs.io
Apache License 2.0
6.81k stars 513 forks source link

[k8s] Support configuring annotations when launching #3757

Open fozziethebeat opened 4 months ago

fozziethebeat commented 4 months ago

When running k8s within AWS, the latest version of the aws-load-balancer-controller defaults to creating load balancer and ingress services that are internal only and bind to an internal only subnet. To create a publicly accessible endpoint these services need to set alb.ingress.kubernetes.io/scheme: internet-facing: internet-facing.

The simplest way to fix this this is to add a stanza to the master sky config setting for arbitrary annotations that apply to either the loadbalancer or the ingress service that's setup.

A user should be able to specify something sorta like:

kubernetes:
  ports: loadbalancer
  ports_annotations:
    alb.ingress.kubernetes.io/scheme: internet-facing: internet-facing

Then this should apply to the internal load balancer template used to configure the service.

I've already manually hacked this to confirm this solves the problem.

fozziethebeat commented 4 months ago

Note: I can tidy up my hack and get this working

romilbhardwaj commented 4 months ago

Thanks for raising this @fozziethebeat - is this behavior now default on EKS? I.e., do load balancer services need to have this annotation to create a public facing endpoint?

Also, I'm curious if using the kubernetes.custom_metadata key in config.yaml would be a workaround:

kubernetes:
  custom_metadata:
    annotations:
      alb.ingress.kubernetes.io/scheme: internet-facing: internet-facing

Note that the custom_metadata specified here is applied to all kubernetes objects created by SkyPilot (think pods, services, ingresses ...), so this annotation would be applied in quite a few places where it shouldn't be applied. Hopefully having those annotations on irrelevant objects will be a no-op?

fozziethebeat commented 4 months ago

The latest version defaults all the ingress services to be internal facing only so I am pretty sure these annotations are required. I tried seeing if you can configure the controller to override that default but the k8s people told me no, it would require forking the controller which seems too much overhead.

Let me try setting this custom metadata and seeing what breaks. Since that pattern already exists maybe we just have a separate annotations section there that only applies to these endpoint services

github-actions[bot] commented 6 days ago

This issue is stale because it has been open 120 days with no activity. Remove stale label or comment or this will be closed in 10 days.