feat: Migrate JWA test-related workflows from kubeflow/kubeflow to notebooks-v1 branch #587 #599

yehudit1987 · 2025-09-15T06:14:48Z

feat: migrate Jupyter Web App tests to notebooks repo

Add testing infrastructure scripts (kind, kustomize, istio)
Update test workflows to use ghcr.io/kubeflow/notebooks/jupyter-web-app
Update JWA component manifests and Makefile for new registry

Enables JWA tests to run in kubeflow/notebooks repository.

Solves issue #587

andyatmiami

Thank you for the contribution and getting the release process started for notebooks-v1!

I think we still need some work on this PR - primarily around communication on why some of these changes are necessary and deviate from how kubeflow/kubeflow was set up.

I am not implying (necessarily) that they are wrong - only that there is no explanation on this PR to be able to understand why the proposed changes were required.

andyatmiami · 2025-09-19T18:02:27Z

components/crud-web-apps/jupyter/Makefile

 docker-build-multi-arch: ##  Build multi-arch docker images with docker buildx
 	cd ../ && docker buildx build --load --platform ${ARCH} --tag ${IMG}:${TAG} -f ${DOCKERFILE} .

+.PHONY: docker-build-multi-arch-verify


can you explain why we needed to add this Makefile target? Why was this not needed in kubeflow/kubeflow but is now needed in kubeflow/notebooks?

This is the kind of thing I would have expected to be called out in the commit message and/or PR description - but I don't see any reference to it...

Tagging each architecture separately would be a breaking change for customers - and I don't think "sneaking that in" in is something I could "sign off on" without discussion from community.

Ok so as I mention, there where issues with ARM64 and I did of course open my proposal to discussion.

This target was added only for this scenario (and not instead of) and it helps to validate what is happening with the problematic image.
Now as I mention, most of the time (if not all) I can't fail the build step since not sure the image build is failing, it just get stuck.

I'm not sue why it's happens here and not in kubeflow/kubeflow but from what I have read it might be related to differences in runners. Maybe the experts from the community can put some light on this issue.

Yep, apologies - I had missed your discussion comment on the PR!

andyatmiami · 2025-09-19T18:09:21Z

.github/workflows/jwa_multi_arch_test.yaml

+    - name: Verify multi-arch builds
+      run: |
+        echo "Verifying multi-arch build results:"
+        echo "Listing all jupyter-web-app images:"
+        docker images --format "table {{.Repository}}\t{{.Tag}}\t{{.Size}}\t{{.CreatedAt}}" | grep jupyter-web-app || echo "No jupyter-web-app images found"
+        
+        echo ""
+        echo "Checking individual architectures:"
+        TAG=$(git describe --tags --always --dirty)
+        echo "Base TAG: $TAG"
+        
+        # Check each architecture
+        if docker images | grep -q "jupyter-web-app.*${TAG}-linux-amd64"; then
+          echo "✅ AMD64: SUCCESS"
+        else
+          echo "❌ AMD64: FAILED"
+        fi
+        
+        if docker images | grep -q "jupyter-web-app.*${TAG}-linux-ppc64le"; then
+          echo "✅ PPC64LE: SUCCESS"
+        else
+          echo "❌ PPC64LE: FAILED"
+        fi
+        
+        if docker images | grep -q "jupyter-web-app.*${TAG}-linux-arm64-v8"; then
+          echo "✅ ARM64: SUCCESS"
+        else
+          echo "ℹ️  ARM64: Not built (expected if timeout occurred)"
+        fi


Related to my comment on https://github.com/kubeflow/notebooks/pull/599/files#diff-07cbcedb761f69b198691b554bea634acaf78ebc7005415b45a3fe51033cc561...

I am not sure why this Verify command is necessary... are you observing cases where the build step does NOT load an image - and yet the GitHub Action workflow continues on? That would be the only situation in which I would think we need this explicit "Verify" step - and yet I would expec the build step to fail (and likewise the workflow to error).

I need more details around necessity of this logic before I could sign off on these changes.

I'm unclear why this Verify target is necessary (?)

Yes, exactly. I'm observing cases where image not loaded (not sure if they will pop since the step is hang). As mentioned before I can't decide on failure in the build phase. In this PR my initial suggestion is to wrap it with timeout and then run some validations, of course in that phase we can fail the build if needed.

andyatmiami · 2025-09-19T18:11:26Z

.github/workflows/jwa_multi_arch_test.yaml

+    - name: Build ARM64 Image (Optional)
+      timeout-minutes: 60
+      continue-on-error: true
+      run: |
+        cd components/crud-web-apps/jupyter
+        ARCH=linux/arm64/v8 make docker-build-multi-arch-verify
+        echo "Completed linux/arm64/v8 build!"


This was not an optional step in the original kubeflow/kubeflow JWA Multi-Arch Build Test workflow.. Would need you to add additional context here to judge if this is in fact a warranted/necessary change.

So as mentioned before, from my investigation it might be a runner issue but I'm not sure.

andyatmiami · 2025-09-19T18:12:51Z

.github/workflows/jwa_multi_arch_test.yaml

+      - components/crud-web-apps/common/**
+      - releasing/version/VERSION
+    branches:
+      - master


kubeflow/notebooks does NOT have a master branch... I would assume this is intended to read main (?)

Absolutely correct, my bad. Will fix it in the next patch after we will resolve some of those discussions above.

andyatmiami · 2025-09-29T17:36:44Z

/ok-to-test

juliusvonkohout · 2025-09-30T11:03:20Z

.github/workflows/jwa_backend_unittests.yaml

+
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.7"


May we use 3.12? 3.7 is EOL.

@juliusvonkohout

as outlined in my #kubeflow-notebooks Slack message - I'd prefer for this initial PR to migrate the workflows - that we try to adhere as close as possible to the kubeflow/kubeflow implementation.

Your feedback/insights totally spot on - I'd just prefer a follow up PR once we have notebooks-v1 branch of kubeflow/notebooks at feature parity .

juliusvonkohout · 2025-09-30T11:04:43Z

.github/workflows/jwa_backend_unittests.yaml

+      - name: Setup Python environment
+        run: |
+          cd components/crud-web-apps/jupyter/backend
+          make install-deps


make install-dependencies i would prefer.

see: #599 (comment)

juliusvonkohout · 2025-09-30T11:07:25Z

components/testing/gh-actions/install_istio.sh

+set -euo pipefail
+
+ISTIO_VERSION="1.17.8"
+ISTIO_URL="https://istio.io/downloadIstio"


Lets use https://github.com/kubeflow/manifests/blob/6aad970e15f07af7841c46c0b03f65e7b06bf02f/example/kustomization.yaml#L39 if you want to stay close to the platform and not maintain istio yourself. This is also used in the KFP repository here https://github.com/kubeflow/pipelines/blob/51ab5e678feee36b59a000f3a03abe0ccd174360/.github/resources/scripts/deploy-kfp.sh#L117

see: #599 (comment)

juliusvonkohout · 2025-09-30T11:09:20Z

components/testing/gh-actions/install_kind.sh

You can copy https://github.com/kubeflow/manifests/blob/master/tests/install_KinD_create_KinD_cluster_install_kustomize.sh for kind 0.29

You can also more easily free space if needed if you take a look at the KFP GHA instead of working with the swapfile

see: #599 (comment)

juliusvonkohout · 2025-09-30T11:10:40Z

components/testing/gh-actions/install_kustomize.sh

You can copy https://github.com/kubeflow/manifests/blob/master/tests/install_KinD_create_KinD_cluster_install_kustomize.sh

see: #599 (comment)

juliusvonkohout · 2025-09-30T11:11:16Z

components/testing/gh-actions/kind-1-25.yaml

+        "service-account-signing-key-file": "/etc/kubernetes/pki/sa.key"
+nodes:
+- role: control-plane
+  image: kindest/node:v1.25.3@sha256:f52781bc0d7a19fb6c405c2af83abfeb311f130707a0e219175677e366cc45d1


Please test with Kubernetes 1.33 as in You can copy https://github.com/kubeflow/manifests/blob/master/tests/install_KinD_create_KinD_cluster_install_kustomize.sh

see: #599 (comment)

andyatmiami

Thanks for all the work and experimentation here.

Just a few comments to review/clean up and I think we'll be ready to get this in. Please reach out if you have any questions!

.github/workflows/jwa_multi_arch_test.yaml

.github/workflows/jwa_frontend_tests.yaml

…tebooks-v1 branch kubeflow#587 Signed-off-by: Yehudit Kerido <[email protected]>

andyatmiami

/lgtm

Thanks for all this work @yehudit1987 - was certainly more "fun/challenging" than I originally anticipated - but appreciate you working through it all.

I'm comfortable with the changes and have verified the workflows behave as expected.

Tested on my fork:

andyatmiami#54

kimwnasptd · 2025-10-21T09:40:13Z

really great work @yehudit1987!

And thank you @andyatmiami for the very thorough review, as always!

/lgtm
/approve

google-oss-prow · 2025-10-21T09:40:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kimwnasptd

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [kimwnasptd]

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

github-project-automation bot added this to Kubeflow Notebooks Sep 15, 2025

github-project-automation bot moved this to Needs Triage in Kubeflow Notebooks Sep 15, 2025

yehudit1987 marked this pull request as draft September 15, 2025 06:14

google-oss-prow bot requested review from kimwnasptd and thesuperzapper September 15, 2025 06:14

google-oss-prow bot added do-not-merge/work-in-progress size/L labels Sep 15, 2025

yehudit1987 force-pushed the feat/migrate_jwa_tests branch 9 times, most recently from 0acf6b9 to 1c8c9ac Compare September 17, 2025 09:03

google-oss-prow bot added area/backend area - related to backend components area/ci area - related to ci area/frontend area - related to frontend components area/v1 area - version - kubeflow notebooks v1 labels Sep 17, 2025

yehudit1987 force-pushed the feat/migrate_jwa_tests branch 10 times, most recently from 0ac0946 to 8b9c3cc Compare September 18, 2025 11:39

andyatmiami suggested changes Sep 19, 2025

View reviewed changes

google-oss-prow bot assigned andyatmiami Sep 19, 2025

yehudit1987 force-pushed the feat/migrate_jwa_tests branch 4 times, most recently from e0c3ab3 to 9ee858b Compare September 29, 2025 13:50

andyatmiami mentioned this pull request Sep 29, 2025

ci: add PVCViewer test workflows #601

Merged

google-oss-prow bot added the ok-to-test label Sep 29, 2025

juliusvonkohout reviewed Sep 30, 2025

View reviewed changes

yehudit1987 force-pushed the feat/migrate_jwa_tests branch from 9ee858b to 534a8ac Compare October 5, 2025 09:03

andyatmiami suggested changes Oct 6, 2025

View reviewed changes

This was referenced Oct 6, 2025

feat: Migrate vwa workflows clean #625

Draft

feat: Migrate TWA test-related workflows #627

Open

feat: Migrate JWA test-related workflows from kubeflow/kubeflow to no…

825bc66

…tebooks-v1 branch kubeflow#587 Signed-off-by: Yehudit Kerido <[email protected]>

yehudit1987 force-pushed the feat/migrate_jwa_tests branch from 534a8ac to 825bc66 Compare October 15, 2025 06:08

andyatmiami approved these changes Oct 20, 2025

View reviewed changes

google-oss-prow bot added the lgtm label Oct 20, 2025

google-oss-prow bot assigned kimwnasptd Oct 21, 2025

google-oss-prow bot added the approved label Oct 21, 2025

google-oss-prow bot merged commit 0ea96ab into kubeflow:notebooks-v1 Oct 21, 2025
23 checks passed

github-project-automation bot moved this from Needs Triage to Done in Kubeflow Notebooks Oct 21, 2025

yehudit1987 deleted the feat/migrate_jwa_tests branch October 22, 2025 09:40

feat: Migrate JWA test-related workflows from kubeflow/kubeflow to notebooks-v1 branch #587 #599

feat: Migrate JWA test-related workflows from kubeflow/kubeflow to notebooks-v1 branch #587 #599

Uh oh!

Conversation

yehudit1987 commented Sep 15, 2025

Uh oh!

andyatmiami left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andyatmiami commented Sep 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juliusvonkohout Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andyatmiami left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andyatmiami left a comment

Choose a reason for hiding this comment

Uh oh!

kimwnasptd commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

google-oss-prow bot commented Oct 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

juliusvonkohout Sep 30, 2025 •

edited

Loading

kimwnasptd commented Oct 21, 2025 •

edited

Loading