Migrate standalone deployment to workload identity on GCP #2619

Bobgy · 2019-11-18T07:04:06Z

Part of #1691

Changes:

Added a bash script for conveniently binding workload identity
Changed test infra to use workload identity
Changed samples to not use use_gcp_secret
Changed container builder to not use gcp secret (Future work of supporting both is tracked in [SDK] Container builder should allow secret configuration #2702)
Changed tensorboard template to not use gcp secret

Note, I didn't completely remove test infra for other than workload identity, because I think it still makes sense to also test KFP in for example full scope cluster. So that we can easily add another test configuration in prow with different ENV to use a different one.

/assign @IronPan
/cc @gaoning777
/cc @rmgogogo

This change is

Bobgy

@IronPan Can you help me briefly review?
Focus on https://github.com/kubeflow/pipelines/pull/2619/files#diff-e5921efca5c992d5c5ab4e91927c9ff3.
It is the script I will let users run.

manifests/kustomize/gcp-workload-identity-setup.sh

rmgogogo · 2019-11-19T04:17:19Z

/lgtm

rmgogogo · 2019-11-19T04:17:31Z

/test

Bobgy · 2019-11-19T10:09:02Z

/retest

gaoning777 · 2019-11-19T17:15:22Z

Is this PR ready to be reviewed?

Bobgy · 2019-11-20T00:00:54Z

Is this PR ready to be reviewed?

Yes, but only review the script used to set up workload identity.

I ran samples successfully with it. However, still struggling to migrate test infra on workload identity.

Bobgy · 2019-11-22T08:12:09Z

It turns out that tests failed before because our image builder docker image was built a year ago with google/cloud-sdk. The old version of cloud sdk doesn't work well with workload identity.

Bobgy · 2019-11-27T06:49:59Z

UPDATE, I removed all usages of use_gcp_secret to pass sample tests.
However, most of the samples need documentation update about how to authorize on GCP

Bobgy · 2019-12-10T13:16:49Z

Verify testing again for flakiness
/test kubeflow-pipeline-e2e-test
/test kubeflow-pipeline-sample-test

rmgogogo · 2019-12-12T15:11:25Z

/lgtm

Bobgy · 2019-12-16T10:48:32Z

/test kubeflow-pipeline-sample-test
sidecar sample is flaky

Bobgy · 2019-12-17T04:41:17Z

container builder seems to be flaky at fetching some image: https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/kubeflow_pipelines/2619/kubeflow-pipeline-sample-test/1206752832519147523#1:build-log.txt%3A4165
secret sample has the following error, I haven't yet figured out if that's a workload identity transient error or not
https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/kubeflow_pipelines/2619/kubeflow-pipeline-sample-test/1206752832519147523#1:build-log.txt%3A4000

Test again to verify
/test kubeflow-pipeline-sample-test

Bobgy · 2019-12-17T05:29:40Z

Verify again for flakiness
/test kubeflow-pipeline-sample-test

rmgogogo · 2019-12-17T05:31:10Z

/lgtm

/retest

Ark-kun · 2020-02-06T22:34:24Z

manifests/kustomize/wi-utils.sh

+  local gsa_full="$gsa@$project.iam.gserviceaccount.com"
+  local namespace=${4:-$NAMESPACE}
+
+  gcloud iam service-accounts add-iam-policy-binding $gsa_full \


Is it possible to add retries here? This command is a source of some test flakiness:

++ gcloud iam service-accounts add-iam-policy-binding test-argo@ml-pipeline-test.iam.gserviceaccount.com '--member=serviceAccount:ml-pipeline-test.svc.id.goog[kubeflow/test-runner]' --role=roles/iam.workloadIdentityUser ERROR: (gcloud.iam.service-accounts.add-iam-policy-binding) ABORTED: There were concurrent policy changes. Please retry the whole read-modify-write with exponential backoff.

Sure, let me put up a PR

Ark-kun · 2020-02-11T06:18:40Z

sdk/python/kfp/containers/_k8s_job_helper.py

-                                           name=yaml_spec['spec']['containers'][0]['env'][0]['name'],
-                                           value=yaml_spec['spec']['containers'][0]['env'][0]['value'],
-                                       )])
+                                       args = yaml_spec['spec']['containers'][0]['args'])


I'm a bit concerned about this change to the container-building functions for non-GKE deployments.
For container-building functions there might be no way for the user to supply secrets.

Yes, I also think it's important to fix. However, I don't currently have bandwidth. Let's get this scheduled soon then.

Ark-kun · 2020-10-06T22:21:24Z

samples/core/secret/secret.py

@@ -57,11 +56,8 @@ def use_gcp_api_op():
 def secret_op_pipeline(url='gs://ml-pipeline-playground/shakespeare1.txt'):
  """A pipeline that uses secret to access cloud hosted resouces."""

-  gcs_read_task = gcs_read_op(url).apply(


The use of secrets seems to be the core feature demonstrated by this sample. Perhaps this should not be removed.

Yes, you are right. Current situation is that we are not running tests that use a service account token. Shall we remove this sample from presubmit/postsubmit tests?

) * Script to set up workload identity for standalone deployment * Migrate tests to run on standalone + workload identity * Fix test script * Switch to static GSAs for testing, because they have name length limit * Add workload identity binding for argo * Fix argo workload identity bindings * Remove user-gcp-sa from tests * Remove use_gcp_secret from xgboost sample * Allow debugging tests locally * Wait for policies to take effect * Update deploy-pipeline-lite.sh * Update deploy-pipeline-lite.sh * [WIP] test gcloud auth list with test-runner sa * Add namespace * test again * Use new image builder * test again * Remove debug code * Remove usages of use_gcp_secret * Fix unit test and tensorboard pod template * Add debug code again to test * Try waiting until workload identity bindings are ready * Fix some other samples * Fix parameterized tfx oss sample * Add retry to image building * Try fixing tfx oss sample * Fix compiled tfx oss sample * Update all google/cloud-sdk to latest * Try fixing parameterized tfx oss sample again * Also verify pipeline-runner ksa is working * Fix parameterized_tfx_oss sample * Update gcp-workload-identity-setup.sh * Revert unneeded change * Pin to new google/cloud-sdk * Remove wrongly commited binaries

Signed-off-by: ddelange <14880945+ddelange@users.noreply.github.com> Signed-off-by: ddelange <14880945+ddelange@users.noreply.github.com>

k8s-ci-robot assigned IronPan Nov 18, 2019

k8s-ci-robot requested review from gaoning777 and rmgogogo November 18, 2019 07:04

k8s-ci-robot added the size/L label Nov 18, 2019

Bobgy commented Nov 18, 2019

View reviewed changes

manifests/kustomize/gcp-workload-identity-setup.sh Outdated Show resolved Hide resolved

Bobgy changed the title ~~Migrate standalone deployment to workload identity on GCP~~ [WIP] Migrate standalone deployment to workload identity on GCP Nov 18, 2019

k8s-ci-robot added the do-not-merge/work-in-progress label Nov 18, 2019

k8s-ci-robot assigned rmgogogo Nov 19, 2019

k8s-ci-robot added the lgtm label Nov 19, 2019

k8s-ci-robot removed the lgtm label Nov 19, 2019

Bobgy added 13 commits November 27, 2019 15:00

Script to set up workload identity for standalone deployment

eae53c2

Migrate tests to run on standalone + workload identity

522f650

Fix test script

3942851

Switch to static GSAs for testing, because they have name length limit

152f48d

Add workload identity binding for argo

3ebd128

Fix argo workload identity bindings

9d32774

Remove user-gcp-sa from tests

282ca6f

Remove use_gcp_secret from xgboost sample

a36fca5

Allow debugging tests locally

1772d84

Wait for policies to take effect

1a7d1d3

Update deploy-pipeline-lite.sh

25b5e7b

Update deploy-pipeline-lite.sh

9956096

[WIP] test gcloud auth list with test-runner sa

f50fea2

k8s-ci-robot added size/XL and removed size/L labels Dec 10, 2019

k8s-ci-robot added the lgtm label Dec 12, 2019

Merge branch 'master' into wi_migration

5c85e06

k8s-ci-robot added size/XXL and removed lgtm size/XL labels Dec 16, 2019

Remove wrongly commited binaries

a163feb

Bobgy added 2 commits December 17, 2019 08:29

Merge remote-tracking branch 'official/master' into wi_migration

cc71ec5

Merge branch 'master' into wi_migration

f55a3cb

k8s-ci-robot added the lgtm label Dec 17, 2019

k8s-ci-robot merged commit 4a8d262 into kubeflow:master Dec 17, 2019

Bobgy deleted the wi_migration branch December 17, 2019 07:01

This was referenced Dec 18, 2019

[Testing] Upgrade gcp sdk version to reduce flakiness of workload identity #2744

Closed

Drop GCP credentials env if user-gcp-sa secret is not present #2643

Closed

This was referenced Dec 26, 2019

[GCP] pipelines supports workload identity #1691

Closed

[Sample] Clean up accidentally committed file #2775

Merged

Make user-gcp-sa secret optional on GCP #2589

Closed

Ark-kun reviewed Feb 6, 2020

View reviewed changes

Ark-kun reviewed Feb 11, 2020

View reviewed changes

jonasboecquaert mentioned this pull request Mar 4, 2020

build_image_from_working_dir fails with AccessDenied Error #3214

Closed

Ark-kun reviewed Oct 6, 2020

View reviewed changes

magdalenakuhn17 pushed a commit to magdalenakuhn17/pipelines that referenced this pull request Oct 22, 2023

Fix image name (kubeflow#2619)

0933f7c

Signed-off-by: ddelange <14880945+ddelange@users.noreply.github.com> Signed-off-by: ddelange <14880945+ddelange@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate standalone deployment to workload identity on GCP #2619

Migrate standalone deployment to workload identity on GCP #2619

Bobgy commented Nov 18, 2019 •

edited

Loading

Bobgy left a comment

rmgogogo commented Nov 19, 2019

rmgogogo commented Nov 19, 2019

Bobgy commented Nov 19, 2019

gaoning777 commented Nov 19, 2019

Bobgy commented Nov 20, 2019 •

edited

Loading

Bobgy commented Nov 22, 2019

Bobgy commented Nov 27, 2019

Bobgy commented Dec 10, 2019

rmgogogo commented Dec 12, 2019

Bobgy commented Dec 16, 2019

Bobgy commented Dec 17, 2019

Bobgy commented Dec 17, 2019

rmgogogo commented Dec 17, 2019

Ark-kun Feb 6, 2020

Bobgy Feb 7, 2020

Bobgy Feb 7, 2020

Ark-kun Feb 11, 2020

Bobgy Feb 11, 2020

Ark-kun Oct 6, 2020

Bobgy Oct 12, 2020

Migrate standalone deployment to workload identity on GCP #2619

Migrate standalone deployment to workload identity on GCP #2619

Conversation

Bobgy commented Nov 18, 2019 • edited Loading

Bobgy left a comment

Choose a reason for hiding this comment

rmgogogo commented Nov 19, 2019

rmgogogo commented Nov 19, 2019

Bobgy commented Nov 19, 2019

gaoning777 commented Nov 19, 2019

Bobgy commented Nov 20, 2019 • edited Loading

Bobgy commented Nov 22, 2019

Bobgy commented Nov 27, 2019

Bobgy commented Dec 10, 2019

rmgogogo commented Dec 12, 2019

Bobgy commented Dec 16, 2019

Bobgy commented Dec 17, 2019

Bobgy commented Dec 17, 2019

rmgogogo commented Dec 17, 2019

Ark-kun Feb 6, 2020

Choose a reason for hiding this comment

Bobgy Feb 7, 2020

Choose a reason for hiding this comment

Bobgy Feb 7, 2020

Choose a reason for hiding this comment

Ark-kun Feb 11, 2020

Choose a reason for hiding this comment

Bobgy Feb 11, 2020

Choose a reason for hiding this comment

Ark-kun Oct 6, 2020

Choose a reason for hiding this comment

Bobgy Oct 12, 2020

Choose a reason for hiding this comment

Bobgy commented Nov 18, 2019 •

edited

Loading

Bobgy commented Nov 20, 2019 •

edited

Loading