jupyter-web-app's `PodDefault` `configurations` are keyed by their label selector's key, not their name

[ca-scribner]

/kind bug

What steps did you take and what happened: The Notebook spawner page is configured using the spawner_ui_config.yaml input file. The configurations section lets an admin define which PodDefaults should be selected by default. spawner_ui_config.yaml's comments state:

the list of PodDefault names that are selected by default

If I create a PodDefault named example:

apiVersion: kubeflow.org/v1alpha1
kind: PodDefault
metadata:
 name: example
spec:
 desc: Example PodDefault
 env:
 - name: SOMETHING
   value: stuff
 selector:
   matchLabels:
     example-match-selector: "true"

then set the spawner_ui_config.yaml's configurations to:

  configurations:
    readOnly: false

    # the list of PodDefault names that are selected by default
    # (take care to ensure these PodDefaults exist in Profile Namespaces)
    value:
    - example

I do not see it selected in the spawner UI by default:

What did you expect to happen:

When specifying PodDefaults by name in the spawner_ui_config.yaml, I expect them to be selected by default on the spawner page:

Anything else you would like to add:

I think this is a bug in the jwa backend's get_poddefaults(). get_poddefaults() returns a result where the label field is set as the key of the 0th matchLabels selector:

https://github.com/kubeflow/kubeflow/blob/bd7f250df22e144b114177536309d28651b4ddbb/components/crud-web-apps/jupyter/backend/apps/common/routes/get.py#L29-L49

This can be confirmed by setting

  configurations:
    readOnly: false

    # the list of PodDefault names that are selected by default
    # (take care to ensure these PodDefaults exist in Profile Namespaces)
    value:
    - example-match-selector

And reloading/refreshing the spawner page:

I think this should probably instead be:

        label = pd["metadata"]["name"]  # haven't compiled this - might have typo'd something

Environment:

• Kubeflow version: (version number can be found at the bottom left corner of the Kubeflow dashboard): 1.8
• kfctl version: (use kfctl version):
• Kubernetes platform: (e.g. minikube)
• Kubernetes version: (use kubectl version):
• OS (e.g. from /etc/os-release):

[ca-scribner]

This bug is also the cause of kubeflow/notebooks#99, which identified how a the jwa fails to handle PodDefaults that have no matchLabels selector

[ca-scribner]

This bug appears to also affect the tensorboard web app backend, which has a copy of the same get_poddefaults()

[ca-scribner]

I don't understand typescript enough to be certain that the label is intended to be the name of the PodDefault, but given the comments in the spawner_ui_config.yaml I think that's probably true

[boarder7395]

@ca-scribner I'm looking at the code in a little more detail and I see what you're saying. I think both our solutions are not optimal though.

This logic uses the matchLabel key to set a the label on the notebook to true. https://github.com/kubeflow/kubeflow/blob/bd7f250df22e144b114177536309d28651b4ddbb/components/crud-web-apps/jupyter/backend/apps/common/form.py#L253-L261

For this reason I think we need to use the label of the poddefault instead of the name of the poddefault.

BUT I'm not sure this should be the actual logic. Lets take a look at the issue with my solution . Consider the pod default

metadata:
  name: example
spec:
  annotations:
    karpenter.sh/do-not-disrupt: "true"
  selector:
    matchExpressions:
    - key: some-key
      operator: NotIn
      values: ['true']

In this case the configuration should be "some-key" and selecting that configuration in the dropdown would add the "some-key" label to the notebook. Although due to the matchExpressions that is doing the exact opposite of what we want.

I'm currently at a wall with matchExpressions because what we really want to do use this selector to create a label that conforms to the matchExpressions requirement.

[boarder7395]

@thesuperzapper Do you have any input on the above

[andreyvelich]

/transfer notebooks