Skip to content

Commit

Permalink
feat(setting): add tolerate node disk empty selector volume setting
Browse files Browse the repository at this point in the history 
ref: longhorn/longhorn 4826

Signed-off-by: Jack Lin <jack.lin@suse.com>
  • Loading branch information
@ChanYiLin
ChanYiLin committed Jul 17, 2023
1 parent a5b67e5 commit e413988
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 26 deletions.
7 changes: 6 additions & 1 deletion controller/volume_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2304,10 +2304,15 @@ func (c *VolumeController) listReadySchedulableAndScheduledNodes(volume *longhor
return nil, err
}

tolerateEmptyNodeSelectorVolume, err := c.ds.GetSettingAsBool(types.SettingNameTolerateEmptyNodeSelectorVolume)
if err != nil {
logrus.Errorf("Failed to get tolerate empty node selector volume setting: %v", err)
}

filteredReadyNodes := readyNodes
if len(volume.Spec.NodeSelector) != 0 {
for nodeName, node := range readyNodes {
if !types.IsSelectorsInTags(node.Spec.Tags, volume.Spec.NodeSelector) {
if !types.IsSelectorsInTags(node.Spec.Tags, volume.Spec.NodeSelector, tolerateEmptyNodeSelectorVolume) {
delete(filteredReadyNodes, nodeName)
}
}
Expand Down
25 changes: 20 additions & 5 deletions scheduler/replica_scheduler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func (rcs *ReplicaScheduler) getDiskCandidates(nodeInfo map[string]*longhorn.Nod

nodeSoftAntiAffinity, err := rcs.ds.GetSettingAsBool(types.SettingNameReplicaSoftAntiAffinity)
if err != nil {
logrus.Errorf("error getting replica soft anti-affinity setting: %v", err)
logrus.Errorf("Failed to get replica soft anti-affinity setting: %v", err)
}

if volume.Spec.ReplicaSoftAntiAffinity != longhorn.ReplicaSoftAntiAffinityDefault &&
Expand All @@ -157,7 +157,7 @@ func (rcs *ReplicaScheduler) getDiskCandidates(nodeInfo map[string]*longhorn.Nod

zoneSoftAntiAffinity, err := rcs.ds.GetSettingAsBool(types.SettingNameReplicaZoneSoftAntiAffinity)
if err != nil {
logrus.Errorf("Error getting replica zone soft anti-affinity setting: %v", err)
logrus.Errorf("Failed to get replica zone soft anti-affinity setting: %v", err)
}
if volume.Spec.ReplicaZoneSoftAntiAffinity != longhorn.ReplicaZoneSoftAntiAffinityDefault &&
volume.Spec.ReplicaZoneSoftAntiAffinity != "" {
Expand Down Expand Up @@ -202,14 +202,19 @@ func (rcs *ReplicaScheduler) getDiskCandidates(nodeInfo map[string]*longhorn.Nod
return result
}

tolerateEmptyNodeSelectorVolume, err := rcs.ds.GetSettingAsBool(types.SettingNameTolerateEmptyNodeSelectorVolume)
if err != nil {
logrus.Errorf("Failed to get tolerate empty node selector volume setting: %v", err)
}

unusedNodes := map[string]*longhorn.Node{}
unusedNodesInNewZones := map[string]*longhorn.Node{}
nodesInUnusedZones := map[string]*longhorn.Node{}
nodesWithEvictingReplicas := getNodesWithEvictingReplicas(replicas, nodeInfo)

for nodeName, node := range nodeInfo {
// Filter Nodes. If the Nodes don't match the tags, don't bother marking them as candidates.
if !types.IsSelectorsInTags(node.Spec.Tags, volume.Spec.NodeSelector) {
if !types.IsSelectorsInTags(node.Spec.Tags, volume.Spec.NodeSelector, tolerateEmptyNodeSelectorVolume) {
continue
}
if _, ok := usedNodes[nodeName]; !ok {
Expand Down Expand Up @@ -291,6 +296,11 @@ func (rcs *ReplicaScheduler) filterNodeDisksForReplica(node *longhorn.Node, disk
multiError = util.NewMultiError()
preferredDisks = map[string]*Disk{}

tolerateEmptyDiskSelectorVolume, err := rcs.ds.GetSettingAsBool(types.SettingNameTolerateEmptyDiskSelectorVolume)
if err != nil {
logrus.Errorf("Failed to get tolerate empty tag volume setting: %v", err)
}

if len(disks) == 0 {
multiError.Append(util.NewMultiError(longhorn.ErrorReplicaScheduleDiskUnavailable))
return preferredDisks, multiError
Expand Down Expand Up @@ -349,7 +359,7 @@ func (rcs *ReplicaScheduler) filterNodeDisksForReplica(node *longhorn.Node, disk
}

// Check if the Disk's Tags are valid.
if !types.IsSelectorsInTags(diskSpec.Tags, volume.Spec.DiskSelector) {
if !types.IsSelectorsInTags(diskSpec.Tags, volume.Spec.DiskSelector, tolerateEmptyDiskSelectorVolume) {
multiError.Append(util.NewMultiError(longhorn.ErrorReplicaScheduleTagsNotFulfilled))
continue
}
Expand Down Expand Up @@ -559,6 +569,11 @@ func (rcs *ReplicaScheduler) isFailedReplicaReusable(r *longhorn.Replica, v *lon
return false
}

tolerateEmptyDiskSelectorVolume, err := rcs.ds.GetSettingAsBool(types.SettingNameTolerateEmptyDiskSelectorVolume)
if err != nil {
logrus.Errorf("Failed to get tolerate empty tag volume setting: %v", err)
}

node, exists := nodeInfo[r.Spec.NodeID]
if !exists {
return false
Expand Down Expand Up @@ -595,7 +610,7 @@ func (rcs *ReplicaScheduler) isFailedReplicaReusable(r *longhorn.Replica, v *lon
if !diskSpec.AllowScheduling || diskSpec.EvictionRequested {
return false
}
if !types.IsSelectorsInTags(diskSpec.Tags, v.Spec.DiskSelector) {
if !types.IsSelectorsInTags(diskSpec.Tags, v.Spec.DiskSelector, tolerateEmptyDiskSelectorVolume) {
return false
}
}
Expand Down
30 changes: 30 additions & 0 deletions types/setting.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ const (
SettingNameV2DataEngine = SettingName("v2-data-engine")
SettingNameV2DataEngineHugepageLimit = SettingName("v2-data-engine-hugepage-limit")
SettingNameOfflineReplicaRebuilding = SettingName("offline-replica-rebuilding")
SettingNameTolerateEmptyNodeSelectorVolume = SettingName("tolerate-empty-node-selector-volume")
SettingNameTolerateEmptyDiskSelectorVolume = SettingName("tolerate-empty-disk-selector-volume")
)

var (
Expand Down Expand Up @@ -179,6 +181,8 @@ var (
SettingNameV2DataEngine,
SettingNameV2DataEngineHugepageLimit,
SettingNameOfflineReplicaRebuilding,
SettingNameTolerateEmptyNodeSelectorVolume,
SettingNameTolerateEmptyDiskSelectorVolume,
}
)

Expand Down Expand Up @@ -277,6 +281,8 @@ var (
SettingNameV2DataEngine: SettingDefinitionV2DataEngine,
SettingNameV2DataEngineHugepageLimit: SettingDefinitionV2DataEngineHugepageLimit,
SettingNameOfflineReplicaRebuilding: SettingDefinitionOfflineReplicaRebuilding,
SettingNameTolerateEmptyNodeSelectorVolume: SettingDefinitionTolerateEmptyNodeSelectorVolume,
SettingNameTolerateEmptyDiskSelectorVolume: SettingDefinitionTolerateEmptyDiskSelectorVolume,
}

SettingDefinitionBackupTarget = SettingDefinition{
Expand Down Expand Up @@ -1103,6 +1109,26 @@ var (
ReadOnly: true,
Default: "1024",
}

SettingDefinitionTolerateEmptyNodeSelectorVolume = SettingDefinition{
DisplayName: "Allow Scheduling Empty Node Selector Volumes To Any Node",
Description: "Allow replica of the volume without node selector to be scheduled on node with tags, default true",
Category: SettingCategoryScheduling,
Type: SettingTypeBool,
Required: true,
ReadOnly: false,
Default: "true",
}

SettingDefinitionTolerateEmptyDiskSelectorVolume = SettingDefinition{
DisplayName: "Allow Scheduling Empty Disk Selector Volumes To Any Disk",
Description: "Allow replica of the volume without disk selector to be scheduled on disk with tags, default true",
Category: SettingCategoryScheduling,
Type: SettingTypeBool,
Required: true,
ReadOnly: false,
Default: "true",
}
)

type NodeDownPodDeletionPolicy string
Expand Down Expand Up @@ -1198,6 +1224,10 @@ func ValidateSetting(name, value string) (err error) {
fallthrough
case SettingNameV2DataEngine:
fallthrough
case SettingNameTolerateEmptyNodeSelectorVolume:
fallthrough
case SettingNameTolerateEmptyDiskSelectorVolume:
fallthrough
case SettingNameAllowCollectingLonghornUsage:
if value != "true" && value != "false" {
return fmt.Errorf("value %v of setting %v should be true or false", value, sName)
Expand Down
8 changes: 7 additions & 1 deletion types/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -995,12 +995,18 @@ func GetLHVolumeAttachmentNameFromVolumeName(volName string) string {

// IsSelectorsInTags checks if all the selectors are present in the tags slice.
// It returns true if all selectors are found, false otherwise.
func IsSelectorsInTags(tags, selectors []string) bool {
func IsSelectorsInTags(tags, selectors []string, tolerateEmptySelector bool) bool {
if !sort.StringsAreSorted(tags) {
logrus.Debug("BUG: Tags are not sorted, sorting now")
sort.Strings(tags)
}

if len(selectors) == 0 {
if !tolerateEmptySelector && len(tags) != 0 {
return false
}
}

for _, selector := range selectors {
index := sort.SearchStrings(tags, selector)
// If the selector is not found or the index is out of bounds, return false.
Expand Down
50 changes: 31 additions & 19 deletions types/types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,43 +123,55 @@ func (s *TestSuite) TestParseToleration(c *C) {

func (s *TestSuite) TestIsSelectorsInTags(c *C) {
type testCase struct {
inputTags []string
inputSelectors []string
inputTags []string
inputSelectors []string
tolerateEmptySelector bool

expected bool
}
testCases := map[string]testCase{
"selectors exist": {
inputTags: []string{"aaa", "bbb", "ccc"},
inputSelectors: []string{"aaa", "bbb", "ccc"},
expected: true,
inputTags: []string{"aaa", "bbb", "ccc"},
inputSelectors: []string{"aaa", "bbb", "ccc"},
tolerateEmptySelector: true,
expected: true,
},
"selectors mis-matched": {
inputTags: []string{"aaa", "bbb", "ccc"},
inputSelectors: []string{"aaa", "b", "ccc"},
expected: false,
inputTags: []string{"aaa", "bbb", "ccc"},
inputSelectors: []string{"aaa", "b", "ccc"},
tolerateEmptySelector: true,
expected: false,
},
"selectors empty": {
inputTags: []string{"aaa", "bbb", "ccc"},
inputSelectors: []string{},
expected: true,
"selectors empty and tolerate": {
inputTags: []string{"aaa", "bbb", "ccc"},
inputSelectors: []string{},
tolerateEmptySelector: true,
expected: true,
},
"selectors empty and not tolerate": {
inputTags: []string{"aaa", "bbb", "ccc"},
inputSelectors: []string{},
tolerateEmptySelector: false,
expected: false,
},
"tags unsorted": {
inputTags: []string{"bbb", "aaa", "ccc"},
inputSelectors: []string{"aaa", "bbb", "ccc"},
expected: true,
inputTags: []string{"bbb", "aaa", "ccc"},
inputSelectors: []string{"aaa", "bbb", "ccc"},
tolerateEmptySelector: true,
expected: true,
},
"tags empty": {
inputTags: []string{},
inputSelectors: []string{"aaa", "bbb", "ccc"},
expected: false,
inputTags: []string{},
inputSelectors: []string{"aaa", "bbb", "ccc"},
tolerateEmptySelector: true,
expected: false,
},
}

for testName, testCase := range testCases {
fmt.Printf("testing %v\n", testName)

actual := IsSelectorsInTags(testCase.inputTags, testCase.inputSelectors)
actual := IsSelectorsInTags(testCase.inputTags, testCase.inputSelectors, testCase.tolerateEmptySelector)
c.Assert(actual, Equals, testCase.expected, Commentf(TestErrResultFmt, testName))
}
}

0 comments on commit e413988

Please sign in to comment.