skai-x / elastic-jupyter-operator

Cloud-native way to provide elastic Jupyter Notebooks on Kubernetes. Run remote kernels, natively.
Apache License 2.0
194 stars 28 forks source link

Customized Labels & Annotations are not supported in notebook CRD refered pod & deployment #59

Closed mkkb473 closed 2 years ago

mkkb473 commented 2 years ago

Behavior: kubectl apply the following yaml:

apiVersion: kubeflow.tkestack.io/v1alpha1
kind: JupyterNotebook
metadata:
  name: jupyternotebook-elastic-with-custom-kernels
  namespace: public
spec:
  gateway:
    name: jupytergateway-elastic-with-custom-kernels
    namespace: public
  # Disable the password and token based auth in this example,
  # please do not do it in PROD.
  auth:
    mode: disable
  template:
    metadata:
      annotations:
        cni: macvlan 

get the following:

apiVersion: v1
items:
- apiVersion: kubeflow.tkestack.io/v1alpha1
  kind: JupyterNotebook
  metadata:
    annotations:
      cni: macvlan
    creationTimestamp: "2022-03-21T09:50:31Z"
    generation: 3
    name: jupyternotebook-elastic
    namespace: public
    resourceVersion: "152659678"
    selfLink: /apis/kubeflow.tkestack.io/v1alpha1/namespaces/public/jupyternotebooks/jupyternotebook-elastic
    uid: 12d437c1-45e5-4afa-99bf-38371533046e
  spec:
    auth:
      mode: disable
    gateway:
      name: jupytergateway-elastic
      namespace: public
    template:
      metadata: {}
kind: List
metadata:
  resourceVersion: ""
  selfLink: ""

Manually Editing the jupyternotebook to add spec.template.metadata.annotations does not work either.

Found that spec.template.metadata.annotations is never used/considered in reconciling deloyment: https://github.com/skai-x/elastic-jupyter-operator/blob/4415a90f4fcb48341b7aff5587aeed3fc48cb771/pkg/notebook/reconcile.go#L71

https://github.com/skai-x/elastic-jupyter-operator/blob/0d1472bf97697e2cf50d215d6266d9a92f8898e3/pkg/notebook/generate.go#L116 https://github.com/skai-x/elastic-jupyter-operator/blob/0d1472bf97697e2cf50d215d6266d9a92f8898e3/pkg/notebook/generate.go#L125

gaocegege commented 2 years ago

Good catch! Would you mind creating a PR to add it? Or I can fix it later.

mkkb473 commented 2 years ago

Good catch! Would you mind creating a PR to add it? Or I can fix it later.

I'll fix it :)

gaocegege commented 2 years ago

Thanks! Please assign me to review if the PR is ready

Thanks for your contribution! :tada: :+1: