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

Notebooks 2.0 // Controller // Implement VirtualService reconciliation #37

Open
thesuperzapper opened this issue Aug 29, 2024 · 5 comments · May be fixed by #42
Open

Notebooks 2.0 // Controller // Implement VirtualService reconciliation #37

thesuperzapper opened this issue Aug 29, 2024 · 5 comments · May be fixed by #42
Assignees
Labels
area/controller area - related to controller components project/notebooks-v2 project - kubeflow notebooks v2

Comments

@thesuperzapper
Copy link
Member

thesuperzapper commented Aug 29, 2024

related #85

Now that #34 is merged, there are only a small number of remaining tasks for the "Kubeflow Workspaces Controller to be finished.

One task to finish the workspace controller loop, so it also reconciles the Istio VirtualServices required to make the Workspace Pods accessible on the Istio mesh

Implementation

Here is the TODO in the code:

//
// TODO: reconcile the Istio VirtualService to expose the Workspace
// and implement the `spec.podTemplate.httpProxy` options
//

VirtualService Definition

The structure of the HTTP paths exposed by each Workspace should be: /workspace/<NAMESPACE_NAME>/<WORKSPACE_NAME>/<PORT_ID>/

Where PORT_ID is the id of a port defined in the currently selected spec.podTemplate.options.imageConfig, which is defined in the WorkspaceKind under spec.podTemplate.options.imageConfig.values[].spec.ports

Note, this path structure is already reflected in the httpPathPrefix function for the spec.podTemplate.extraEnv[0].value go templates (which should probably be factored into a helper):

// define go string template functions
// NOTE: these are used in places like the `extraEnv` values
containerPortsIdMap := make(map[string]kubefloworgv1beta1.ImagePort)
httpPathPrefixFunc := func(portId string) string {
port, ok := containerPortsIdMap[portId]
if ok {
return fmt.Sprintf("/workspace/%s/%s/%s/", workspace.Namespace, workspace.Name, port.Id)
} else {
return ""
}
}

Example VirtualService

Here is a templated example of the VirtualService we should reconcile:

apiVersion: networking.istio.io/v1
kind: VirtualService
metadata:
  ## for reference, see the existing `generateNamePrefix()` usage for `Service` and `StatefulSet` generation
  name: "ws-<WORKSPACE_NAME>-<RANDOM_SUFFIX>"
  namespace: "<PROFILE_NAME>"

## NOTE: see the full VirtualService spec here:
##       https://istio.io/latest/docs/reference/config/networking/virtual-service/
spec:

  gateways:
    ## read this from an environment variable called `ISTIO_GATEWAY`, set on the controller
    ## DEFAULT: "kubeflow/kubeflow-gateway"
    - "<ISTIO_GATEWAY>"

  hosts:
    ## read this from an environment variable called `ISTIO_HOST`, set on the controller
    ## DEFAULT: "*"
    - "<ISTIO_HOST>"

  http:

    ## there should be an element for EACH of the `ports` in the Workspace currently selected `imageConfig`
    - match:
        - uri:
            prefix: "/workspace/<PROFILE_NAME>/<WORKSPACE_NAME>/<PORT_ID>/"

      ## if `httpProxy.removePathPrefix` is true, include this block. otherwise, remove it
      rewrite:
        uri: "/"

      headers:

        ## implement any `httpProxy.requestHeaders` manipulation here
        ## DOCS: https://istio.io/latest/docs/reference/config/networking/virtual-service/#Headers-HeaderOperations
        request:

          ## NOTE: the default is {}, this is just an example
          ## NOTE: this turns into "/workspace/{PROFILE_NAME}/{WORKSPACE_NAME}/{PORT_ID}/" when rendered
          ## NOTE: implement go templates the same as `httpPathPrefix` used in the `spec.podTemplate.extraEnv[].value`
          ## WARNING: remember to add a new Webhook validation for this field (and `add`) to ensure it's a valid Go template
          set:
            X-RStudio-Root-Path: |-
              {{ httpPathPrefix "PORT_ID" }}

          add: {}

          remove: []

      route:
        - destination:
            ## read CLUSTER_DOMAIN from an environment variable called `CLUSTER_DOMAIN`, set on the controller
            ## DEFAULT: "cluster.local"
            host: "<SERVICE_NAME>.<PROFILE_NAME>.svc.<CLUSTER_DOMAIN>"
            port:
              number: <PORT_NUMBER>

CRDs

Besides making the Pod available, the VirtualService will also implement the spec.podTemplate.ports[].httpProxy part of the WorkspaceKind spec:

spec:
  podTemplate:

    ## port configs (MUTABLE)
    ##
    ports:

      ## configs to apply to a specific port
      ##  - ports are identified by their `ports[].id` from the `imageConfig` spec
      ##
      - portId: "jupyterlab"

        ## httpProxy configs for the port
        ##  - note, only "HTTP" protocol ports are supported
        ##
        httpProxy:

          ## if the HTTP path prefix should be removed
          ##  - if true, the '/workspace/{PROFILE_NAME}/{WORKSPACE_NAME}/{PORT_ID}' path prefix
          ##    will be stripped from incoming requests
          ##  - that is, the application sees the request as if it was made to '/...'
          ##  - this only works if the application serves RELATIVE URLs for its assets
          ##
          removePathPrefix: false

          ## header manipulation rules for incoming HTTP requests
          ##  - sets the `spec.http[].headers.request` of the Istio VirtualService
          ##    https://istio.io/latest/docs/reference/config/networking/virtual-service/#Headers-HeaderOperations
          ##  - the following go template functions are available:
          ##      - `httpPathPrefix(portId string)`: returns the HTTP path prefix of the specified port (regardless of `removePathPrefix`)
          ##
          requestHeaders: {}
            #set: { "X-RStudio-Root-Path": "{{ httpPathPrefix 'rstudio' }}" } # example for RStudio
            #add: {}
            #remove: []

Changes from current CRDs

After designing and discussing this implementation, we figured that we need to make some small additions to the existing Notebooks 2.0 CRDs:

  • WorkspaceKind:
    • Remove the spec.podTemplate.httpProxy map, and replace it with spec.podTemplate.ports list.
    • Each element of spec.podTemplate.ports will be a map with the following keys:
      • portId: the map-index of this array and determines which port the configs are applied to
      • httpProxy: the same as the old ones, but now scoped to a specific port.
@thesuperzapper
Copy link
Member Author

@jiridanek @Adembc, @kimwnasptd, or @ederign you might be interested in picking this up.

@thesuperzapper thesuperzapper added the project/notebooks-v2 project - kubeflow notebooks v2 label Aug 29, 2024
@thesuperzapper
Copy link
Member Author

@jiridanek as discussed in the Notebook WG meeting, I have added some better examples of what the VirtualService should look like.

I have also realized that we will need to change a few things about the WorkspaceKind CRD spec, namely that the spec.podTemplate.httpProxy map needs to be replaced with spec.podTemplate.ports list, so different proxy configs can be applied to each port. (See above issue for more info).

@thesuperzapper thesuperzapper added this to the v2.0.0-alpha.0 milestone Sep 3, 2024
@thesuperzapper thesuperzapper added the area/controller area - related to controller components label Sep 3, 2024
@thesuperzapper thesuperzapper moved this from Needs Triage to In Progress in Kubeflow Notebooks Sep 3, 2024
@thesuperzapper thesuperzapper moved this from In Progress to In Discussion in Kubeflow Notebooks Sep 3, 2024
@mcruzdev
Copy link

mcruzdev commented Sep 4, 2024

Hi @thesuperzapper could you assign it to me?

@jiridanek
Copy link
Member

I'll be on vacation for the next week, so, all the best to @mcruzdev!

@mcruzdev
Copy link

mcruzdev commented Sep 6, 2024

Thank you @jiridanek, good vacaction for you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/controller area - related to controller components project/notebooks-v2 project - kubeflow notebooks v2
Projects
Status: In Discussion
Development

Successfully merging a pull request may close this issue.

3 participants