Central Dashboard and Jupyter Web App manifests don't use configmap

[davidspek]

/kind bug

The central-dashboard manifests create a configmap named centraldashboard-parameters which contain the values for the USERID_HEADER, USERID_PREFIX, PROFILES_KFAM_SERVICE_HOST, REGISTRATION_FLOW and DASHBOARD_LINKS_CONFIGMAP. However, this configmap is never mounted to the deployment, so changing one of this values in the configmap does not result in the expected.

The current setup seems to be replacing the values in the deployment.yaml directly using the values in the params.env file directly. This seems to be done by https://github.com/kubeflow/kubeflow/blob/5487b167738d5e11755b38c2f93dde78486fbf18/components/centraldashboard/manifests/base/kustomization.yaml#L28, which is planned to be removed from kustomize in the future.

The deployment manifest should be changed to mount the centraldashboard-parameters configmap and use the values in there instead.

/cc @kubeflow/wg-notebooks-leads

[davidspek]

This is also the happening with the Jupyter Web App.

[yanniszark]

I would classify this as a feature, not a bug.
Yes, manifests are structured in an older way that uses vars and we want to move away from that.
But this is an improvement, not a bug.

[davidspek]

A configmap is being deployed that is meant to configure the application, but it is never used in the deployment. I'd say this is probably one of the simplest examples of a bug in kustomize manifests. If having manifests function as expected is a feature, why would we even bother with providing manifests?

[martialblog]

Hey, I just ran into the same issue. It's quite confusing if parameters occur multiple times. This pattern seems strange to me and is very unintuitive from a Kubernetes point-of-view.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[juliusvonkohout]

@davidspek is this still relevant?

[mvoitko]

@juliusvonkohout it does for Kubeflow 1.5. The question is whether we want to move to using kustomize params (we just need to delete config-map generation) or to ConfigMaps (we need to add reading values from ConfigMap in the Deployment). Let's confirm the direction and I could make the changes.

[juliusvonkohout]

@kubeflow/wg-notebooks-leads can we move forward with this? There is already a PR for the centraldashboard at https://github.com/kubeflow/kubeflow/pull/6072/files

[martialblog]

Let me know if I should rebase/refactor the PR.

[andreyvelich]

/transfer dashboard

[google-oss-prow]

@davidspek: The label(s) kind/bug cannot be applied, because the repository doesn't have them.

In response to this:

/kind bug

The central-dashboard manifests create a configmap named centraldashboard-parameters which contain the values for the USERID_HEADER, USERID_PREFIX, PROFILES_KFAM_SERVICE_HOST, REGISTRATION_FLOW and DASHBOARD_LINKS_CONFIGMAP. However, this configmap is never mounted to the deployment, so changing one of this values in the configmap does not result in the expected.

The current setup seems to be replacing the values in the deployment.yaml directly using the values in the params.env file directly. This seems to be done by https://github.com/kubeflow/kubeflow/blob/5487b167738d5e11755b38c2f93dde78486fbf18/components/centraldashboard/manifests/base/kustomization.yaml#L28, which is planned to be removed from kustomize in the future.

The deployment manifest should be changed to mount the centraldashboard-parameters configmap and use the values in there instead.

/cc @kubeflow/wg-notebooks-leads

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.