test: install kfp on github periodic functional tests workflow #10859

tmvfb · 2024-06-02T08:22:20Z

Description of your changes:
Hopefully last fix for periodic functional tests. Current problems (#10851):

Tests fail in GH workflows as they try to connect to the same GCP cluster that prow tests run on. The corresponding environment is not configured in GitHub.
Tests fail in prow as they try to run sudo which is not available on python3.7-slim (runs on debian).

Solutions:

For GH actions, use the kind cluster defined in the workflow file to run the test instead of GCP test cluster. This requires actually installing kfp on this kind cluster. I copypasted installation from the backend test workflow file. Let me know if there is a simpler/faster solution (test runs for 15 mins), standalone kfp deployment didn't work for me.
Make tests aware of the environment they are running on. If running in GitHub actions (we can check if KIND_REGISTRY env var created by kind action is available), tests work without indicating host. For prow tests, run apt without sudo, as it was initially defined.

Checklist:

The title for your pull request (PR) should follow our title convention. Learn more about the pull request title convention used in this repository.

google-cla · 2024-06-02T08:22:25Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-oss-prow · 2024-06-02T08:22:31Z

Hi @tmvfb. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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-sigs/prow repository.

hbelmiro · 2024-06-03T17:45:09Z

/ok-to-test

rimolive

Thank you for your contributions! I left a few comments, let's work on these items before lgtm/approve

rimolive · 2024-06-03T18:54:57Z

test/kfp-functional-test/kfp-functional-test.sh


-python3 "${source_root}/test/kfp-functional-test/run_kfp_functional_test.py" --host "${HOST}"
+if [ -n "$KIND_REGISTRY" ]; then


When this variable is specified when GH Action is running?

This variable gets created when the kind action runs, see here. If we don't need prow tests, we can remove this conditional anyway.

rimolive · 2024-06-03T18:57:23Z

.github/workflows/periodic.yml

+      - name: Build images
+        run: ./scripts/deploy/github/build-images.sh
+      - name: Deploy kfp-tekton
+        run: ./scripts/deploy/github/deploy-kfp.sh


These new lines are supposed to be only used in kfp-tekton backend tests. Periodic tests don't run any tests against kfp-tekton.

I guess we can run the tests against standalone kfp then? I realised that my tests didn't work since I didn't allow for any time for the kfp pods to come up in the test cluster.

rimolive · 2024-06-03T18:58:29Z

test/kfp-functional-test/kfp-functional-test.sh

+if [ -n "$KIND_REGISTRY" ]; then
+    # if KIND_REGISTRY is available, we're running GH actions, host is not required
+    python3 "${source_root}/test/kfp-functional-test/run_kfp_functional_test.py" --host ""
+else


We expect to remove the Prow jobs to run these tests, I think this confitional is not required.

tmvfb · 2024-06-03T23:37:24Z

@rimolive, completely reworked the PR, now we deploy standalone kfp inside GitHub actions kind cluster instead of kfp-tekton. I also simplified shell script considering we won't need prow tests anymore.

rimolive

One last comment.

test/kfp-functional-test/kfp-functional-test.sh

rimolive · 2024-06-05T20:26:38Z

Don't forget to sign-off your commits

tmvfb · 2024-06-05T20:33:18Z

Don't forget to sign-off your commits

Done!

google-oss-prow · 2024-06-05T20:34:16Z

@tmvfb: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
kubeflow-pipeline-e2e-test	`99298b8`	link	false	`/test kubeflow-pipeline-e2e-test`

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-sigs/prow repository. I understand the commands that are listed here.

rimolive · 2024-06-05T22:02:06Z

Thank you!

/lgttm

cc @DharmitD @chensun

DharmitD

changes /lgtm

@tmvfb could you provide a sample run on your fork with these changes?

tmvfb · 2024-06-05T22:21:05Z

@DharmitD, https://github.com/prokube-ai/pipelines/actions/runs/9390604598

DharmitD

/lgtm

rimolive · 2024-06-06T13:30:19Z

/approve

rimolive · 2024-06-06T13:30:45Z

cc @chensun

rimolive · 2024-06-13T16:36:30Z

@chensun Can you approve?

DharmitD

/Approve

google-oss-prow · 2024-07-01T13:07:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DharmitD, rimolive

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:

~~.github/OWNERS~~ [DharmitD]
~~test/OWNERS~~ [DharmitD]

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

hbelmiro · 2024-07-01T14:47:11Z

@tmvfb can you please rebase?

tmvfb · 2024-07-01T16:48:43Z

@tmvfb can you please rebase?

@hbelmiro, @DharmitD, done

hbelmiro · 2024-07-01T18:48:57Z

.github/workflows/periodic.yml

          kubectl_version: v1.29.2
          version: v0.22.0
          node_image: kindest/node:v1.29.2
+      - name: Deploy kfp


One more thing @tmvfb. Sorry.
Can you please use the new kfp-cluster action here? Like what is done in

pipelines/.github/workflows/e2e-test.yaml

Line 26 in 2fb4922

- name: Create KFP cluster

.

@hbelmiro, no problem, edited commits and tested here. Sorry for so many force pushes :D

hbelmiro · 2024-07-01T18:50:28Z

/hold

Signed-off-by: Igor Kvachenok <igor.kvachenok@prokube.ai>

hbelmiro

/unhold
/lgtm

google-oss-prow bot added the size/S label Jun 2, 2024

google-oss-prow bot added the needs-ok-to-test label Jun 2, 2024

google-oss-prow bot requested review from jlyaoyuli and zijianjoy June 2, 2024 08:22

tmvfb force-pushed the fix/functional_tests_missing_kfp_cluster branch from ca941ce to d486dbe Compare June 2, 2024 08:23

google-oss-prow bot added ok-to-test and removed needs-ok-to-test labels Jun 3, 2024

rimolive reviewed Jun 3, 2024

View reviewed changes

tmvfb force-pushed the fix/functional_tests_missing_kfp_cluster branch from d486dbe to d0b7262 Compare June 3, 2024 23:33

rimolive reviewed Jun 4, 2024

View reviewed changes

test/kfp-functional-test/kfp-functional-test.sh Outdated Show resolved Hide resolved

tmvfb force-pushed the fix/functional_tests_missing_kfp_cluster branch 2 times, most recently from c9907bd to 1e0bdb6 Compare June 5, 2024 20:21

tmvfb force-pushed the fix/functional_tests_missing_kfp_cluster branch from 1e0bdb6 to 99298b8 Compare June 5, 2024 20:32

DharmitD reviewed Jun 5, 2024

View reviewed changes

google-oss-prow bot assigned DharmitD Jun 5, 2024

google-oss-prow bot added the lgtm label Jun 5, 2024

DharmitD approved these changes Jul 1, 2024

View reviewed changes

google-oss-prow bot added the approved label Jul 1, 2024

tmvfb force-pushed the fix/functional_tests_missing_kfp_cluster branch from 99298b8 to 056ffaf Compare July 1, 2024 16:47

google-oss-prow bot removed the lgtm label Jul 1, 2024

hbelmiro suggested changes Jul 1, 2024

View reviewed changes

google-oss-prow bot added the do-not-merge/hold label Jul 1, 2024

tmvfb force-pushed the fix/functional_tests_missing_kfp_cluster branch 2 times, most recently from 8029cb8 to 60be588 Compare July 2, 2024 10:24

tmvfb added 2 commits July 2, 2024 12:33

test: add kfp cluster to functional tests workflow

79a99c8

Signed-off-by: Igor Kvachenok <igor.kvachenok@prokube.ai>

test: portforward kfp cluster for functional tests

e14d5c8

Signed-off-by: Igor Kvachenok <igor.kvachenok@prokube.ai>

tmvfb force-pushed the fix/functional_tests_missing_kfp_cluster branch from 60be588 to e14d5c8 Compare July 2, 2024 10:35

hbelmiro reviewed Jul 2, 2024

View reviewed changes

google-oss-prow bot removed the do-not-merge/hold label Jul 2, 2024

google-oss-prow bot assigned hbelmiro Jul 2, 2024

google-oss-prow bot added the lgtm label Jul 2, 2024

google-oss-prow bot merged commit 7d2ec39 into kubeflow:master Jul 2, 2024
3 checks passed

tmvfb deleted the fix/functional_tests_missing_kfp_cluster branch July 5, 2024 15:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: install kfp on github periodic functional tests workflow #10859

test: install kfp on github periodic functional tests workflow #10859

tmvfb commented Jun 2, 2024

google-cla bot commented Jun 2, 2024

google-oss-prow bot commented Jun 2, 2024

hbelmiro commented Jun 3, 2024

rimolive left a comment

rimolive Jun 3, 2024

tmvfb Jun 3, 2024

rimolive Jun 3, 2024

tmvfb Jun 3, 2024

rimolive Jun 3, 2024

tmvfb commented Jun 3, 2024

rimolive left a comment

rimolive commented Jun 5, 2024

tmvfb commented Jun 5, 2024

google-oss-prow bot commented Jun 5, 2024

rimolive commented Jun 5, 2024

DharmitD left a comment

tmvfb commented Jun 5, 2024

DharmitD left a comment

rimolive commented Jun 6, 2024

rimolive commented Jun 6, 2024

rimolive commented Jun 13, 2024

DharmitD left a comment

google-oss-prow bot commented Jul 1, 2024

hbelmiro commented Jul 1, 2024

tmvfb commented Jul 1, 2024

hbelmiro Jul 1, 2024

tmvfb Jul 2, 2024 •

edited

Loading

hbelmiro commented Jul 1, 2024

hbelmiro left a comment


		python3 "${source_root}/test/kfp-functional-test/run_kfp_functional_test.py" --host "${HOST}"
		if [ -n "$KIND_REGISTRY" ]; then

test: install kfp on github periodic functional tests workflow #10859

test: install kfp on github periodic functional tests workflow #10859

Conversation

tmvfb commented Jun 2, 2024

google-cla bot commented Jun 2, 2024

google-oss-prow bot commented Jun 2, 2024

hbelmiro commented Jun 3, 2024

rimolive left a comment

Choose a reason for hiding this comment

rimolive Jun 3, 2024

Choose a reason for hiding this comment

tmvfb Jun 3, 2024

Choose a reason for hiding this comment

rimolive Jun 3, 2024

Choose a reason for hiding this comment

tmvfb Jun 3, 2024

Choose a reason for hiding this comment

rimolive Jun 3, 2024

Choose a reason for hiding this comment

tmvfb commented Jun 3, 2024

rimolive left a comment

Choose a reason for hiding this comment

rimolive commented Jun 5, 2024

tmvfb commented Jun 5, 2024

google-oss-prow bot commented Jun 5, 2024

rimolive commented Jun 5, 2024

DharmitD left a comment

Choose a reason for hiding this comment

tmvfb commented Jun 5, 2024

DharmitD left a comment

Choose a reason for hiding this comment

rimolive commented Jun 6, 2024

rimolive commented Jun 6, 2024

rimolive commented Jun 13, 2024

DharmitD left a comment

Choose a reason for hiding this comment

google-oss-prow bot commented Jul 1, 2024

hbelmiro commented Jul 1, 2024

tmvfb commented Jul 1, 2024

hbelmiro Jul 1, 2024

Choose a reason for hiding this comment

tmvfb Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

hbelmiro commented Jul 1, 2024

hbelmiro left a comment

Choose a reason for hiding this comment

tmvfb Jul 2, 2024 •

edited

Loading