Volcano adapts to the k8s v1.25 #2533

wangyang0616 · 2022-10-13T03:00:24Z

Signed-off-by: wangyang wangyang289@huawei.com

/kind feature #2510

Volcano adapts to the k8s v1.25
After Volcano is upgraded to K8S V1.25, the volumebinding package is imported to Volcano to ensure that csiStorageCapacity is compatible with K8S v1.23, v1.21, and v1.19. Use the csiStorageCapacity API of the v1beta1 version in the volumebinding package and do not upgrade the v1 version.

volcano-sh-bot · 2022-10-13T03:00:28Z

@wangyang0616: The label(s) kind/[#2510](https://github.com/volcano-sh/volcano/issues/2510) cannot be applied. These labels are supported: ``

In response to this:

Signed-off-by: wangyang wangyang289@huawei.com

/kind feature #2510

Volcano adapts to the k8s v1.25

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.

Thor-wl

Is it nessary to update the volcano.sh/apis with k8s v1.25 first?

wangyang0616 · 2022-10-18T09:32:10Z

Is it nessary to update the volcano.sh/apis with k8s v1.25 first?

It is true that API changes should be incorporated first.
The API has been incorporated. The volcano.sh/api version is referenced in the current PR.

volcano.sh/apis adapts to K8S V1.25 volcano-sh/apis#93

volcano-sh-bot · 2022-10-19T01:17:57Z

@wangyang0616: The label(s) kind/[#2510](https://github.com/volcano-sh/volcano/issues/2510) cannot be applied. These labels are supported: ``

In response to this:

Signed-off-by: wangyang wangyang289@huawei.com

/kind feature #2510

Volcano adapts to the k8s v1.25

After Volcano is upgraded to K8S V1.25, the volumebinding package is imported to Volcano to ensure that csiStorageCapacity is compatible with K8S v1.23, v1.21, and v1.19. Use the csiStorageCapacity API of the v1beta1 version in the volumebinding package and do not upgrade the v1 version.

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.

jinzhejz · 2022-10-27T01:57:35Z

pkg/scheduler/plugins/volumebinding/volume_binding.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"sync"
 	"time"
+	"volcano.sh/volcano/cmd/scheduler/app/options"


suggest to move package volcano.sh/volcano/cmd/scheduler/app/options behind k8s.io

In K8S V1.25, the EnableCSIStorageCapacity feature in the k8s.io/kubernetes/pkg/scheduler/framework/plugins/feature package is removed and cannot be used.

After the Volcano adapts to K8S V1.25, to ensure compatibility of earlier versions (K8S V1.21 and V1.23), the Volcano internal feature switch options.ServerOpts.EnableCSIStorage is still used, that is, volcano.sh/volcano/cmd/scheduler/app/options is introduced.

i guess its meaning go fmt is needed.

i guess its meaning go fmt is needed.

Thank you. I misunderstood.
It's been revised.

jiangkaihua · 2022-10-27T03:04:30Z

pkg/scheduler/plugins/volumebinding/binder.go

@@ -25,6 +25,7 @@ import (

 	v1 "k8s.io/api/core/v1"
 	storagev1 "k8s.io/api/storage/v1"
+	storagev1beta1 "k8s.io/api/storage/v1beta1"


Both v1 and v1beta1 is reserved in references, would they disturbe each other?

The usage of k8s v1.23 is still used, and no conflict occurs. CSIStorageCapacity uses v1beta1 and other storage interfaces use v1. Ensure the forward compatibility after the Volcano is upgraded to K8S V1.25 (for example, K8S V1.23 and V1.21).

jiangkaihua · 2022-10-27T03:22:38Z

pkg/scheduler/plugins/volumebinding/binder.go

@@ -227,7 +231,7 @@ func NewVolumeBinder(
 	pvcInformer coreinformers.PersistentVolumeClaimInformer,
 	pvInformer coreinformers.PersistentVolumeInformer,
 	storageClassInformer storageinformers.StorageClassInformer,
-	capacityCheck CapacityCheck,
+	capacityCheck *CapacityCheck,


Why does capacityCheck is modified to pointer?

Why does capacityCheck is modified to pointer?

The Volcano uses the feature switch (options.ServerOpts.EnableCSIStorage) to control the use of the CSIStorageCapacity function. For details, see the implementation of K8S V1.23. The value of CapacityCheck is changed back to the pointer type.

If the CSIStorageCapacity function is not enabled, the pointer is null. The informer is not initialized when the CSIStorageCapacity function is enabled. After the CSIStorageCapacity function is enabled, the external invoker can assign a value to the CapacityCheck pointer. The informer is initialized when the CSIStorageCapacity function is enabled.

jiangkaihua · 2022-10-27T03:45:49Z

cmd/controller-manager/app/server.go

@@ -87,7 +87,7 @@ func Run(opt *options.ServerOption) error {
 	// add a uniquifier so that two processes on the same host don't accidentally both become active
 	id := hostname + "_" + string(uuid.NewUUID())

-	rl, err := resourcelock.New(resourcelock.ConfigMapsResourceLock,
+	rl, err := resourcelock.New(resourcelock.ConfigMapsLeasesResourceLock,


Why locktype is changed from configmaps to configmapsleases?

In K8S V1.25, configMapsResourceLock is removed and replaced by ConfigMapsLeasesResourceLock.

wangyang0616 · 2022-10-27T06:43:28Z

Is it nessary to update the volcano.sh/apis with k8s v1.25 first?

The volcano.sh/apis referenced by the current backbone can still use the APIs of K8S V1.23. When version 1.7.0 is released, volcano.sh/apis will be upgraded to K8S V1.25.

volcano.sh/apis adapts to K8S V1.25 volcano-sh/apis#93

#2533 (comment)

It is true that API changes should be incorporated first.
The API has been incorporated. The volcano.sh/api version is referenced in the current PR.

jiangkaihua · 2022-10-27T07:17:04Z

/lgtm

volcano-sh-bot · 2022-10-27T07:17:13Z

@jiangkaihua: changing LGTM is restricted to collaborators

In response to this:

/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.

Signed-off-by: wangyang <wangyang289@huawei.com>

…rsions. Signed-off-by: wangyang <wangyang289@huawei.com>

william-wang · 2022-10-28T03:06:01Z

pkg/scheduler/plugins/predicates/predicates.go

 	podAffinityFilter := plugin.(*interpodaffinity.InterPodAffinity)
 	// 6. NodeVolumeLimits
+	features := feature.Features{}


Does feature struct contain a lot of sub-feature?

Copy that.
Feature.Features{} needs to be initialized and assigned with a value.

Modified.

william-wang · 2022-10-28T03:07:52Z

pkg/scheduler/plugins/predicates/predicates.go

@@ -398,7 +398,8 @@ func (pp *predicatesPlugin) OnSessionOpen(ssn *framework.Session) {
 		}

 		// InterPodAffinity Predicate
-		status = podAffinityFilter.PreFilter(context.TODO(), state, task.Pod)
+		// TODO use framework.PreFilterResult


Do you want to add some thing more here? if not, please delete useless codes or comments.

Copy that. Modified.

william-wang · 2022-10-28T03:08:07Z

pkg/scheduler/plugins/predicates/predicates.go

@@ -415,7 +416,8 @@ func (pp *predicatesPlugin) OnSessionOpen(ssn *framework.Session) {
 		}

 		// Check VolumeBinding: handle immediate claims unbounded case
-		status = volumebindingFilter.PreFilter(context.TODO(), state, task.Pod)
+		// TODO use framework.PreFilterResult


The same with above

Copy that. Modified.

william-wang · 2022-10-28T03:08:16Z

pkg/scheduler/plugins/predicates/predicates.go

@@ -433,7 +435,8 @@ func (pp *predicatesPlugin) OnSessionOpen(ssn *framework.Session) {
 		}

 		// Check PodTopologySpread
-		status = podTopologySpreadFilter.PreFilter(context.TODO(), state, task.Pod)
+		// TODO use framework.PreFilterResult


The save as above

Copy that. Modified.

william-wang · 2022-10-28T03:13:08Z

pkg/scheduler/cache/cache.go

@@ -50,7 +50,7 @@ import (
 	"k8s.io/client-go/util/workqueue"
 	"k8s.io/klog"
 	podutil "k8s.io/kubernetes/pkg/api/v1/pod"
-	volumescheduling "k8s.io/kubernetes/pkg/scheduler/framework/plugins/volumebinding"
+	volumescheduling "volcano.sh/volcano/pkg/scheduler/plugins/volumebinding"


cache should not depend plugin

suggest add capabilities directory like pkg/scheduler/capabilities/volumebinding and put our volumebinding there

Copy that. Modified.

william-wang · 2022-10-28T03:51:40Z

pkg/scheduler/plugins/nodeorder/nodeorder.go

-		EnableReadWriteOncePod:             utilFeature.DefaultFeatureGate.Enabled(features.ReadWriteOncePod),
-		EnableVolumeCapacityPriority:       utilFeature.DefaultFeatureGate.Enabled(features.VolumeCapacityPriority),
-		EnableCSIStorageCapacity:           utilFeature.DefaultFeatureGate.Enabled(features.CSIStorageCapacity),
+		EnableReadWriteOncePod:                       utilFeature.DefaultFeatureGate.Enabled(features.ReadWriteOncePod),


EnablePodDisruptionBudget is removed?

The PodDisruptionBudget feature has passed GA. The EnablePodDisruptionBudget feature is removed in 1.25.

william-wang · 2022-10-28T03:52:43Z

pkg/scheduler/plugins/nodeorder/nodeorder.go

@@ -362,7 +361,7 @@ func (pp *nodeOrderPlugin) OnSessionOpen(ssn *framework.Session) {
 	ssn.AddNodeOrderFn(pp.Name(), nodeOrderFn)

 	plArgs := &config.InterPodAffinityArgs{}
-	p, _ = interpodaffinity.New(plArgs, handle, fts)
+	p, _ = interpodaffinity.New(plArgs, handle)


what's the reason of deleting fts for interpodaffinity?

The PodAffinityNamespaceSelector is GA in V1.24. The switch for this feature is deleted.

In this case, the fst parameter is not transferred when the PodAffinityNamespaceSelector is new.

william-wang · 2022-10-28T03:53:04Z

pkg/scheduler/plugins/nodeorder/nodeorder.go

@@ -371,7 +370,7 @@ func (pp *nodeOrderPlugin) OnSessionOpen(ssn *framework.Session) {
 	ptsArgs := &config.PodTopologySpreadArgs{
 		DefaultingType: config.SystemDefaulting,
 	}
-	p, _ = podtopologyspread.New(ptsArgs, handle)
+	p, _ = podtopologyspread.New(ptsArgs, handle, fts)


what's the reason of adding fts to podtopologyspread?

In K8S V1.25, the alpha and beta feature switches related to TopologySpread are added, for example, EnableMinDomainsInPodTopologySpread, EnableNodeInclusionPolicyInPodTopologySpread, EnableMatchLabelKeysInPodTopologySpread.

…che on the plugin. Signed-off-by: wangyang <wangyang289@huawei.com>

william-wang

/lgtm

volcano-sh-bot · 2022-10-31T01:57:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: william-wang

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:

~~OWNERS~~ [william-wang]

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 added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. labels Oct 13, 2022

volcano-sh-bot requested review from alcorj-mizar, merryzhou and shinytang6 October 13, 2022 03:00

volcano-sh-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 13, 2022

wangyang0616 force-pushed the feature_support_k8s_v1.25 branch from 047a109 to ed07e12 Compare October 14, 2022 09:21

Thor-wl reviewed Oct 14, 2022

View reviewed changes

wangyang0616 force-pushed the feature_support_k8s_v1.25 branch 3 times, most recently from 42d931b to cf1dd30 Compare October 18, 2022 09:14

wangyang0616 marked this pull request as ready for review October 18, 2022 11:37

volcano-sh-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 18, 2022

wangyang0616 force-pushed the feature_support_k8s_v1.25 branch 3 times, most recently from 54e727b to d390372 Compare October 19, 2022 01:07

wangyang0616 force-pushed the feature_support_k8s_v1.25 branch from 2ed5181 to e00f74a Compare October 19, 2022 03:47

wangyang0616 mentioned this pull request Oct 21, 2022

Volcano adapts to the k8s v1.25 volcano-sh/apis#93

Merged

wangyang0616 force-pushed the feature_support_k8s_v1.25 branch 2 times, most recently from e130f7a to 981c423 Compare October 22, 2022 03:15

jinzhejz reviewed Oct 27, 2022

View reviewed changes

jiangkaihua reviewed Oct 27, 2022

View reviewed changes

wangyang0616 added 2 commits October 28, 2022 10:00

Volcano adapts to the k8s v1.25

0ca936c

Signed-off-by: wangyang <wangyang289@huawei.com>

The volume binding function is applicable to K8S v1.23 and earlier ve…

c414ca5

…rsions. Signed-off-by: wangyang <wangyang289@huawei.com>

wangyang0616 force-pushed the feature_support_k8s_v1.25 branch from cae2f30 to c414ca5 Compare October 28, 2022 02:05

william-wang reviewed Oct 28, 2022

View reviewed changes

wangyang0616 force-pushed the feature_support_k8s_v1.25 branch 5 times, most recently from 4ef6e8d to b8dbeb1 Compare October 29, 2022 02:18

Migrate the volume binding location to avoid the dependency of the ca…

cd7a58f

…che on the plugin. Signed-off-by: wangyang <wangyang289@huawei.com>

wangyang0616 force-pushed the feature_support_k8s_v1.25 branch from b8dbeb1 to cd7a58f Compare October 29, 2022 02:22

william-wang approved these changes Oct 31, 2022

View reviewed changes

volcano-sh-bot assigned william-wang Oct 31, 2022

volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 31, 2022

volcano-sh-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 31, 2022

volcano-sh-bot merged commit c7d87c7 into volcano-sh:master Oct 31, 2022

wangyang0616 mentioned this pull request Dec 27, 2022

Request to be a member of Volcano community volcano-sh/community#36

Closed

wangyang0616 mentioned this pull request Jul 29, 2023

Request to be an Reviewer of Volcano community volcano-sh/community#41

Closed

Volcano adapts to the k8s v1.25 #2533

Volcano adapts to the k8s v1.25 #2533

Conversation

wangyang0616 commented Oct 13, 2022 • edited Loading

volcano-sh-bot commented Oct 13, 2022

Thor-wl left a comment

Choose a reason for hiding this comment

wangyang0616 commented Oct 18, 2022 • edited Loading

volcano-sh-bot commented Oct 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wangyang0616 commented Oct 27, 2022

jiangkaihua commented Oct 27, 2022

volcano-sh-bot commented Oct 27, 2022

Choose a reason for hiding this comment

wangyang0616 Oct 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

william-wang left a comment

Choose a reason for hiding this comment

volcano-sh-bot commented Oct 31, 2022

wangyang0616 commented Oct 13, 2022 •

edited

Loading

wangyang0616 commented Oct 18, 2022 •

edited

Loading

wangyang0616 Oct 29, 2022 •

edited

Loading