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.

Sign up for GitHub

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

Jump to bottom

[Feature] Update Kubeflow Training Operator 1.9.0 #4066

Merged
merged 2 commits into from
Jan 28, 2025
Merged

[Feature] Update Kubeflow Training Operator 1.9.0 #4066

merged 2 commits into from
Jan 28, 2025

Conversation

mszadkow
Copy link
Contributor

@mszadkow mszadkow commented Jan 27, 2025 •
edited by k8s-ci-robot
Loading

What type of PR is this?

/kind feature

What this PR does / why we need it:

Update is required to finally support ManagedBy feature of Kubeflow related jobs.

Which issue(s) this PR fixes:

Relates to #2552

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 27, 2025
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 27, 2025
@netlify Netlify
Copy link

netlify bot commented Jan 27, 2025 •
edited
Loading

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name Link
🔨 Latest commit c002521
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/67990f5624a9c4000843a927

@mszadkow mszadkow mentioned this pull request Jan 27, 2025
Closed
@mszadkow
Copy link
Contributor Author

/cc @mimowo

@k8s-ci-robot k8s-ci-robot requested a review from mimowo January 27, 2025 11:23
@mszadkow
Copy link
Contributor Author

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jan 27, 2025
@mszadkow mszadkow force-pushed the feature/kubeflow-update-remove-mxjob branch from 2b22561 to 56e7931 Compare January 27, 2025 12:00
@mimowo
Copy link
Contributor

mimowo commented Jan 27, 2025

/cc @mbobrovskyi for the first pass

@k8s-ci-robot
Copy link
Contributor

@mimowo: GitHub didn't allow me to request PR reviews from the following users: the, first, pass, for.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @mbobrovskyi for the first pass

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-sigs/prow repository.

@mszadkow mszadkow force-pushed the feature/kubeflow-update-remove-mxjob branch from 56e7931 to f3ec223 Compare January 27, 2025 13:52
@mszadkow
Copy link
Contributor Author

/retest

@mszadkow mszadkow marked this pull request as ready for review January 27, 2025 14:03
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 27, 2025
@mszadkow mszadkow force-pushed the feature/kubeflow-update-remove-mxjob branch from f3ec223 to 9fa7049 Compare January 27, 2025 14:18
@mszadkow
Copy link
Contributor Author

/retest

@kannon92
Copy link
Contributor

Should we not merge this until Kubeflow Training Operator has an offical release version?

@mbobrovskyi
Copy link
Contributor

mbobrovskyi commented Jan 28, 2025 •
edited
Loading

Does it fix #1429?

@mimowo
Copy link
Contributor

mimowo commented Jan 28, 2025

I think it is ok to merge using rc1, afaik keueue is only using the API from kubeflow and this should not change at this point.

Merging this PR would make it easier to work on the follow up which is the support for managedBy in Kueue.

While I don't consider it a strict requirement, I would prefer we release Keueue 0.11 in mid March using the full release of Kubeflow. @tenzen-y do you know when the full release is planned?

@mbobrovskyi
Copy link
Contributor

/hold

Wait for stable release v1.9.x.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 28, 2025
@mimowo
Copy link
Contributor

mimowo commented Jan 28, 2025

Any particular reason @mbobrovskyi you suggest to wait for 1.9.x, do you mean x>0?

Actually, in the past we used to compile against "RC" releases of the frameworks:

  • 0.5.0 was built using training-operator 1.7.0 release candidate link
  • 0.6.0 was built using KubeRay alpha link

Ideally, we use full releases, but I don't think we need to be strict about that. IIUC we only use APIs of these projects, and these would rarely change between candidate and full release. Let me know @tenzen-y @dgrove-oss if you have some opinion here.

@mimowo mimowo mentioned this pull request Jan 28, 2025
Open
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: e7af9f630a9323897a0e6d619d561edddfd299f0

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 28, 2025
@tenzen-y
Copy link
Member

/release-note-edit

NONE

I think that we can drop the release note since we have a dedicated PR to drop the MXJob supports.

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jan 28, 2025
@tenzen-y
Copy link
Member

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 28, 2025
@mszadkow mszadkow changed the title [Feature] Update Kubeflow Training Operator 1.9.0-rc0 [Feature] Update Kubeflow Training Operator 1.9.0 Jan 28, 2025
@mszadkow mszadkow changed the title [Feature] Update Kubeflow Training Operator 1.9.0 [Feature] Update Kubeflow Training Operator 1.9.0-rc0 Jan 28, 2025
@mszadkow
Copy link
Contributor Author

Will update to v1.9.0 as released https://github.com/kubeflow/training-operator/releases/tag/v1.9.0

@mszadkow mszadkow force-pushed the feature/kubeflow-update-remove-mxjob branch from 16ac548 to bdff060 Compare January 28, 2025 16:53
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 28, 2025
@k8s-ci-robot k8s-ci-robot requested a review from tenzen-y January 28, 2025 16:53
@mszadkow mszadkow changed the title [Feature] Update Kubeflow Training Operator 1.9.0-rc0 [Feature] Update Kubeflow Training Operator 1.9.0 Jan 28, 2025
@tenzen-y
Copy link
Member

Will update to v1.9.0 as released https://github.com/kubeflow/training-operator/releases/tag/v1.9.0

I'm ok with relying on a bot. A bot will update it to v1.9.0 in the next Monday.

@tenzen-y
Copy link
Member

Will update to v1.9.0 as released https://github.com/kubeflow/training-operator/releases/tag/v1.9.0

I'm ok with relying on a bot. A bot will update it to v1.9.0 in the next Monday.

Oh, you already have addressed that. Awesome! Thank you!
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 28, 2025
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 5460cb29c4c7e821d45459557442f43e5104c233

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mszadkow, tenzen-y

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

@tenzen-y
Copy link
Member

@mszadkow It seems that we need to perform with go mod tidy.

@mszadkow mszadkow force-pushed the feature/kubeflow-update-remove-mxjob branch from bdff060 to c002521 Compare January 28, 2025 17:09
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 28, 2025
@tenzen-y
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 28, 2025
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: a6d65dd2951117ca5bd6ebc06517eb7d6293e616

@k8s-ci-robot k8s-ci-robot merged commit fbc9017 into kubernetes-sigs:main Jan 28, 2025
18 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v0.11 milestone Jan 28, 2025
FillZpp pushed a commit to leptonai/kueue that referenced this pull request Feb 5, 2025
@mszadkow @FillZpp
[Feature] Update Kubeflow Training Operator 1.9.0 (kubernetes-sigs#4066)
ba550e5 
* Update Training Operator pkg to v1.9.0

* Add --server-side for Kubeflow CRDs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbobrovskyi mbobrovskyi mbobrovskyi left review comments

@PBundyra PBundyra Awaiting requested review from PBundyra

@mimowo mimowo Awaiting requested review from mimowo

@tenzen-y tenzen-y Awaiting requested review from tenzen-y

Assignees

@tenzen-y tenzen-y

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v0.11
Development

Successfully merging this pull request may close these issues.
6 participants
@mszadkow @k8s-ci-robot @mimowo @kannon92 @mbobrovskyi @tenzen-y