Skip to content

Commit

Permalink
feat(controller,filter): add blockdevice tag label while claiming (#400)
Browse files Browse the repository at this point in the history
- add support for new label `openebs.io/block-device-tag`

- add a new filter while claiming block devices. If `openebs.io/block-device-tag` is present on a BlockDevice, only BDCs having a matching selector will be able to claim that block device

- Fix a bug where selector is accidentally updated on BDC while updating other fields.
  BDC controller used a pointer reference of the selector. This
  has been changed to copy the selector and use it. This is done so that
  an update on the BDC object does not cause the selector to get
  updated with a different value other than the one which was applied by
  user.

- Add unit tests for new filter

Sample BDC Yaml which makes use of `openebs.io/block-device-tag` label

```yaml
apiVersion: openebs.io/v1alpha1
kind: BlockDeviceClaim
metadata:
  finalizers:
  - openebs.io/bdc-protection
  name: bdc-ss35
  namespace: default
spec:
  blockDeviceName: blockdevice-91d422d8517f935431daae722f8fdfa0
  resources:
    requests:
      storage: 10M
  selector:
    matchLabels:
      openebs.io/block-device-tag: X
status:
  phase: Bound
```

BlockDevice that was claimed by the above BDC
```yaml
kind: BlockDevice
metadata:
  labels:
    kubernetes.io/hostname: gke-akhil-ndm-pool-1-4349c998-vt36
    ndm.io/blockdevice-type: blockdevice
    ndm.io/managed: "true"
    openebs.io/block-device-tag: X
  name: blockdevice-91d422d8517f935431daae722f8fdfa0
  namespace: default
spec:
  capacity:
    storage: 53687091200
  claimRef:
    apiVersion: openebs.io/v1alpha1
    kind: BlockDeviceClaim
    name: bdc-ss35
    namespace: default
    resourceVersion: "72629912"
    uid: f6c7af75-783e-11ea-9bcf-42010a80015a
  details:
    compliance: SPC-4
    deviceType: disk
    driveType: HDD
    firmwareRevision: '1   '
    hardwareSectorSize: 512
    logicalBlockSize: 512
    model: PersistentDisk
    physicalBlockSize: 4096
    serial: akhil-disk-3
    vendor: Google
  devlinks:
  - kind: by-id
    links:
    - /dev/disk/by-id/scsi-0Google_PersistentDisk_akhil-disk-3
    - /dev/disk/by-id/google-akhil-disk-3
  - kind: by-path
    links:
    - /dev/disk/by-path/pci-0000:00:03.0-scsi-0:0:2:0
  nodeAttributes:
    nodeName: gke-akhil-ndm-pool-1-4349c998-vt36
  path: /dev/sdc
status:
  claimState: Claimed
  state: Active
```

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
  • Loading branch information
akhilerm authored and kmova committed Apr 7, 2020
1 parent 61a60e0 commit 620af53
Show file tree
Hide file tree
Showing 5 changed files with 272 additions and 1 deletion.
5 changes: 5 additions & 0 deletions cmd/ndm_daemonset/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,13 @@ const (
NDMVersion = openEBSLabelPrefix + "v1alpha1"
// reconcileKey is the key used for enable/disable of reconciliation
reconcileKey = "reconcile"
// blockDeviceTagKey is the key used to tag a block device
blockDeviceTagKey = "block-device-tag"
// OpenEBSReconcile is used in annotation to check whether CR is to be reconciled or not
OpenEBSReconcile = openEBSLabelPrefix + reconcileKey
// BlockDeviceTagLabel is the label to tag a blockdevice so that it can be claimed
// only by BDC having a matching label selector
BlockDeviceTagLabel = openEBSLabelPrefix + blockDeviceTagKey
// NDMNotPartitioned is used to say blockdevice does not have any partition.
NDMNotPartitioned = "No"
// NDMPartitioned is used to say blockdevice has some partitions.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ func generateSelector(bdc apis.BlockDeviceClaim) *v1.LabelSelector {

// the hostname label is added into the user given list of labels. If the user hasn't
// given any selector, then the selector object is initialized.
selector := bdc.Spec.Selector
selector := bdc.Spec.Selector.DeepCopy()
if selector == nil {
selector = &v1.LabelSelector{}
}
Expand Down
74 changes: 74 additions & 0 deletions pkg/select/blockdevice/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ import (
apis "github.com/openebs/node-disk-manager/pkg/apis/openebs/v1alpha1"
"github.com/openebs/node-disk-manager/pkg/select/verify"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/selection"
"k8s.io/klog"
)

const (
Expand All @@ -40,6 +43,9 @@ const (
FilterOutSparseBlockDevices = "filterSparseBlockDevice"
// FilterNodeName is used to filter based on nodename
FilterNodeName = "filterNodeName"
// FilterBlockDeviceTag is used to filter out blockdevices having
// openebs.io/blockdevice-tag label
FilterBlockDeviceTag = "filterBlockDeviceTag"
)

// filterFunc is the func type for the filter functions
Expand All @@ -54,6 +60,7 @@ var filterFuncMap = map[string]filterFunc{
FilterResourceStorage: filterResourceStorage,
FilterOutSparseBlockDevices: filterOutSparseBlockDevice,
FilterNodeName: filterNodeName,
FilterBlockDeviceTag: filterBlockDeviceTag,
}

// ApplyFilters apply the filter specified in the filterkeys on the given BD List,
Expand Down Expand Up @@ -241,3 +248,70 @@ func filterNodeName(originalBD *apis.BlockDeviceList, spec *apis.DeviceClaimSpec
}
return filteredBDList
}

// filterBlockDeviceTag is used to filter out BlockDevices which do not have the
// block-device-tag label. This filter works on a block device list which has
// already been filtered by the given selector.
func filterBlockDeviceTag(originalBD *apis.BlockDeviceList, spec *apis.DeviceClaimSpec) *apis.BlockDeviceList {

// if the block-device-tag label was already included in the selector
// given in the BDC by the user, then this filter is not required. This
// is because it would have already performed the filter operation with the
// label. If the label is not present, a new selector is made to remove
// devices which have that label.
if !isBDTagDoesNotExistSelectorRequired(spec.Selector) {
return originalBD
}

// a DoesNotExist requirement is created to filter out devices which have
// the block-device-tag label
blockDeviceTagRequirement, err := labels.NewRequirement(controller.BlockDeviceTagLabel, selection.DoesNotExist, []string{})

// this error should never occur, because error for DoesNotExist happen
// only when non zero length of values are passed
if err != nil {
klog.Info("could not create requirement for label ", controller.BlockDeviceTagLabel)
return originalBD
}

blockDeviceTagDoesNotExistSelector := labels.NewSelector()
blockDeviceTagDoesNotExistSelector =
blockDeviceTagDoesNotExistSelector.Add(*blockDeviceTagRequirement)

filteredBDList := &apis.BlockDeviceList{
TypeMeta: metav1.TypeMeta{
Kind: "BlockDevice",
APIVersion: "openebs.io/v1alpha1",
},
}

for _, bd := range originalBD.Items {
// if the tag label is not present, the BD will be included in the list
if blockDeviceTagDoesNotExistSelector.Matches(labels.Set(bd.Labels)) {
filteredBDList.Items = append(filteredBDList.Items, bd)
}
}
return filteredBDList
}

// isBDTagDoesNotExistSelectorRequired is used to check whether a selector
// was present on the BDC. It is used to decide whether a `does not exist` selector
// for the block-device-tag label should be applied or not.
//
// all the filters are applied after the List call which uses the selector.
// therefore, we don't need to again apply a label selector.
func isBDTagDoesNotExistSelectorRequired(options *metav1.LabelSelector) bool {
if options == nil {
return true
}
if _, ok := options.MatchLabels[controller.BlockDeviceTagLabel]; ok {
return false
}
requirements := options.MatchExpressions
for _, req := range requirements {
if req.Key == controller.BlockDeviceTagLabel {
return false
}
}
return true
}
190 changes: 190 additions & 0 deletions pkg/select/blockdevice/filters_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
package blockdevice

import (
"fmt"
"testing"

controller "github.com/openebs/node-disk-manager/cmd/ndm_daemonset/controller"
apis "github.com/openebs/node-disk-manager/pkg/apis/openebs/v1alpha1"
"github.com/stretchr/testify/assert"
"k8s.io/apimachinery/pkg/apis/meta/v1"
)

const (
TestNoOfBDs = 6
)

type BDLabel map[string]string
type BDLabelList []BDLabel

func TestFilterBlockDeviceTag(t *testing.T) {

// label list with no additional labels
bdLabelList1 := []BDLabel{
map[string]string{
controller.KubernetesHostNameLabel: "host1",
},
map[string]string{
controller.KubernetesHostNameLabel: "host2",
},
map[string]string{
controller.KubernetesHostNameLabel: "host3",
},
map[string]string{
controller.KubernetesHostNameLabel: "host4",
},
map[string]string{
controller.KubernetesHostNameLabel: "host5",
},
map[string]string{
controller.KubernetesHostNameLabel: "host6",
},
}

// label list with all devices having same tag
bdLabelList2 := []BDLabel{
map[string]string{
controller.KubernetesHostNameLabel: "host1",
controller.BlockDeviceTagLabel: "X",
},
map[string]string{
controller.KubernetesHostNameLabel: "host2",
controller.BlockDeviceTagLabel: "X",
},
map[string]string{
controller.KubernetesHostNameLabel: "host3",
controller.BlockDeviceTagLabel: "X",
},
map[string]string{
controller.KubernetesHostNameLabel: "host4",
controller.BlockDeviceTagLabel: "X",
},
map[string]string{
controller.KubernetesHostNameLabel: "host5",
controller.BlockDeviceTagLabel: "X",
},
map[string]string{
controller.KubernetesHostNameLabel: "host6",
controller.BlockDeviceTagLabel: "X",
},
}

// label list with some devices having default label and some devices
// with device tag
bdLabelList3 := []BDLabel{
map[string]string{
controller.KubernetesHostNameLabel: "host1",
},
map[string]string{
controller.KubernetesHostNameLabel: "host2",
},
map[string]string{
controller.KubernetesHostNameLabel: "host3",
controller.BlockDeviceTagLabel: "X",
},
map[string]string{
controller.KubernetesHostNameLabel: "host4",
controller.BlockDeviceTagLabel: "X",
},
map[string]string{
controller.KubernetesHostNameLabel: "host5",
controller.BlockDeviceTagLabel: "Y",
},
map[string]string{
controller.KubernetesHostNameLabel: "host6",
controller.BlockDeviceTagLabel: "Y",
},
}

type args struct {
bdLabelList BDLabelList
spec *apis.DeviceClaimSpec
}
tests := map[string]struct {
args args
wantedNoofBDs int
}{
"no labels on any BD and no selector on BDC": {
args: args{
bdLabelList: bdLabelList1,
spec: &apis.DeviceClaimSpec{},
},
wantedNoofBDs: 6,
},
"all BDs have same device tag label and no selector": {
args: args{
bdLabelList: bdLabelList2,
spec: &apis.DeviceClaimSpec{},
},
wantedNoofBDs: 0,
},
"all BDs have same device tag label and selector for tag": {
args: args{
bdLabelList: bdLabelList2,
spec: &apis.DeviceClaimSpec{
Selector: &v1.LabelSelector{
MatchLabels: map[string]string{controller.BlockDeviceTagLabel: "X"},
},
},
},
wantedNoofBDs: 6,
},
"all BDs have same device tag label and custom label used in selector": {
args: args{
bdLabelList: bdLabelList2,
spec: &apis.DeviceClaimSpec{
Selector: &v1.LabelSelector{
MatchLabels: map[string]string{"ndm.io/test": "test"},
},
},
},
wantedNoofBDs: 0,
},
"some BDs with tag and some without tag, combined with no selector": {
args: args{
bdLabelList: bdLabelList3,
spec: &apis.DeviceClaimSpec{
Selector: &v1.LabelSelector{},
},
},
wantedNoofBDs: 2,
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
bdLabelList := test.args.bdLabelList
spec := test.args.spec
wantedNoOfBDs := test.wantedNoofBDs
originalBDList := createFakeBlockDeviceList(bdLabelList, TestNoOfBDs)
got := filterBlockDeviceTag(originalBDList, spec)
assert.Equal(t, wantedNoOfBDs, len(got.Items))
})
}
}

func createFakeBlockDeviceList(labelList BDLabelList, noOfBDs int) *apis.BlockDeviceList {
bdListAPI := &apis.BlockDeviceList{
TypeMeta: v1.TypeMeta{
Kind: "BlockDevice",
APIVersion: "openebs.io/v1alpha1",
},
Items: []apis.BlockDevice{},
}
for i := 0; i < noOfBDs; i++ {
bdName := fmt.Sprint("bd", i)
bdListAPI.Items = append(bdListAPI.Items, createFakeBlockDevice(bdName, labelList[i]))
}
return bdListAPI
}

func createFakeBlockDevice(name string, label map[string]string) apis.BlockDevice {
bdAPI := apis.BlockDevice{
TypeMeta: v1.TypeMeta{
Kind: "BlockDevice",
APIVersion: "openebs.io/v1alpha1",
},
}
bdAPI.Name = name
bdAPI.Labels = label
return bdAPI
}
2 changes: 2 additions & 0 deletions pkg/select/blockdevice/select.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ func (c *Config) getCandidateDevices(bdList *apis.BlockDeviceList) (*apis.BlockD
// Sparse BDs can be claimed only by manual selection. Therefore, all
// sparse BDs will be filtered out in auto mode
FilterOutSparseBlockDevices,
// remove block devices which do not have the blockdevice tag
FilterBlockDeviceTag,
FilterDeviceType,
FilterVolumeMode,
FilterNodeName,
Expand Down

0 comments on commit 620af53

Please sign in to comment.