feat(backend): Parameterizing v2 Launcher and Driver Images #10269

DharmitD · 2023-11-28T13:16:19Z

Description of your changes:
Resolves #10229

Parameterizing the v2 launcher and driver images so it's possible to extend/provide fixes to the code.

Testing instructions:

Follow instructions listed here and run a make install-compiler to ensure that the changes compile correctly.
Save the code changes locally and build an API Server image
Deploy KFP using steps listed here and replace the API Server image with the above image. Make sure API Server still comes up correctly.

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-oss-prow · 2023-11-28T13:16:30Z

Hi @DharmitD. 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/test-infra repository.

backend/src/v2/compiler/argocompiler/container.go

yhwang · 2023-11-30T16:21:17Z

backend/src/v2/compiler/argocompiler/container.go

+				Env: []k8score.EnvVar{
+					{
+						Name:  LauncherImageEnvVar,
+						Value: GetLauncherImage(),


is this value the same as c.launcherImage at line 241? if yes, you can directly use it. no need to call the function again, although it gets the same value.

@yhwang good catch!
I updated it to set Image directly to the result of GetLauncherImage() and remove the Env field.

Tomcli · 2023-11-30T16:46:45Z

backend/src/v2/compiler/argocompiler/argo.go

@@ -117,7 +117,7 @@ func Compile(jobArg *pipelinespec.PipelineJob, kubernetesSpecArg *pipelinespec.S
 		templates: make(map[string]*wfapi.Template),
 		// TODO(chensun): release process and update the images.
 		driverImage:   "gcr.io/ml-pipeline/kfp-driver@sha256:8e60086b04d92b657898a310ca9757631d58547e76bbbb8bfc376d654bef1707",


Not sure do you also want to parameterize the driver image? This image is also hard coded into the spec unless the Argo compiler move it to use Argo HTTP Template.

+1, please do the same for driver as well, the two are usually updated together

@Tomcli @chensun Added a commit to parameterize the driver image as well. Could you please take a look?

Tomcli · 2023-11-30T18:50:02Z

/ok-to-test

chensun · 2023-12-01T23:23:42Z

/test kubeflow-pipeline-upgrade-test

DharmitD · 2023-12-02T13:41:59Z

/test kubeflow-pipelines-samples-v2

DharmitD · 2023-12-02T16:44:19Z

/test kubeflow-pipelines-samples-v2

chensun · 2023-12-05T17:52:38Z

Seems like the error is related to this issue #10282 @connor-mccarthy Do you know how can we pass this test? It seems like this is an issue that could block future PRs as well. Thank you.

kubeflow-pipelines-samples-v2 is broken and it's a know issue, this test is not a required test though, it won't block any PR merge.

chensun

Thanks @DharmitD for the contribution.

I'll leave it to @Tomcli to add the lgtm label for merging

/approve

backend/src/v2/compiler/argocompiler/container.go

chensun · 2023-12-05T17:56:32Z

backend/src/v2/compiler/argocompiler/container.go

@@ -111,7 +133,7 @@ func (c *workflowCompiler) addContainerDriverTemplate() string {
 			},
 		},
 		Container: &k8score.Container{
-			Image:   c.driverImage,
+			Image:   GetDriverImage(),


nit: It would be great if you can add some unit tests to this change. Not necessarily a blocker for merging this PR though.

google-oss-prow · 2023-12-05T17:59:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chensun

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:

~~backend/OWNERS~~ [chensun]

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

google-oss-prow · 2023-12-05T18:15:54Z

@DharmitD: 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-pipelines-samples-v2	`af2fa35`	link	false	`/test kubeflow-pipelines-samples-v2`

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

Tomcli · 2023-12-05T21:01:37Z

/lgtm

Thanks @DharmitD

DharmitD · 2023-12-06T11:04:53Z

Thank you for reviewing @chensun @Tomcli !

…#10269) * Parameterize v2 launcher image * Parameterize v2 driver image

xixici · 2023-12-27T03:10:26Z

I added the environment variables in the API server, but it doesn't seem to have any effect. Here are the commands I used:

echo 'export V2_LAUNCHER_IMAGE=kfp-launcher:0.0.1-aarch64-beta' >> ~/.bashrc
echo 'export V2_DRIVER_IMAGE=kfp-driver:0.0.1-aarch64-beta' >> ~/.bashrc

source ~/.bashrc

I also added the environment variables in the ArgoExec image, but unfortunately, it doesn't work either.
I'm confused about this feature. I am not a Golang engineer, so I might have made some mistakes in understanding or implementing the necessary configurations.

@DharmitD @chensun

xin053 · 2024-01-30T07:50:09Z

I added the environment variables in the API server, but it doesn't seem to have any effect. Here are the commands I used:
echo 'export V2_LAUNCHER_IMAGE=kfp-launcher:0.0.1-aarch64-beta' >> ~/.bashrc
echo 'export V2_DRIVER_IMAGE=kfp-driver:0.0.1-aarch64-beta' >> ~/.bashrc

source ~/.bashrc
I also added the environment variables in the ArgoExec image, but unfortunately, it doesn't work either. I'm confused about this feature. I am not a Golang engineer, so I might have made some mistakes in understanding or implementing the necessary configurations.

@DharmitD @chensun

kubectl -n kubeflow edit deployment.apps/ml-pipeline

update image gcr.io/ml-pipeline/api-server to use new tag，such as gcr.io/ml-pipeline/api-server:2.0.5

then add env V2_LAUNCHER_IMAGE and V2_DRIVER_IMAGE

…#10269) * Parameterize v2 launcher image * Parameterize v2 driver image

thesuperzapper · 2024-04-19T20:48:03Z

I want to highlight that V2_LAUNCHER_IMAGE will NOT affect V2_COMPATIBLE mode runs.

These runs will still use the _DEFAULT_LAUNCHER_IMAGE which was hard-coded in the version of the kfp SDK you are using which is usually gcr.io/ml-pipeline/kfp-launcher:1.8.7:

It is defined here in kfp==2.7.0
It is defined here in kfp==1.8.7

…455) Enable charm configuration of launcher and driver images used during pipeline steps. Those can be configured through config options `launcher-image` and `driver-image`. When unset, KFP uses the default values defined. This is based on upstream implementation introduced in kubeflow/pipelines#10269. Closes #452

google-oss-prow bot added size/M needs-ok-to-test labels Nov 28, 2023

google-oss-prow bot requested review from chensun and gkcalat November 28, 2023 13:17

DharmitD marked this pull request as draft November 28, 2023 14:36

google-oss-prow bot added the do-not-merge/work-in-progress label Nov 28, 2023

DharmitD changed the title ~~feat(backend): Parameterizing v2 Launcher Image.~~ WIP: feat(backend): Parameterizing v2 Launcher Image. Nov 28, 2023

gmfrasca suggested changes Nov 28, 2023

View reviewed changes

google-oss-prow bot added size/S and removed size/M labels Nov 29, 2023

DharmitD changed the title ~~WIP: feat(backend): Parameterizing v2 Launcher Image.~~ feat(backend): Parameterizing v2 Launcher Image. Nov 29, 2023

DharmitD marked this pull request as ready for review November 29, 2023 14:59

google-oss-prow bot removed the do-not-merge/work-in-progress label Nov 29, 2023

DharmitD marked this pull request as draft November 29, 2023 16:03

google-oss-prow bot added the do-not-merge/work-in-progress label Nov 29, 2023

DharmitD changed the title ~~feat(backend): Parameterizing v2 Launcher Image.~~ WIP: feat(backend): Parameterizing v2 Launcher Image. Nov 29, 2023

DharmitD changed the title ~~WIP: feat(backend): Parameterizing v2 Launcher Image.~~ feat(backend): Parameterizing v2 Launcher Image. Nov 30, 2023

DharmitD marked this pull request as ready for review November 30, 2023 08:59

google-oss-prow bot removed the do-not-merge/work-in-progress label Nov 30, 2023

yhwang reviewed Nov 30, 2023

View reviewed changes

Tomcli reviewed Nov 30, 2023

View reviewed changes

google-oss-prow bot added ok-to-test and removed needs-ok-to-test labels Nov 30, 2023

google-oss-prow bot added size/M and removed size/S labels Dec 4, 2023

chensun approved these changes Dec 5, 2023

View reviewed changes

google-oss-prow bot added the approved label Dec 5, 2023

Parameterize v2 driver image

af2fa35

google-oss-prow bot added size/S and removed size/M labels Dec 5, 2023

google-oss-prow bot assigned Tomcli Dec 5, 2023

google-oss-prow bot added the lgtm label Dec 5, 2023

google-oss-prow bot merged commit 23a2bc3 into kubeflow:master Dec 5, 2023
5 of 6 checks passed

DharmitD mentioned this pull request Dec 6, 2023

[upstream] [feature] Parametrize launcher image in KFPV2 opendatahub-io/data-science-pipelines-operator#476

Closed

petethegreat pushed a commit to petethegreat/pipelines that referenced this pull request Dec 10, 2023

feat(backend): Parameterizing v2 Launcher and Driver Images (kubeflow…

57fe77a

…#10269) * Parameterize v2 launcher image * Parameterize v2 driver image

xixici mentioned this pull request Dec 27, 2023

feat(backend): Read launcher/driver images from ENV vars #10088

Closed

1 task

This was referenced Jan 2, 2024

REQUEST: New membership for DharmitD kubeflow/internal-acls#629

Closed

Add DharmitD as member kubeflow/internal-acls#630

Merged

xin053 mentioned this pull request Jan 30, 2024

[feature] Should be able to set _DEFAULT_LAUNCHER_IMAGE using environment variable in notebook pod #7225

Closed

kimwnasptd mentioned this pull request Feb 12, 2024

Create an airgapped release of CKF 1.8 canonical/bundle-kubeflow#818

Closed

stijntratsaertit pushed a commit to stijntratsaertit/kfp that referenced this pull request Feb 16, 2024

feat(backend): Parameterizing v2 Launcher and Driver Images (kubeflow…

eace4ce

…#10269) * Parameterize v2 launcher image * Parameterize v2 driver image

This was referenced May 7, 2024

bump rocks to kfp v2.0.5 in main canonical/pipelines-rocks#69

Closed

Expose configuration of kfp-api's launcher and driver images in charm config in main canonical/kfp-operators#452

Closed

orfeas-k mentioned this pull request May 9, 2024

feat(api-server): Enable charm configuration of launcher and driver images canonical/kfp-operators#455

Merged

orfeas-k mentioned this pull request May 22, 2024

backport(feat): Enable configuration in kfp-api of launcher and driver images from #455 canonical/kfp-operators#467

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(backend): Parameterizing v2 Launcher and Driver Images #10269

feat(backend): Parameterizing v2 Launcher and Driver Images #10269

DharmitD commented Nov 28, 2023 •

edited

Loading

google-oss-prow bot commented Nov 28, 2023

yhwang Nov 30, 2023

DharmitD Dec 1, 2023

Tomcli Nov 30, 2023

chensun Dec 1, 2023

DharmitD Dec 4, 2023

Tomcli commented Nov 30, 2023

chensun commented Dec 1, 2023

DharmitD commented Dec 2, 2023

DharmitD commented Dec 2, 2023

chensun commented Dec 5, 2023

chensun left a comment

chensun Dec 5, 2023

google-oss-prow bot commented Dec 5, 2023

google-oss-prow bot commented Dec 5, 2023

Tomcli commented Dec 5, 2023

DharmitD commented Dec 6, 2023

xixici commented Dec 27, 2023

xin053 commented Jan 30, 2024

thesuperzapper commented Apr 19, 2024

feat(backend): Parameterizing v2 Launcher and Driver Images #10269

feat(backend): Parameterizing v2 Launcher and Driver Images #10269

Conversation

DharmitD commented Nov 28, 2023 • edited Loading

google-oss-prow bot commented Nov 28, 2023

yhwang Nov 30, 2023

Choose a reason for hiding this comment

DharmitD Dec 1, 2023

Choose a reason for hiding this comment

Tomcli Nov 30, 2023

Choose a reason for hiding this comment

chensun Dec 1, 2023

Choose a reason for hiding this comment

DharmitD Dec 4, 2023

Choose a reason for hiding this comment

Tomcli commented Nov 30, 2023

chensun commented Dec 1, 2023

DharmitD commented Dec 2, 2023

DharmitD commented Dec 2, 2023

chensun commented Dec 5, 2023

chensun left a comment

Choose a reason for hiding this comment

chensun Dec 5, 2023

Choose a reason for hiding this comment

google-oss-prow bot commented Dec 5, 2023

google-oss-prow bot commented Dec 5, 2023

Tomcli commented Dec 5, 2023

DharmitD commented Dec 6, 2023

xixici commented Dec 27, 2023

xin053 commented Jan 30, 2024

thesuperzapper commented Apr 19, 2024

DharmitD commented Nov 28, 2023 •

edited

Loading