Skip to content

chore: make dashboard release-able (part 1)#115

Merged
google-oss-prow[bot] merged 1 commit intokubeflow:mainfrom
yehuditkerido:fix_migration_images_path
Sep 12, 2025
Merged

chore: make dashboard release-able (part 1)#115
google-oss-prow[bot] merged 1 commit intokubeflow:mainfrom
yehuditkerido:fix_migration_images_path

Conversation

@yehuditkerido
Copy link
Copy Markdown
Contributor

@yehuditkerido yehuditkerido commented Jul 8, 2025

Migrate Docker Images from kubeflow/kubeflow to kubeflow/dashboard

Migrates all dashboard component images from ghcr.io/kubeflow/kubeflow/* to ghcr.io/kubeflow/dashboard/* and standardizes component naming.

Key Changes

Image Path Updates:

  • central-dashboard → dashboard
  • centraldashboard-angular → dashboard-angular
  • kfam → access-management
  • admission-webhook → poddefaults-webhook
  • profile-controller → profile-controller

Updated Files:

  • GitHub CI/CD workflows
  • Makefiles and Kubernetes manifests
  • Integration tests and deployment scripts

@yehuditkerido yehuditkerido changed the title Update images paths from ghcr.io/kubeflow/kubeflow/* to ghcr.io/kubef… ci: Update images paths from ghcr.io/kubeflow/kubeflow/* to ghcr.io/kubef… Jul 8, 2025
@andyatmiami
Copy link
Copy Markdown
Contributor

/retest

@kimwnasptd
Copy link
Copy Markdown
Member

kimwnasptd commented Jul 8, 2025

I think the problem here is that, in the current tests, the KFAM image is never built for the integration-tests tag, as it was in the past.

Specifically, IIUC, what is going on is:

  1. The profiles-kfam-integration test gets triggered
  2. The test installs Profiles Manifests by using the deploy_component.sh script
  3. That script builds the image only for the Profiles Controller
  4. KFAM never gets built, and the tests are trying to pull an image that doesn't exist

This wasn't the case in the past, as tests were building both KFAM and Profiles Controller.

IIUC, right now even if we manually publish a ghcr.io/kubeflow/dashboard/kfam:integration-tests image, then the tests would always be using this image and will be never building and using a KFAM image as they used to.

(But LMK if I'm missing something)

@yehuditkerido yehuditkerido force-pushed the fix_migration_images_path branch 3 times, most recently from 6b41308 to 0f395bc Compare July 10, 2025 06:38
@google-oss-prow google-oss-prow Bot added size/L and removed size/M labels Jul 10, 2025
@yehuditkerido yehuditkerido force-pushed the fix_migration_images_path branch from 0f395bc to 6660d1e Compare July 10, 2025 07:09
@kimwnasptd
Copy link
Copy Markdown
Member

So indeed, looking at the updated code, this was the issue.

@yehudit1987 it looks like now the tests are failing because the CentralDashboard gets deployed with the e2e-test tag that doesn't exist. So we'll need to do the same and re-build this image as well.

Could you try to do the changes just to confirm that this is the case?

It looks like #101 has broken the CI and how it was working in the past and we'll need to make modifications. But again, let's first confirm the above.

@yehuditkerido yehuditkerido force-pushed the fix_migration_images_path branch 2 times, most recently from f090404 to 04ff8ae Compare July 10, 2025 09:12
@yehuditkerido yehuditkerido marked this pull request as draft July 10, 2025 09:39
@yehuditkerido yehuditkerido force-pushed the fix_migration_images_path branch 2 times, most recently from 52cba39 to 4f20b8c Compare July 10, 2025 12:04
@yehuditkerido yehuditkerido marked this pull request as ready for review July 10, 2025 12:39
@andyatmiami
Copy link
Copy Markdown
Contributor

/ok-to-test

@yehuditkerido
Copy link
Copy Markdown
Contributor Author

legacy/invalid(?) files

cloudbuild.yaml

I think we should remove the cloudbuild.yaml files present in the following locations:

  • components/centraldashboard/cloudbuild.yaml
  • components/access-management/cloudbuild.yaml
  • components/profile-controller/cloudbuild.yaml

FYI: @kimwnasptd @juliusvonkohout @thesuperzapper

This seems to be a (very) outdated CI/CD artifact related to GCB that is no longer required/leveraged. As our CI/CD now leverages ghcr - would be good to remove these files files entirely to avoid any future confusion. As the files also construct image name references - its particularly relevant to this PR.

make-win.ps1

In a similar vein - I'm not sure what to do about this make-win.ps1 file that contains an $IMG reference using the gcr.io repo as well as the now outdated image name:

This gcr.io kubeflow-images-public registry contains a lot of images that were created 5+ years ago - but interestingly seemed to be updated near on the onset of 2025 🤔

README.md

And finally, the README.md is similarly incorrect/outdated for centraldashboard with references to gcr.io as well as label selectors that are or will be invalid upon merge of this PR

Regarding that -

  1. I deleted the cloudbuild files since they do seems to be redundant and not relevant.
  2. As for the make-win.ps1 I updated the refences in it to the correct repo (for cases of windows users). If not needed at all it can be deleted but it's not my decision to make.
  3. I updated the README.md file.

- name: ghcr.io/kubeflow/kubeflow/central-dashboard
newName: ghcr.io/kubeflow/kubeflow/central-dashboard
- name: ghcr.io/kubeflow/dashboard/dashboard
newName: ghcr.io/kubeflow/dashboard/dashboard
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to carry out the centraldashboard -> dashboard rename consistently throughout this entire file...

There are many references to centraldashboard in this file - so I will not comment on each of them... but do please perform the rename - thanks!

Copy link
Copy Markdown
Contributor

@andyatmiami andyatmiami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to update this configmap code from the application source:

image

Copy link
Copy Markdown
Contributor

@andyatmiami andyatmiami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to make sure the components/centraldashboard k8s manifest files are consistently using the dashboard name (as opposed to centraldashboard) to be consistent with the rest of the changes we are introducing.

image

env:
DASHBOARD_IMG: ghcr.io/kubeflow/dashboard/central-dashboard
KFAM_IMG: ghcr.io/kubeflow/dashboard/kfam
DASHBOARD_IMG: ghcr.io/kubeflow/dashboard/dashboard
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are numerous references in this file that still pass a label selector (or an argument to a script that is consumed as a label selector) that is using the centraldashboard name..

we should update all such occurrences to dashboard

env:
CENTRALDASHBOARD_IMG: ghcr.io/kubeflow/dashboard/centraldashboard
CENTRALDASHBOARD_ANGULAR_IMG: ghcr.io/kubeflow/dashboard/centraldashboard-angular
CENTRALDASHBOARD_IMG: ghcr.io/kubeflow/dashboard/dashboard
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are numerous references in this file that still pass a label selector (or an argument to a script that is consumed as a label selector) that is using the centraldashboard name..

we should update all such occurrences to dashboard

Comment thread components/centraldashboard/README.md Outdated
Comment on lines 26 to 27
kubectl --record deployment.apps/centraldashboard \
set image deployment.v1.apps/centraldashboard \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need up update this reference to the deployment name to dashboard to align as part of other PR review comments left.

Copy link
Copy Markdown
Contributor

@andyatmiami andyatmiami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As part of these changes - I think we will ALSO need to update go.mod and referenced imports for the go-based components:

  • access-management
  • poddefaults-webhooks
  • profile-controller

So references such as this github.com/kubeflow/kubeflow/components/access-management should actually be github.com/kubeflow/dashboard/components/access-management

This naturally then applies to actual import declarations as well.

go mod tidy can then help validate these changes.

@yehuditkerido yehuditkerido force-pushed the fix_migration_images_path branch from ecda377 to ec192b6 Compare August 27, 2025 07:54
@yehuditkerido yehuditkerido force-pushed the fix_migration_images_path branch 3 times, most recently from ced864f to dcd390b Compare September 3, 2025 09:17
@juliusvonkohout
Copy link
Copy Markdown
Member

juliusvonkohout commented Sep 4, 2025

@yehudit1987 @andyatmiami can we merge this soon, even if it i s not perfect ? Because we are blocking many other PRs as for example #135 (comment) and #133

I am willing to approve, because we need to move forward and get a release and i do not see activity from other maintainers.

@juliusvonkohout
Copy link
Copy Markdown
Member

@yehudit1987 you can also invite @andyatmiami in git to collaborate in your branch/repository

@yehuditkerido
Copy link
Copy Markdown
Contributor Author

yehuditkerido commented Sep 7, 2025

@yehudit1987 @andyatmiami can we merge this soon, even if it i s not perfect ? Because we are blocking many other PRs as for example #135 (comment) and #133

I am willing to approve, because we need to move forward and get a release and i do not see activity from other maintainers.

Hi @juliusvonkohout and @andyatmiami as for today status is - I finished checking testing logs and fixing issues I saw. Hopefully I didn't miss anything. I believe we can merge it as is (if review is passing of course). If any other issues are remain - such as logs I miss or changing imports at go files etc. I suggest to open separate tasks for that.

@andyatmiami
Copy link
Copy Markdown
Contributor

I just want to test actually publishing from my fork tomorrow...

Assuming that goes well - I plan on giving this an lgtm with the understanding we can always fix edge cases if they crop up after the fact. Tests have been passing for awhile - and I'm reasonably confident this last pass @yehudit1987 did would have cleared up any of the potentially problematic bits. 💯

@yehuditkerido
Copy link
Copy Markdown
Contributor Author

yehuditkerido commented Sep 8, 2025

I just want to test actually publishing from my fork tomorrow...

Assuming that goes well - I plan on giving this an lgtm with the understanding we can always fix edge cases if they crop up after the fact. Tests have been passing for awhile - and I'm reasonably confident this last pass @yehudit1987 did would have cleared up any of the potentially problematic bits. 💯

Hi @andyatmiami the publishing workflows are working as well.
image

…low/dashboard/*

Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com>
@andyatmiami
Copy link
Copy Markdown
Contributor

andyatmiami commented Sep 8, 2025

/lgtm

⚠️ We may want to sync with @thesuperzapper prior to merging this PR to make sure the permissions of the kubeflow/dashboard container registry are matching expectations.

In addition to the green checks across the board on this PR, I verified on my fork that publishing of images appears to be appropriate...

GHAs

kfam -> access-management

admission-webhook -> poddefaults-webhook

centraldashboard-angular -> dashboard-angular

centraldashboard -> dashboard

profile-controller -> profile-controller

Kind verification

➜ scripts/ git:(experimental/remote-deployment) $ kubectl get deployments -n kubeflow -o json | jq -r '.items[] | "\(.metadata.name): \(.spec.template.spec.containers[].image)"'

admission-webhook-deployment: ghcr.io/andyatmiami/kubeflow-dashboard/poddefaults-webhook:2.0.0-dev-verify
centraldashboard: ghcr.io/andyatmiami/kubeflow-dashboard/dashboard:2.0.0-dev-verify
profiles-deployment: ghcr.io/andyatmiami/kubeflow-dashboard/access-management:2.0.0-dev-verify
profiles-deployment: ghcr.io/andyatmiami/kubeflow-dashboard/profile-controller:2.0.0-dev-verify
  • Can observe I updated a minimal kind cluster that I have for NBv2 development with the images that I published above

➜ scripts/ git:(experimental/remote-deployment) $ kubectl get deployment,pod -A

NAMESPACE                     NAME                                                      READY   UP-TO-DATE   AVAILABLE   AGE
auth                          deployment.apps/dex                                       1/1     1            1           33m
cert-manager                  deployment.apps/cert-manager                              1/1     1            1           34m
cert-manager                  deployment.apps/cert-manager-cainjector                   1/1     1            1           34m
cert-manager                  deployment.apps/cert-manager-webhook                      1/1     1            1           34m
istio-system                  deployment.apps/istio-ingressgateway                      1/1     1            1           33m
istio-system                  deployment.apps/istiod                                    1/1     1            1           33m
kube-system                   deployment.apps/coredns                                   2/2     2            2           34m
kubeflow-workspaces           deployment.apps/workspaces-backend                        1/1     1            1           30m
kubeflow-workspaces           deployment.apps/workspaces-frontend                       1/1     1            1           26m
kubeflow                      deployment.apps/admission-webhook-deployment              1/1     1            1           9m44s
kubeflow                      deployment.apps/centraldashboard                          1/1     1            1           33m
kubeflow                      deployment.apps/profiles-deployment                       1/1     1            1           32m
local-path-storage            deployment.apps/local-path-provisioner                    1/1     1            1           34m
oauth2-proxy                  deployment.apps/oauth2-proxy                              2/2     2            2           33m
workspace-controller-system   deployment.apps/workspace-controller-controller-manager   1/1     1            1           31m

NAMESPACE                     NAME                                                           READY   STATUS    RESTARTS   AGE
auth                          pod/dex-dd4df85fb-sfjmr                                        1/1     Running   0          33m
cert-manager                  pod/cert-manager-5f864bbfd-78kl5                               1/1     Running   0          34m
cert-manager                  pod/cert-manager-cainjector-589dc747b5-n62l7                   1/1     Running   0          34m
cert-manager                  pod/cert-manager-webhook-5987c7ff58-w77w8                      1/1     Running   0          34m
istio-system                  pod/istio-ingressgateway-68f85c766d-nx2v2                      1/1     Running   0          33m
istio-system                  pod/istiod-84c9d58b6f-c6bh6                                    1/1     Running   0          33m
kube-system                   pod/coredns-668d6bf9bc-69v9s                                   1/1     Running   0          34m
kube-system                   pod/coredns-668d6bf9bc-9glp9                                   1/1     Running   0          34m
kube-system                   pod/etcd-kubeflow-control-plane                                1/1     Running   0          34m
kube-system                   pod/istio-cni-node-xk26s                                       1/1     Running   0          33m
kube-system                   pod/kindnet-p47mf                                              1/1     Running   0          34m
kube-system                   pod/kube-apiserver-kubeflow-control-plane                      1/1     Running   0          34m
kube-system                   pod/kube-controller-manager-kubeflow-control-plane             1/1     Running   0          34m
kube-system                   pod/kube-proxy-n7kzh                                           1/1     Running   0          34m
kube-system                   pod/kube-scheduler-kubeflow-control-plane                      1/1     Running   0          34m
kubeflow-workspaces           pod/workspaces-backend-c98b96555-tzmql                         2/2     Running   0          30m
kubeflow-workspaces           pod/workspaces-frontend-787f75ff98-8nf57                       2/2     Running   0          26m
kubeflow                      pod/admission-webhook-deployment-85c6b4954c-p9plw              1/1     Running   0          8m42s
kubeflow                      pod/centraldashboard-84997d7f9c-56258                          2/2     Running   0          21m
kubeflow                      pod/profiles-deployment-89fdb7754-wtpzt                        3/3     Running   0          19m
local-path-storage            pod/local-path-provisioner-58cc7856b6-v4c8w                    1/1     Running   0          34m
oauth2-proxy                  pod/oauth2-proxy-c696dfff5-lxdtp                               1/1     Running   0          33m
oauth2-proxy                  pod/oauth2-proxy-c696dfff5-zmckt                               1/1     Running   0          33m
workspace-controller-system   pod/workspace-controller-controller-manager-5b64c78667-59mbw   1/1     Running   0          31m
  • cluster is happy/healthy with all workloads running and available
image - UI loads appropriately

@juliusvonkohout
Copy link
Copy Markdown
Member

I can also check the permissions as administrator if needed. I am in favor of just merging and then adjusting permissions if needed, but let's give @kimwnasptd and @thesuperzapper a day for feedback.

@juliusvonkohout
Copy link
Copy Markdown
Member

/approve
As explained above

@google-oss-prow
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: juliusvonkohout

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants