Skip to content

Commit c84b1a8

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#86601 from angao/no-disk-conflict
move NoDiskConflict predicate to its filter plugin
2 parents bd1195c + e5d90c5 commit c84b1a8

File tree

5 files changed

+88
-324
lines changed

5 files changed

+88
-324
lines changed

‎pkg/scheduler/algorithm/predicates/predicates.go‎

Lines changed: 0 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -147,73 +147,6 @@ func Ordering() []string {
147147
// The failure information is given by the error.
148148
type FitPredicate func(pod *v1.Pod, meta Metadata, nodeInfo *schedulernodeinfo.NodeInfo) (bool, []PredicateFailureReason, error)
149149

150-
func isVolumeConflict(volume v1.Volume, pod *v1.Pod) bool {
151-
// fast path if there is no conflict checking targets.
152-
if volume.GCEPersistentDisk == nil && volume.AWSElasticBlockStore == nil && volume.RBD == nil && volume.ISCSI == nil {
153-
return false
154-
}
155-
156-
for _, existingVolume := range pod.Spec.Volumes {
157-
// Same GCE disk mounted by multiple pods conflicts unless all pods mount it read-only.
158-
if volume.GCEPersistentDisk != nil && existingVolume.GCEPersistentDisk != nil {
159-
disk, existingDisk := volume.GCEPersistentDisk, existingVolume.GCEPersistentDisk
160-
if disk.PDName == existingDisk.PDName && !(disk.ReadOnly && existingDisk.ReadOnly) {
161-
return true
162-
}
163-
}
164-
165-
if volume.AWSElasticBlockStore != nil && existingVolume.AWSElasticBlockStore != nil {
166-
if volume.AWSElasticBlockStore.VolumeID == existingVolume.AWSElasticBlockStore.VolumeID {
167-
return true
168-
}
169-
}
170-
171-
if volume.ISCSI != nil && existingVolume.ISCSI != nil {
172-
iqn := volume.ISCSI.IQN
173-
eiqn := existingVolume.ISCSI.IQN
174-
// two ISCSI volumes are same, if they share the same iqn. As iscsi volumes are of type
175-
// RWO or ROX, we could permit only one RW mount. Same iscsi volume mounted by multiple Pods
176-
// conflict unless all other pods mount as read only.
177-
if iqn == eiqn && !(volume.ISCSI.ReadOnly && existingVolume.ISCSI.ReadOnly) {
178-
return true
179-
}
180-
}
181-
182-
if volume.RBD != nil && existingVolume.RBD != nil {
183-
mon, pool, image := volume.RBD.CephMonitors, volume.RBD.RBDPool, volume.RBD.RBDImage
184-
emon, epool, eimage := existingVolume.RBD.CephMonitors, existingVolume.RBD.RBDPool, existingVolume.RBD.RBDImage
185-
// two RBDs images are the same if they share the same Ceph monitor, are in the same RADOS Pool, and have the same image name
186-
// only one read-write mount is permitted for the same RBD image.
187-
// same RBD image mounted by multiple Pods conflicts unless all Pods mount the image read-only
188-
if haveOverlap(mon, emon) && pool == epool && image == eimage && !(volume.RBD.ReadOnly && existingVolume.RBD.ReadOnly) {
189-
return true
190-
}
191-
}
192-
}
193-
194-
return false
195-
}
196-
197-
// NoDiskConflict evaluates if a pod can fit due to the volumes it requests, and those that
198-
// are already mounted. If there is already a volume mounted on that node, another pod that uses the same volume
199-
// can't be scheduled there.
200-
// This is GCE, Amazon EBS, ISCSI and Ceph RBD specific for now:
201-
// - GCE PD allows multiple mounts as long as they're all read-only
202-
// - AWS EBS forbids any two pods mounting the same volume ID
203-
// - Ceph RBD forbids if any two pods share at least same monitor, and match pool and image, and the image is read-only
204-
// - ISCSI forbids if any two pods share at least same IQN and ISCSI volume is read-only
205-
// TODO: migrate this into some per-volume specific code?
206-
func NoDiskConflict(pod *v1.Pod, meta Metadata, nodeInfo *schedulernodeinfo.NodeInfo) (bool, []PredicateFailureReason, error) {
207-
for _, v := range pod.Spec.Volumes {
208-
for _, ev := range nodeInfo.Pods() {
209-
if isVolumeConflict(v, ev) {
210-
return false, []PredicateFailureReason{ErrDiskConflict}, nil
211-
}
212-
}
213-
}
214-
return true, nil, nil
215-
}
216-
217150
// MaxPDVolumeCountChecker contains information to check the max number of volumes for a predicate.
218151
type MaxPDVolumeCountChecker struct {
219152
filter VolumeFilter
@@ -963,25 +896,6 @@ func PodFitsHostPortsPredicate(pod *v1.Pod, meta []*v1.ContainerPort, nodeInfo *
963896
return true, nil, nil
964897
}
965898

966-
// haveOverlap searches two arrays and returns true if they have at least one common element; returns false otherwise.
967-
func haveOverlap(a1, a2 []string) bool {
968-
if len(a1) > len(a2) {
969-
a1, a2 = a2, a1
970-
}
971-
m := map[string]bool{}
972-
973-
for _, val := range a1 {
974-
m[val] = true
975-
}
976-
for _, val := range a2 {
977-
if _, ok := m[val]; ok {
978-
return true
979-
}
980-
}
981-
982-
return false
983-
}
984-
985899
// GeneralPredicates checks a group of predicates that the kubelet cares about.
986900
// DEPRECATED: this exist only because kubelet uses it. We should change kubelet to execute the individual predicates it requires.
987901
func GeneralPredicates(pod *v1.Pod, meta Metadata, nodeInfo *schedulernodeinfo.NodeInfo) (bool, []PredicateFailureReason, error) {

‎pkg/scheduler/algorithm/predicates/predicates_test.go‎

Lines changed: 0 additions & 232 deletions
Original file line numberDiff line numberDiff line change
@@ -721,238 +721,6 @@ func TestPodFitsHostPorts(t *testing.T) {
721721
}
722722
}
723723

724-
func TestGCEDiskConflicts(t *testing.T) {
725-
volState := v1.PodSpec{
726-
Volumes: []v1.Volume{
727-
{
728-
VolumeSource: v1.VolumeSource{
729-
GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{
730-
PDName: "foo",
731-
},
732-
},
733-
},
734-
},
735-
}
736-
volState2 := v1.PodSpec{
737-
Volumes: []v1.Volume{
738-
{
739-
VolumeSource: v1.VolumeSource{
740-
GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{
741-
PDName: "bar",
742-
},
743-
},
744-
},
745-
},
746-
}
747-
tests := []struct {
748-
pod *v1.Pod
749-
nodeInfo *schedulernodeinfo.NodeInfo
750-
isOk bool
751-
name string
752-
}{
753-
{&v1.Pod{}, schedulernodeinfo.NewNodeInfo(), true, "nothing"},
754-
{&v1.Pod{}, schedulernodeinfo.NewNodeInfo(&v1.Pod{Spec: volState}), true, "one state"},
755-
{&v1.Pod{Spec: volState}, schedulernodeinfo.NewNodeInfo(&v1.Pod{Spec: volState}), false, "same state"},
756-
{&v1.Pod{Spec: volState2}, schedulernodeinfo.NewNodeInfo(&v1.Pod{Spec: volState}), true, "different state"},
757-
}
758-
expectedFailureReasons := []PredicateFailureReason{ErrDiskConflict}
759-
760-
for _, test := range tests {
761-
t.Run(test.name, func(t *testing.T) {
762-
ok, reasons, err := NoDiskConflict(test.pod, nil, test.nodeInfo)
763-
if err != nil {
764-
t.Errorf("unexpected error: %v", err)
765-
}
766-
if !ok && !reflect.DeepEqual(reasons, expectedFailureReasons) {
767-
t.Errorf("unexpected failure reasons: %v, want: %v", reasons, expectedFailureReasons)
768-
}
769-
if test.isOk && !ok {
770-
t.Errorf("expected ok, got none. %v %s", test.pod, test.nodeInfo)
771-
}
772-
if !test.isOk && ok {
773-
t.Errorf("expected no ok, got one. %v %s", test.pod, test.nodeInfo)
774-
}
775-
})
776-
}
777-
}
778-
779-
func TestAWSDiskConflicts(t *testing.T) {
780-
volState := v1.PodSpec{
781-
Volumes: []v1.Volume{
782-
{
783-
VolumeSource: v1.VolumeSource{
784-
AWSElasticBlockStore: &v1.AWSElasticBlockStoreVolumeSource{
785-
VolumeID: "foo",
786-
},
787-
},
788-
},
789-
},
790-
}
791-
volState2 := v1.PodSpec{
792-
Volumes: []v1.Volume{
793-
{
794-
VolumeSource: v1.VolumeSource{
795-
AWSElasticBlockStore: &v1.AWSElasticBlockStoreVolumeSource{
796-
VolumeID: "bar",
797-
},
798-
},
799-
},
800-
},
801-
}
802-
tests := []struct {
803-
pod *v1.Pod
804-
nodeInfo *schedulernodeinfo.NodeInfo
805-
isOk bool
806-
name string
807-
}{
808-
{&v1.Pod{}, schedulernodeinfo.NewNodeInfo(), true, "nothing"},
809-
{&v1.Pod{}, schedulernodeinfo.NewNodeInfo(&v1.Pod{Spec: volState}), true, "one state"},
810-
{&v1.Pod{Spec: volState}, schedulernodeinfo.NewNodeInfo(&v1.Pod{Spec: volState}), false, "same state"},
811-
{&v1.Pod{Spec: volState2}, schedulernodeinfo.NewNodeInfo(&v1.Pod{Spec: volState}), true, "different state"},
812-
}
813-
expectedFailureReasons := []PredicateFailureReason{ErrDiskConflict}
814-
815-
for _, test := range tests {
816-
t.Run(test.name, func(t *testing.T) {
817-
ok, reasons, err := NoDiskConflict(test.pod, nil, test.nodeInfo)
818-
if err != nil {
819-
t.Errorf("unexpected error: %v", err)
820-
}
821-
if !ok && !reflect.DeepEqual(reasons, expectedFailureReasons) {
822-
t.Errorf("unexpected failure reasons: %v, want: %v", reasons, expectedFailureReasons)
823-
}
824-
if test.isOk && !ok {
825-
t.Errorf("expected ok, got none. %v %s", test.pod, test.nodeInfo)
826-
}
827-
if !test.isOk && ok {
828-
t.Errorf("expected no ok, got one. %v %s", test.pod, test.nodeInfo)
829-
}
830-
})
831-
}
832-
}
833-
834-
func TestRBDDiskConflicts(t *testing.T) {
835-
volState := v1.PodSpec{
836-
Volumes: []v1.Volume{
837-
{
838-
VolumeSource: v1.VolumeSource{
839-
RBD: &v1.RBDVolumeSource{
840-
CephMonitors: []string{"a", "b"},
841-
RBDPool: "foo",
842-
RBDImage: "bar",
843-
FSType: "ext4",
844-
},
845-
},
846-
},
847-
},
848-
}
849-
volState2 := v1.PodSpec{
850-
Volumes: []v1.Volume{
851-
{
852-
VolumeSource: v1.VolumeSource{
853-
RBD: &v1.RBDVolumeSource{
854-
CephMonitors: []string{"c", "d"},
855-
RBDPool: "foo",
856-
RBDImage: "bar",
857-
FSType: "ext4",
858-
},
859-
},
860-
},
861-
},
862-
}
863-
tests := []struct {
864-
pod *v1.Pod
865-
nodeInfo *schedulernodeinfo.NodeInfo
866-
isOk bool
867-
name string
868-
}{
869-
{&v1.Pod{}, schedulernodeinfo.NewNodeInfo(), true, "nothing"},
870-
{&v1.Pod{}, schedulernodeinfo.NewNodeInfo(&v1.Pod{Spec: volState}), true, "one state"},
871-
{&v1.Pod{Spec: volState}, schedulernodeinfo.NewNodeInfo(&v1.Pod{Spec: volState}), false, "same state"},
872-
{&v1.Pod{Spec: volState2}, schedulernodeinfo.NewNodeInfo(&v1.Pod{Spec: volState}), true, "different state"},
873-
}
874-
expectedFailureReasons := []PredicateFailureReason{ErrDiskConflict}
875-
876-
for _, test := range tests {
877-
t.Run(test.name, func(t *testing.T) {
878-
ok, reasons, err := NoDiskConflict(test.pod, nil, test.nodeInfo)
879-
if err != nil {
880-
t.Errorf("unexpected error: %v", err)
881-
}
882-
if !ok && !reflect.DeepEqual(reasons, expectedFailureReasons) {
883-
t.Errorf("unexpected failure reasons: %v, want: %v", reasons, expectedFailureReasons)
884-
}
885-
if test.isOk && !ok {
886-
t.Errorf("expected ok, got none. %v %s", test.pod, test.nodeInfo)
887-
}
888-
if !test.isOk && ok {
889-
t.Errorf("expected no ok, got one. %v %s", test.pod, test.nodeInfo)
890-
}
891-
})
892-
}
893-
}
894-
895-
func TestISCSIDiskConflicts(t *testing.T) {
896-
volState := v1.PodSpec{
897-
Volumes: []v1.Volume{
898-
{
899-
VolumeSource: v1.VolumeSource{
900-
ISCSI: &v1.ISCSIVolumeSource{
901-
TargetPortal: "127.0.0.1:3260",
902-
IQN: "iqn.2016-12.server:storage.target01",
903-
FSType: "ext4",
904-
Lun: 0,
905-
},
906-
},
907-
},
908-
},
909-
}
910-
volState2 := v1.PodSpec{
911-
Volumes: []v1.Volume{
912-
{
913-
VolumeSource: v1.VolumeSource{
914-
ISCSI: &v1.ISCSIVolumeSource{
915-
TargetPortal: "127.0.0.1:3260",
916-
IQN: "iqn.2017-12.server:storage.target01",
917-
FSType: "ext4",
918-
Lun: 0,
919-
},
920-
},
921-
},
922-
},
923-
}
924-
tests := []struct {
925-
pod *v1.Pod
926-
nodeInfo *schedulernodeinfo.NodeInfo
927-
isOk bool
928-
name string
929-
}{
930-
{&v1.Pod{}, schedulernodeinfo.NewNodeInfo(), true, "nothing"},
931-
{&v1.Pod{}, schedulernodeinfo.NewNodeInfo(&v1.Pod{Spec: volState}), true, "one state"},
932-
{&v1.Pod{Spec: volState}, schedulernodeinfo.NewNodeInfo(&v1.Pod{Spec: volState}), false, "same state"},
933-
{&v1.Pod{Spec: volState2}, schedulernodeinfo.NewNodeInfo(&v1.Pod{Spec: volState}), true, "different state"},
934-
}
935-
expectedFailureReasons := []PredicateFailureReason{ErrDiskConflict}
936-
937-
for _, test := range tests {
938-
t.Run(test.name, func(t *testing.T) {
939-
ok, reasons, err := NoDiskConflict(test.pod, nil, test.nodeInfo)
940-
if err != nil {
941-
t.Errorf("unexpected error: %v", err)
942-
}
943-
if !ok && !reflect.DeepEqual(reasons, expectedFailureReasons) {
944-
t.Errorf("unexpected failure reasons: %v, want: %v", reasons, expectedFailureReasons)
945-
}
946-
if test.isOk && !ok {
947-
t.Errorf("expected ok, got none. %v %s", test.pod, test.nodeInfo)
948-
}
949-
if !test.isOk && ok {
950-
t.Errorf("expected no ok, got one. %v %s", test.pod, test.nodeInfo)
951-
}
952-
})
953-
}
954-
}
955-
956724
// TODO: Add test case for RequiredDuringSchedulingRequiredDuringExecution after it's implemented.
957725
func TestPodFitsSelector(t *testing.T) {
958726
tests := []struct {

‎pkg/scheduler/algorithmprovider/defaults/register_predicates.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,12 @@ func init() {
117117
)
118118

119119
// Fit is determined by non-conflicting disk volumes.
120-
scheduler.RegisterFitPredicate(predicates.NoDiskConflictPred, predicates.NoDiskConflict)
120+
scheduler.RegisterFitPredicateFactory(
121+
predicates.NoDiskConflictPred,
122+
func(args scheduler.AlgorithmFactoryArgs) predicates.FitPredicate {
123+
return nil
124+
},
125+
)
121126

122127
// GeneralPredicates are the predicates that are enforced by all Kubernetes components
123128
// (e.g. kubelet and all schedulers)

‎pkg/scheduler/framework/plugins/volumerestrictions/BUILD‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ go_library(
77
visibility = ["//visibility:public"],
88
deps = [
99
"//pkg/scheduler/algorithm/predicates:go_default_library",
10-
"//pkg/scheduler/framework/plugins/migration:go_default_library",
1110
"//pkg/scheduler/framework/v1alpha1:go_default_library",
1211
"//pkg/scheduler/nodeinfo:go_default_library",
1312
"//staging/src/k8s.io/api/core/v1:go_default_library",

0 commit comments

Comments
 (0)