feat: K8s allow proxying through Gateway #9469

NicholasBlaskey · 2024-06-04T18:44:35Z

Ticket

https://hpe-aiatscale.atlassian.net/browse/RM-137

Description

Add support for reaching K8s tasks in additional k8s clusters.

Test Plan

Checklist

Changes have been manually QA'd
User-facing API changes need the "User-facing API Change" label.
Release notes should be added as a separate file under docs/release-notes/.
See Release Note for details.
Licenses should be included for new code which was copied and/or modified from any external code.

netlify · 2024-06-04T18:44:51Z

✅ Deploy Preview for determined-ui canceled.

Name	Link
🔨 Latest commit	`3a99b0c`
🔍 Latest deploy log	https://app.netlify.com/sites/determined-ui/deploys/6671b8c0bd17b90007caf3d9

codecov · 2024-06-04T18:47:18Z

Codecov Report

Attention: Patch coverage is 54.62795% with 250 lines in your changes missing coverage. Please review.

Project coverage is 49.33%. Comparing base (fec31a1) to head (131d0a6).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9469      +/-   ##
==========================================
+ Coverage   49.31%   49.33%   +0.01%     
==========================================
  Files        1241     1243       +2     
  Lines      161338   161827     +489     
  Branches     2868     2868              
==========================================
+ Hits        79566    79831     +265     
- Misses      81600    81824     +224     
  Partials      172      172

Flag	Coverage Δ
backend	`44.02% <54.82%> (+0.10%)`	⬆️
harness	`63.80% <0.00%> (-0.02%)`	⬇️
web	`44.91% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
master/internal/rm/kubernetesrm/gateway_spec.go	`100.00% <100.00%> (ø)`
master/internal/task/allocation.go	`76.42% <100.00%> (ø)`
master/pkg/check/check_numbers.go	`28.00% <100.00%> (+11.56%)`	⬆️
master/pkg/check/check_string.go	`78.94% <100.00%> (+78.94%)`	⬆️
...nal/rm/kubernetesrm/kubernetes_resource_manager.go	`30.81% <0.00%> (-0.11%)`	⬇️
harness/determined/exec/prep_container.py	`17.30% <0.00%> (-0.17%)`	⬇️
master/internal/rm/kubernetesrm/spec.go	`82.02% <97.18%> (+2.72%)`	⬆️
master/internal/config/resource_manager_config.go	`68.42% <84.21%> (+3.15%)`	⬆️
master/internal/rm/kubernetesrm/request_queue.go	`82.40% <50.00%> (-7.50%)`	⬇️
master/internal/rm/kubernetesrm/gateway_service.go	`66.66% <66.66%> (ø)`
... and 3 more

... and 4 files with indirect coverage changes

.circleci/real_config.yml

NicholasBlaskey · 2024-06-13T14:39:05Z

docs/setup-cluster/k8s/internal-task-gateway.rst

+provisioner provided by Project Contour.
+
+On a local dev machine, you can use Minikube and Contour as the controller. We provide a script to
+simplify the process. This can be found in `tools/k8s/launch-minikube-with-gateway.sh`.


I think it might make sense to list out the steps for deploying the gateway and explaining each step so customers will have an easier time understanding.

specifically about this script or in general for deploying a gateway?
There are some shared steps but the rest are controller dependent. We don't add a lot of custom steps on top, do we?
https://projectcontour.io/getting-started/ gives a good description for Contour would we do something differently

hamidzr · 2024-06-13T21:52:34Z

master/internal/config/resource_manager_config.go

-const defaultResourcePoolName = "default"
+const (
+	defaultResourcePoolName = "default"
+	validGWPortRangeStart   = 1025


reasonable?

hamidzr · 2024-06-13T21:59:44Z

master/internal/rm/kubernetesrm/spec.go

+		}
+
+		for i, proxyPort := range j.req.ProxyPorts {
+			sharedName := fmt.Sprintf("%s-%d", j.jobName, i)


not sure what'd be a reasonable way to de-indent this but it's going too far right makes it hard to read on gh

carolinaecalderon

LGTM -- a few clarifying comments, but code looks good!

carolinaecalderon · 2024-06-17T17:03:44Z

master/pkg/check/check_test.go

+	}{
+		{"valid-label", false},
+		{"Valid-Label_123", false},
+		{"a", false},


nit: can we add a case with special characters like "#$%"?

carolinaecalderon · 2024-06-17T17:07:54Z

docs/setup-cluster/k8s/controller-reviews.rst

+https://projectcontour.io/docs/1.29/guides/gateway-api/
+
+***************
+ Envoy Gateway


is there a minimum version we recommend?

for k8s? or this particula gateway implementaiton?
for k8s we do mention 1.27 in the other page.

docs/setup-cluster/k8s/internal-task-gateway.rst

carolinaecalderon · 2024-06-17T17:26:19Z

master/internal/rm/kubernetesrm/gateway_service.go

+// I wanted to do this all in patches, but Gateways don't yet support strategic merge patch.
+// Its pretty easy to use json patch to add the port, but removing the port is a lot harder
+// as I don't think you can remove by value. Instead just serialize reading then submitting
+// updates on the gateway.


should we add a TODO for this? in the rm backlog

I removed this comment since it is less relevant that we switched the design to use the gateway to decide which ports are free

tara-det-ai · 2024-06-17T19:02:15Z

docs/release-notes/gateway.rst

+-  Kubernetes: Introduced the Internal Task Gateway feature which allows Determined tasks running on
+   remote K8s clusters to be exposed to the Determined master and proxies. This feature supports
+   multi-resource manager setups by configuring a Gateway controller in the external K8s cluster.
+


:orphan:

New Features

Kubernetes: The Internal Task Gateway feature enables Determined tasks running on remote Kubernetes clusters to be exposed to the Determined master and proxies. This feature facilitates multi-resource manager setups by configuring a Gateway controller in the external Kubernetes cluster.

tara-det-ai · 2024-06-17T19:05:06Z

docs/release-notes/gateway.rst

+      Ensure that you have the necessary security measures in place to limit access to the exposed
+      tasks and secure communication between the external cluster and the main one. Recommended
+      measures include setting up a firewall, using a VPN, IP white-listing, K8s Network Policies,
+      or other security measures.


.. important::

Enabling this feature exposes Determined tasks to the outside world. It is crucial to implement appropriate security measures to restrict access to exposed tasks and secure communication between the external cluster and the main cluster. Recommended measures include:

Setting up a firewall

Using a VPN

Implementing IP whitelisting

Configuring Kubernetes Network Policies

Employing other security measures as needed

for a similar restructured text format, visit this release note: https://docs.determined.ai/latest/release-notes.html#version-0-30-0

tara-det-ai · 2024-06-17T19:07:46Z

docs/setup-cluster/k8s/controller-reviews.rst

+********
+
+https://docs.cilium.io/en/stable/network/servicemesh/gateway-api/gateway-api/#gs-gateway-api Based
+on Envoy.


For any https:// URL, use this format

Link text <https://url>_

e.g.,

Cilium Gateway API Support <https://docs.cilium.io/en/stable/network/servicemesh/gateway-api/gateway-api/#gs-gateway-api>_

tara-det-ai · 2024-06-17T19:09:16Z

docs/setup-cluster/k8s/install-on-kubernetes.rst

+   If you'd like to enable north-south access to Determined proxied tasks in external-to-master
+   clusters you'd need to set up a gateway as described in the docs :doc:`Internal Task Gateway
+   <internal-task-gateway>`
+


.. note::

To enable north-south access to Determined proxied tasks in external-to-master
clusters, set up a gateway as described in the docs :doc:Internal Task Gateway <internal-task-gateway>

tara-det-ai · 2024-06-17T19:12:36Z

docs/setup-cluster/k8s/internal-task-gateway.rst

+   firewall, using a VPN, IP white-listing, K8s Network Policies, native cloud solutions, or other
+   security measures. Some Determined tasks including JupyterLab Notebooks and shells already have
+   secure transport built-in. Tensorboards do not use TLS currently so a tunneling solution should
+   be considered.


Enabling this feature exposes Determined tasks to the outside world. It is crucial to implement appropriate security measures to restrict access to exposed tasks and secure communication between the external cluster and the main cluster. Recommended measures include:

Setting up a firewall

Using a VPN

Implementing IP whitelisting

Configuring Kubernetes Network Policies

Employing other security measures as needed

Some tasks including JupyterLab notebooks and shells already have secure transport built-in.
Since TensorBoards do not use TLS, you should consider a tunneling solution.

bump up slots per trial to 2 on a new set of tests

also rename internal exposeProxyConfig var

spell out kubernetes apply review comments minus https replace with annotated titles rename titles to just "Reference" update titles. add a bit about the 3 categories

molinamelendezj · 2024-06-18T14:45:29Z

.circleci/real_config.yml

@@ -3304,10 +3304,12 @@ jobs:
                  curl -LO https://storage.googleapis.com/minikube/releases/latest/minikube-linux-amd64 && sudo install minikube-linux-amd64 /usr/local/bin/minikube
            - run:
                name: Start defaultrm minikube
-                command: minikube start --profile defaultrm
+                command: |
+                  K8S_VERSION=1.29.5 source tools/k8s/launch-minikube-with-gateway.sh defaultrm


Any chance we can stamp this version at the top of file in case we need test other version later?

I pushed it up to the job level. I'm thinking we can push it higher up if/when we use it elsewhere?
Also I wasn't sure how those top-level parameters exactly work

molinamelendezj · 2024-06-18T14:45:52Z

.circleci/real_config.yml

            - run:
                name: Start additionalrm minikube
-                command: minikube start --profile additionalrm
+                command: minikube start --profile additionalrm --kubernetes-version 1.29.5


Same here so it's easy to update in one place

cla-bot bot added the cla-signed label Jun 4, 2024

determined-ci requested a review from a team June 10, 2024 18:13

determined-ci added the documentation Improvements or additions to documentation label Jun 10, 2024

NicholasBlaskey force-pushed the notebook_proxy_feature_branch_pods_to_jobs branch from b9c204f to f43e47d Compare June 12, 2024 14:53

hamidzr changed the title ~~Notebook proxy feature branch pods to jobs~~ feat: Notebook proxy feature branch pods to jobs Jun 12, 2024

NicholasBlaskey commented Jun 13, 2024

View reviewed changes

hamidzr reviewed Jun 13, 2024

View reviewed changes

hamidzr marked this pull request as ready for review June 14, 2024 18:19

hamidzr requested review from a team as code owners June 14, 2024 18:19

hamidzr requested review from kkunapuli, jgongd, molinamelendezj and carolinaecalderon and removed request for kkunapuli June 14, 2024 18:19

NicholasBlaskey changed the title ~~feat: Notebook proxy feature branch pods to jobs~~ feat: K8S allow proxying through Gateway Jun 17, 2024

hamidzr approved these changes Jun 17, 2024

View reviewed changes

carolinaecalderon approved these changes Jun 17, 2024

View reviewed changes

tara-det-ai self-requested a review June 17, 2024 18:49

NicholasBlaskey changed the title ~~feat: K8S allow proxying through Gateway~~ feat: K8s allow proxying through Gateway Jun 17, 2024

tara-det-ai reviewed Jun 17, 2024

View reviewed changes

NicholasBlaskey and others added 19 commits June 18, 2024 09:28

progress?

58a770e

tests passing

7280df9

working

c4f4ea9

cleanup

94ec435

hopefully this fixes this

d060fa4

chore: bump up default gw listeners to 128 (#9474)

030c0f3

test: cherry pick port registry tests (#9471)

133f060

bump up slots per trial to 2 on a new set of tests

Reattach gateways (#9481)

90cd7d4

docs: gateway docs part 1 (#9488)

3735597

also rename internal exposeProxyConfig var

test: split multi node and single node (#9494)

1c57856

chore: store notebook proxy info for reattach as annotations (#9502)

6427c5e

fix lint

be44b7e

docs: gateway doc changes p2 (#9499)

8589036

fix: mockery new fmt (#9508)

bce5cb7

chore: remove need for explicit singluar podPort tracking (#9506)

65d7458

chore: cleanup notebook proxy feature branch (#9512)

5a0ea92

add release notes

22bc0f3

apply doc comments

c466932

spell out kubernetes apply review comments minus https replace with annotated titles rename titles to just "Reference" update titles. add a bit about the 3 categories

fmt

d8403ac

NicholasBlaskey force-pushed the notebook_proxy_feature_branch_pods_to_jobs branch from 92f55e7 to d8403ac Compare June 18, 2024 13:29

NicholasBlaskey and others added 7 commits June 18, 2024 09:34

special characters

0c3b737

Carolina's feedback

0c9735e

add doc lint to release note

1e0c3ef

Anda's feedback

876bd41

hardcode test image

22dd401

fix image

131d0a6

set k8s version as a parameter

3a99b0c

molinamelendezj approved these changes Jun 18, 2024

View reviewed changes

hamidzr merged commit 3641bfc into main Jun 18, 2024
84 of 98 checks passed

hamidzr deleted the notebook_proxy_feature_branch_pods_to_jobs branch June 18, 2024 17:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: K8s allow proxying through Gateway #9469

feat: K8s allow proxying through Gateway #9469

NicholasBlaskey commented Jun 4, 2024 •

edited by hamidzr

Loading

netlify bot commented Jun 4, 2024 •

edited

Loading

codecov bot commented Jun 4, 2024 •

edited

Loading

NicholasBlaskey Jun 13, 2024

hamidzr Jun 13, 2024

hamidzr Jun 13, 2024

hamidzr Jun 13, 2024

carolinaecalderon left a comment

carolinaecalderon Jun 17, 2024

carolinaecalderon Jun 17, 2024

hamidzr Jun 18, 2024 •

edited

Loading

carolinaecalderon Jun 17, 2024

NicholasBlaskey Jun 18, 2024

tara-det-ai Jun 17, 2024

tara-det-ai Jun 17, 2024

tara-det-ai Jun 17, 2024

tara-det-ai Jun 17, 2024 •

edited

Loading

tara-det-ai Jun 17, 2024

tara-det-ai Jun 17, 2024

molinamelendezj Jun 18, 2024

hamidzr Jun 18, 2024

molinamelendezj Jun 18, 2024

feat: K8s allow proxying through Gateway #9469

feat: K8s allow proxying through Gateway #9469

Conversation

NicholasBlaskey commented Jun 4, 2024 • edited by hamidzr Loading

Ticket

Description

Test Plan

Checklist

netlify bot commented Jun 4, 2024 • edited Loading

✅ Deploy Preview for determined-ui canceled.

codecov bot commented Jun 4, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carolinaecalderon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hamidzr Jun 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tara-det-ai Jun 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NicholasBlaskey commented Jun 4, 2024 •

edited by hamidzr

Loading

netlify bot commented Jun 4, 2024 •

edited

Loading

codecov bot commented Jun 4, 2024 •

edited

Loading

hamidzr Jun 18, 2024 •

edited

Loading

tara-det-ai Jun 17, 2024 •

edited

Loading