Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Backend + SDK): Update kfp backend and kubernetes sdk to support pod labels and annotations #10393

Merged
merged 11 commits into from
Feb 9, 2024

Conversation

Tomcli
Copy link
Member

@Tomcli Tomcli commented Jan 12, 2024

Description of your changes:
Part of #9714 and #9768
Fixes #10363

Update kfp backend and kubernetes sdk to support pod labels and annotations.
The pod metadata is not part of the pod spec, we can't simply update the podSpecPatch for Argo, so we need to parse the kubernetes_platform spec in the compiler and add the pod labels and annotations in the Argo container definition.

Also fixing go lint style from #10288

Checklist:

Copy link
Member

@connor-mccarthy connor-mccarthy left a comment

Choose a reason for hiding this comment

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

SDK changes LGTM! Thank you!

/assign @chensun for backend changes review.

@Tomcli
Copy link
Member Author

Tomcli commented Jan 18, 2024

/assign @chensun
Let me know if you need any other changes in the backend. Thank you.

@roytman
Copy link
Contributor

roytman commented Jan 26, 2024

Thank you @Tomcli
/lgtm

Copy link

@roytman: changing LGTM is restricted to collaborators

In response to this:

Thank you @Tomcli
/lgtm

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.

@Tomcli
Copy link
Member Author

Tomcli commented Jan 26, 2024

@chensun Could you help review this PR? Thank you.

Copy link
Member

@chensun chensun left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

Thanks, @Tomcli !

@google-oss-prow google-oss-prow bot removed the lgtm label Feb 9, 2024
Copy link

[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:

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

@chensun
Copy link
Member

chensun commented Feb 9, 2024

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Feb 9, 2024
Copy link

@Tomcli: 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 133ccb1 link false /test kubeflow-pipelines-samples-v2
kfp-kubernetes-execution-tests 133ccb1 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.

@google-oss-prow google-oss-prow bot merged commit b3978c1 into kubeflow:master Feb 9, 2024
11 of 13 checks passed
@Tomcli Tomcli deleted the add-podmetadata-v2 branch February 9, 2024 21:15
roytman pushed a commit to roytman/pipelines that referenced this pull request Feb 14, 2024
… pod labels and annotations (kubeflow#10393)

* update kfp kubernetes sdk to include pod labels and annotations

* fix unit test output order

* add podmetadata changes

* update argo compiler to support pod metadata

* update tests

* update go mod to use the latest kubernetes_platform package

* update licenses

* address comments

* update kubernetes_platform package to include the latest spec

---------

Co-authored-by: Chen Sun <chensun@users.noreply.github.com>
stijntratsaertit pushed a commit to stijntratsaertit/kfp that referenced this pull request Feb 16, 2024
… pod labels and annotations (kubeflow#10393)

* update kfp kubernetes sdk to include pod labels and annotations

* fix unit test output order

* add podmetadata changes

* update argo compiler to support pod metadata

* update tests

* update go mod to use the latest kubernetes_platform package

* update licenses

* address comments

* update kubernetes_platform package to include the latest spec

---------

Co-authored-by: Chen Sun <chensun@users.noreply.github.com>
@pffijt
Copy link

pffijt commented Mar 18, 2024

Can you release a new version of kfp-kubernetes? We are missing this functionality

petethegreat pushed a commit to petethegreat/pipelines that referenced this pull request Mar 27, 2024
… pod labels and annotations (kubeflow#10393)

* update kfp kubernetes sdk to include pod labels and annotations

* fix unit test output order

* add podmetadata changes

* update argo compiler to support pod metadata

* update tests

* update go mod to use the latest kubernetes_platform package

* update licenses

* address comments

* update kubernetes_platform package to include the latest spec

---------

Co-authored-by: Chen Sun <chensun@users.noreply.github.com>
petethegreat pushed a commit to petethegreat/pipelines that referenced this pull request Mar 29, 2024
… pod labels and annotations (kubeflow#10393)

* update kfp kubernetes sdk to include pod labels and annotations

* fix unit test output order

* add podmetadata changes

* update argo compiler to support pod metadata

* update tests

* update go mod to use the latest kubernetes_platform package

* update licenses

* address comments

* update kubernetes_platform package to include the latest spec

---------

Co-authored-by: Chen Sun <chensun@users.noreply.github.com>
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.

[feature] Apply Pod Labels and Pod Annotations for Kubeflow Pipeline Tasks
5 participants