feat(Backend + SDK): Update kfp backend and kubernetes sdk to support ConfigMaps as volumes and as env variables #10483

roytman · 2024-02-14T12:31:17Z

Description of your changes:
Part of #9768
Fixes #10251

Update kfp backend and kubernetes sdk to support Config Maps as volumes and environment variables.

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.

…s and as env Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

google-oss-prow · 2024-02-14T12:31:28Z

Hi @roytman. 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.

Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

roytman · 2024-02-14T12:49:57Z

/lint

google-oss-prow

@roytman: 0 warnings.

In response to this:

/lint

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.

Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

Tomcli · 2024-02-14T16:26:45Z

/ok-to-test

Tomcli · 2024-02-14T18:37:04Z

backend/third_party_licenses/apiserver.csv

@@ -108,7 +107,7 @@ gocloud.dev,https://github.com/google/go-cloud/blob/v0.22.0/LICENSE,Apache-2.0
 golang.org/x/crypto,https://cs.opensource.google/go/x/crypto/+/v0.9.0:LICENSE,BSD-3-Clause
 golang.org/x/net,https://cs.opensource.google/go/x/net/+/v0.10.0:LICENSE,BSD-3-Clause
 golang.org/x/oauth2,https://cs.opensource.google/go/x/oauth2/+/d3ed0bb2:LICENSE,BSD-3-Clause
-golang.org/x/sys,https://cs.opensource.google/go/x/sys/+/v0.8.0:LICENSE,BSD-3-Clause
+golang.org/x/sys/unix,https://cs.opensource.google/go/x/sys/+/v0.8.0:LICENSE,BSD-3-Clause


I don't think the latest commit changed the deps on the apiserver. It should only updated the kubernetes_platform version.

Tomcli · 2024-02-14T18:41:30Z

backend/src/v2/driver/driver.go

@@ -512,6 +512,39 @@ func extendPodSpecPatch(
 		}
 	}

+	// Get config map mount information


nit: maybe we can refactor these two for loops into functions so that we can also reuse them for secret mount and secret env

Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

Tomcli

Thanks @roytman, the tests are passed now. Are you planning to refactor the two for loops for configmap mount and configmap env into functions?

/assign @chensun

roytman · 2024-02-15T08:18:24Z

Thanks @roytman, the tests are passed now. Are you planning to refactor the two for loops for configmap mount and configmap env into functions?

Hi @Tomcli , sorry, I don't see how it can be done without changes in kubernetes_platform/proto/kubernetes_executor_config.proto and in k8s.io/api/core/v1 can we check it together?

Tomcli · 2024-02-15T17:17:00Z

Thanks @roytman, the tests are passed now. Are you planning to refactor the two for loops for configmap mount and configmap env into functions?

Hi @Tomcli , sorry, I don't see how it can be done without changes in kubernetes_platform/proto/kubernetes_executor_config.proto and in k8s.io/api/core/v1 can we check it together?

I'm thinking more of refactoring the common pieces between configmap and secret volume mount into a common function. Since it involved some object abstraction for configmapRef/secretRef, we can defer this to a follow up PR.

/lgtm

connor-mccarthy

/lgtm from SDK standpoint

connor-mccarthy · 2024-02-15T22:20:58Z

kubernetes_platform/python/kfp/kubernetes/config_map.py

+    config_map_key_to_env: Dict[str, str],
+) -> PipelineTask:
+    """Use a Kubernetes ConfigMap as an environment variable as described in
+    https://kubernetes.io/docs/tasks/configure-pod-container/configure-pod-configmap/#define-container-environment-variables-using-configmap-data.


Can we style the link using RST for clean rendering in the reference docs?

done, thanks

kubernetes_platform/python/kfp/kubernetes/config_map.py

Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

connor-mccarthy · 2024-02-16T21:01:34Z

/lgtm for SDK

connor-mccarthy · 2024-02-16T21:02:15Z

kubernetes_platform/python/kfp/kubernetes/config_map.py

+    config_map_name: str,
+    config_map_key_to_env: Dict[str, str],
+) -> PipelineTask:
+    """use a Kubernetes ConfigMap as an environment variable as described by the `Kubernetes documentation


Suggested change

"""use a Kubernetes ConfigMap as an environment variable as described by the `Kubernetes documentation

"""Use a Kubernetes ConfigMap as an environment variable as described by the `Kubernetes documentation

Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

Tomcli

/lgtm

connor-mccarthy · 2024-02-20T23:00:03Z

/lgtm for SDK

Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

google-oss-prow · 2024-02-23T21:56:06Z

@roytman: The following tests 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	`36daba6`	link	false	`/test kubeflow-pipelines-samples-v2`
kfp-kubernetes-execution-tests	`36daba6`	link	false	`/test kfp-kubernetes-execution-tests`

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

/lgtm

google-oss-prow · 2024-02-24T05:38:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chensun, connor-mccarthy, Tomcli

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~~ [chensun]

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

… ConfigMaps as volumes and as env variables (kubeflow#10483) * Update kfp backend and kubernetes sdk to support ConfigMaps as volumes and as env Signed-off-by: Alexey Roytman <roytman@il.ibm.com> * update go.mod, apiserver.csv and driver.csv Signed-off-by: Alexey Roytman <roytman@il.ibm.com> * add test/snapshot/data files Signed-off-by: Alexey Roytman <roytman@il.ibm.com> * fix tests Signed-off-by: Alexey Roytman <roytman@il.ibm.com> * go mod tidy Signed-off-by: Alexey Roytman <roytman@il.ibm.com> * update backend/third_party_licenses/apiserver.csv Signed-off-by: Alexey Roytman <roytman@il.ibm.com> * update backend/third_party_licenses/apiserver.csv Signed-off-by: Alexey Roytman <roytman@il.ibm.com> * fix comments Signed-off-by: Alexey Roytman <roytman@il.ibm.com> * fix comments Signed-off-by: Alexey Roytman <roytman@il.ibm.com> * update go.mod, apiserver.csv and driver.csv Signed-off-by: Alexey Roytman <roytman@il.ibm.com> --------- Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

Update kfp backend and kubernetes sdk to support ConfigMaps as volume…

d42bca0

…s and as env Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

google-oss-prow bot requested review from connor-mccarthy and gkcalat February 14, 2024 12:31

google-oss-prow bot added the size/XL label Feb 14, 2024

google-oss-prow bot added the needs-ok-to-test label Feb 14, 2024

update go.mod, apiserver.csv and driver.csv

dcde0c0

Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

google-oss-prow bot reviewed Feb 14, 2024

View reviewed changes

add test/snapshot/data files

b395d10

Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

google-oss-prow bot added ok-to-test and removed needs-ok-to-test labels Feb 14, 2024

Tomcli reviewed Feb 14, 2024

View reviewed changes

roytman added 4 commits February 14, 2024 22:26

fix tests

03015f3

Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

go mod tidy

c703e3d

Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

update backend/third_party_licenses/apiserver.csv

ca26f84

Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

update backend/third_party_licenses/apiserver.csv

8dff487

Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

Tomcli reviewed Feb 14, 2024

View reviewed changes

google-oss-prow bot assigned chensun Feb 14, 2024

google-oss-prow bot assigned Tomcli Feb 15, 2024

google-oss-prow bot added the lgtm label Feb 15, 2024

Tomcli mentioned this pull request Feb 15, 2024

chore: Abstract driver.go configmap/secret volume and env mount into a common function. #10491

Closed

connor-mccarthy requested changes Feb 15, 2024

View reviewed changes

fix comments

fc44104

Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

google-oss-prow bot removed the lgtm label Feb 16, 2024

connor-mccarthy reviewed Feb 16, 2024

View reviewed changes

roytman force-pushed the ConfigMapsSupport3 branch from c3cdaa2 to ca2b0db Compare February 17, 2024 06:32

fix comments

e989d04

Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

roytman force-pushed the ConfigMapsSupport3 branch from ca2b0db to e989d04 Compare February 17, 2024 06:59

Tomcli reviewed Feb 19, 2024

View reviewed changes

google-oss-prow bot added the lgtm label Feb 19, 2024

chensun approved these changes Feb 22, 2024

View reviewed changes

google-oss-prow bot added the approved label Feb 22, 2024

roytman added 2 commits February 23, 2024 22:12

Merge branch 'master' into ConfigMapsSupport3

811d887

update go.mod, apiserver.csv and driver.csv

36daba6

Signed-off-by: Alexey Roytman <roytman@il.ibm.com>

google-oss-prow bot removed the lgtm label Feb 23, 2024

Tomcli approved these changes Feb 24, 2024

View reviewed changes

google-oss-prow bot added the lgtm label Feb 24, 2024

google-oss-prow bot merged commit 1edd85f into kubeflow:master Feb 24, 2024
12 of 14 checks passed

Tomcli mentioned this pull request Mar 8, 2024

Add JosepSampe, revit13, and roytman to Kubeflow org kubeflow/internal-acls#663

Merged

This was referenced Mar 11, 2024

fix(Backend + SDK): Add missing optional field to SecretAsVolume and … #10550

Merged

[sdk] Missing optional parameter to v2 use_config_map_as_volume and use_secret_as_volume #10548

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Backend + SDK): Update kfp backend and kubernetes sdk to support ConfigMaps as volumes and as env variables #10483

feat(Backend + SDK): Update kfp backend and kubernetes sdk to support ConfigMaps as volumes and as env variables #10483

roytman commented Feb 14, 2024

google-oss-prow bot commented Feb 14, 2024

roytman commented Feb 14, 2024

google-oss-prow bot left a comment

Tomcli commented Feb 14, 2024

Tomcli Feb 14, 2024

Tomcli Feb 14, 2024

Tomcli left a comment

roytman commented Feb 15, 2024

Tomcli commented Feb 15, 2024

connor-mccarthy left a comment

connor-mccarthy Feb 15, 2024

roytman Feb 16, 2024

connor-mccarthy commented Feb 16, 2024

connor-mccarthy Feb 16, 2024

roytman Feb 16, 2024

Tomcli left a comment

connor-mccarthy commented Feb 20, 2024

google-oss-prow bot commented Feb 23, 2024 •

edited

Loading

Tomcli left a comment

google-oss-prow bot commented Feb 24, 2024

	"""use a Kubernetes ConfigMap as an environment variable as described by the `Kubernetes documentation
	"""Use a Kubernetes ConfigMap as an environment variable as described by the `Kubernetes documentation

feat(Backend + SDK): Update kfp backend and kubernetes sdk to support ConfigMaps as volumes and as env variables #10483

feat(Backend + SDK): Update kfp backend and kubernetes sdk to support ConfigMaps as volumes and as env variables #10483

Conversation

roytman commented Feb 14, 2024

google-oss-prow bot commented Feb 14, 2024

roytman commented Feb 14, 2024

google-oss-prow bot left a comment

Choose a reason for hiding this comment

Tomcli commented Feb 14, 2024

Tomcli Feb 14, 2024

Choose a reason for hiding this comment

Tomcli Feb 14, 2024

Choose a reason for hiding this comment

Tomcli left a comment

Choose a reason for hiding this comment

roytman commented Feb 15, 2024

Tomcli commented Feb 15, 2024

connor-mccarthy left a comment

Choose a reason for hiding this comment

connor-mccarthy Feb 15, 2024

Choose a reason for hiding this comment

roytman Feb 16, 2024

Choose a reason for hiding this comment

connor-mccarthy commented Feb 16, 2024

connor-mccarthy Feb 16, 2024

Choose a reason for hiding this comment

roytman Feb 16, 2024

Choose a reason for hiding this comment

Tomcli left a comment

Choose a reason for hiding this comment

connor-mccarthy commented Feb 20, 2024

google-oss-prow bot commented Feb 23, 2024 • edited Loading

Tomcli left a comment

Choose a reason for hiding this comment

google-oss-prow bot commented Feb 24, 2024

google-oss-prow bot commented Feb 23, 2024 •

edited

Loading