Skip to content

Fix pipeline default service account#969

Merged
k8s-ci-robot merged 2 commits into
kubeflow:masterfrom
Bobgy:kfp_fix_pipeline_sa
Mar 3, 2020
Merged

Fix pipeline default service account#969
k8s-ci-robot merged 2 commits into
kubeflow:masterfrom
Bobgy:kfp_fix_pipeline_sa

Conversation

@Bobgy

@Bobgy Bobgy commented Mar 2, 2020

Copy link
Copy Markdown
Contributor

Which issue is resolved by this Pull Request:
Resolves kubeflow/kubeflow#4803

Description of your changes:

  1. Use kf-user as pipeline default sa.
  2. Bind pipeline-runner clusterrole to kf-user to avoid breaking existing usages.

Checklist:

  • Unit tests have been rebuilt:
    1. cd manifests/tests
    2. make generate-changed-only
    3. make test

This change is Reviewable

@Bobgy

Bobgy commented Mar 2, 2020

Copy link
Copy Markdown
Contributor Author

/assign @jlewi

@Bobgy

Bobgy commented Mar 2, 2020

Copy link
Copy Markdown
Contributor Author

/retest
The error doesn't seem related

@Bobgy

Bobgy commented Mar 2, 2020

Copy link
Copy Markdown
Contributor Author

Strange, I got error:

util.py                     72 INFO     failed to apply:  (kubeflow.error): Code 500 with message: kfApp Apply failed for kustomize:  (kubeflow.error): Code 500 w
ith message: Apply.Run  Error error when creating "/tmp/kout809907334": ClusterRoleBinding.rbac.authorization.k8s.io "pipeline-runner" is invalid: subjects[1].nam
espace: Required value

I guess when referring to a service account not defined in the same kustomize package, it must have a namespace.

@Bobgy

Bobgy commented Mar 2, 2020

Copy link
Copy Markdown
Contributor Author

I'm not sure this is the right fix, because it seems https://github.com/kubeflow/kfctl/search?q=kf-user&unscoped_q=kf-user kf-user sa only exists in gcp and aws clusters.

I think we'd need a solution to let each application flexibly define what service account needs what binding in their yamls. KCC can support that for GCP, we'd need something similar.

@Bobgy

Bobgy commented Mar 2, 2020

Copy link
Copy Markdown
Contributor Author

The error message seems irrelevant, will retry again.
/retest

# temporarily switched to kf-user, because pipeline-runner isn't bound to workload identity by default
- kind: ServiceAccount
name: kf-user
namespace: kubeflow

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jlewi Should I use var replace here?

@Bobgy

Bobgy commented Mar 2, 2020

Copy link
Copy Markdown
Contributor Author

The failed step cannot pull image: gcr.io/kubeflow-ci/test-worker-py3:789005d.
I checked on gcr.io, the image doesn't exist, seems like we are pinning to an invalid image.

@IronPan

IronPan commented Mar 2, 2020

Copy link
Copy Markdown
Member

/lgtm

@gabrielwen

Copy link
Copy Markdown
Contributor

/retest

@gabrielwen

Copy link
Copy Markdown
Contributor

@Bobgy the test is fixed :)

@jlewi

jlewi commented Mar 3, 2020

Copy link
Copy Markdown
Contributor

/approve

@k8s-ci-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlewi

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot merged commit ad38b41 into kubeflow:master Mar 3, 2020
@Bobgy Bobgy deleted the kfp_fix_pipeline_sa branch March 3, 2020 00:18
@Bobgy

Bobgy commented Mar 3, 2020

Copy link
Copy Markdown
Contributor Author

Thanks! @jlewi @gabrielwen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

KFP builtin samples fail in KF1.0

6 participants