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

feat: Volcano Supports K8s v1.31 #3767

Merged
merged 4 commits into from
Oct 23, 2024
Merged

feat: Volcano Supports K8s v1.31 #3767

merged 4 commits into from
Oct 23, 2024

Conversation

vie-serendipity
Copy link
Contributor

@vie-serendipity vie-serendipity commented Oct 12, 2024 •
edited
Loading

I split the content into four commits.

  1. change go mod
  2. upgrade KUBE_VERSION
  3. solve some api changes after adaptation
  4. sync changes regarding volumebing

I have two tasks to be completed.

  • introduce the updated version of volcano apis, because the pr hasn't been merged yet
  • update README after running e2e tests, unit tests has passed already

@volcano-sh-bot
Copy link
Contributor

Welcome @vie-serendipity!

It looks like this is your first PR to volcano-sh/volcano.

Thank you, and welcome to Volcano. 😃

@volcano-sh-bot volcano-sh-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 12, 2024
@Monokaix
Copy link
Member

great job!

@Monokaix
Copy link
Member

/ok-to-test

@volcano-sh-bot volcano-sh-bot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Oct 16, 2024
@vie-serendipity
Copy link
Contributor Author

I'll fix these issues to pass the test.

@Monokaix
Copy link
Member

You can also refer to 1.30 adaptation #3556 : )

@vie-serendipity vie-serendipity force-pushed the feat/adapt-1.31 branch 2 times, most recently from 1754291 to d50b35b Compare October 16, 2024 11:33
Monokaix
Monokaix reviewed Oct 18, 2024
View reviewed changes
pkg/scheduler/capabilities/volumebinding/assume_cache_test.go
@@ -24,9 +24,10 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/component-helpers/storage/volume"
"k8s.io/klog/v2/ktesting"
"k8s.io/kubernetes/pkg/scheduler/util/assumecache"
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for your contribution！are volumebinding codes synced from k8s?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@Monokaix
Copy link
Member

Please notice that to keep API compatibility，we should modify the code CSIStorageCapacityInformer: fh.SharedInformerFactory().Storage().V1().CSIStorageCapacities() in file pkg/scheduler/capabilities/volumebinding/volume_binding.go, and change from V1() to V1beta1() and test whether v1beat1 is supported in k8s v1.31: )

@vie-serendipity
Copy link
Contributor Author

@Monokaix Thanks for your comment, I've already updated the volume binding-related code and changed v1 to v1beta1.

@Monokaix
Copy link
Member

Please update the task lists in description: )

@Monokaix
Copy link
Member

/lgtm

@volcano-sh-bot volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 21, 2024
@Monokaix
Copy link
Member

cc @wangyang0616 @lowang-bh @hwdef

hwdef
hwdef reviewed Oct 21, 2024
View reviewed changes
.github/workflows/e2e_vcctl.yaml
GO111MODULE="on" go install sigs.k8s.io/kind@v0.23.0
curl -LO https://storage.googleapis.com/kubernetes-release/release/v1.30.1/bin/linux/amd64/kubectl && sudo install kubectl /usr/local/bin/kubectl
GO111MODULE="on" go install sigs.k8s.io/kind@v0.24.0
curl -LO https://storage.googleapis.com/kubernetes-release/release/v1.31.0/bin/linux/amd64/kubectl && sudo install kubectl /usr/local/bin/kubectl
Copy link
Member

Choose a reason for hiding this comment

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

why downgrade this

Copy link
Member

Choose a reason for hiding this comment

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

seems no downgrade: )

Copy link
Member

Choose a reason for hiding this comment

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

Sorry I misread it.

@hwdef
Copy link
Member

hwdef commented Oct 21, 2024

please squash your commit

@volcano-sh-bot volcano-sh-bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 21, 2024
@vie-serendipity
Copy link
Contributor Author

@hwdef I've already squashed them into four, do I need to squash my commits directly into one?

@hwdef
Copy link
Member

hwdef commented Oct 21, 2024

/lgtm

@volcano-sh-bot volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 21, 2024
@volcano-sh-bot volcano-sh-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm Indicates that a PR is ready to be merged. labels Oct 22, 2024
@vie-serendipity
feat: change gomod to adapt to k8s 1.31
51afd93 
Signed-off-by: vie-serendipity <2733147505@qq.com>

feat: solve problems that arise after adaptation

Signed-off-by: vie-serendipity <2733147505@qq.com>
@vie-serendipity
feat: sync the changes in k8s regarding volumebinding
d38bc3e 
Signed-off-by: vie-serendipity <2733147505@qq.com>
@vie-serendipity
chore: update licenses, make manifests, and upgrade the version of co…
2cf0cbe 
…ntroller gen

Signed-off-by: vie-serendipity <2733147505@qq.com>

feat: upgrade version of kind and kubectl

feat: upgrade controller-gen to 0.16.4 and make manifests

Signed-off-by: vie-serendipity <2733147505@qq.com>

feat: upgrade apis and update readme

Signed-off-by: vie-serendipity <2733147505@qq.com>
@vie-serendipity
feat: update volumebinding codes with 1.31 k8s
efc379c 
Signed-off-by: vie-serendipity <2733147505@qq.com>

feat: add capacityCheckEnabled back

Signed-off-by: vie-serendipity <2733147505@qq.com>

feat: remove klog 1.0
Signed-off-by: vie-serendipity <2733147505@qq.com>
@volcano-sh-bot volcano-sh-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 22, 2024
@Monokaix
Copy link
Member

/lgtm
/approve

@volcano-sh-bot volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2024
@volcano-sh-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Monokaix

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

@volcano-sh-bot volcano-sh-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 23, 2024
@volcano-sh-bot volcano-sh-bot merged commit 1ab6088 into volcano-sh:master Oct 23, 2024
17 checks passed
@ouyangshengjia
Copy link

/lgtm

@Monokaix Monokaix mentioned this pull request Nov 14, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alcorj-mizar alcorj-mizar Awaiting requested review from alcorj-mizar

@jiangkaihua jiangkaihua Awaiting requested review from jiangkaihua

@hwdef hwdef Awaiting requested review from hwdef

@ouyangshengjia ouyangshengjia Awaiting requested review from ouyangshengjia

@Monokaix Monokaix Awaiting requested review from Monokaix

Assignees

@hwdef hwdef

@Monokaix Monokaix

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm 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. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vie-serendipity @volcano-sh-bot @Monokaix @hwdef @ouyangshengjia