feat(cspc): add capability to specify allowed BD tags on CSPC (#172)

This commit adds the capability to specify allowed BD tags on CSPC via
annotations. The annotation key is `cstor.openebs.io/allowed-bd-tags`
and the allowed value is comma separated string.
For example:  "fast, slow" is a possible value and in this case CSPC provisioner
will reject BDs that have a BD tag (`openebs.io/block-device-tag`) present on
it and the value is other than fast or slow.

Signed-off-by: Ashutosh Kumar <[email protected]>

go.mod

@@ -9,7 +9,7 @@ require (
 	github.com/imdario/mergo v0.3.8 // indirect
 	github.com/onsi/ginkgo v1.12.0
 	github.com/onsi/gomega v1.9.0
-	github.com/openebs/api v1.12.1-0.20200813113856-a86aa610f932
+	github.com/openebs/api v1.12.1-0.20200908020958-c9b104663c7f
 	github.com/pkg/errors v0.9.1
 	github.com/spf13/cobra v0.0.5
 	github.com/spf13/pflag v1.0.5

go.sum

@@ -391,6 +391,8 @@ github.com/openebs/api v1.12.1-0.20200805040335-b9bd4809e80c h1:fZmO/A7BnxFAg9y1
 github.com/openebs/api v1.12.1-0.20200805040335-b9bd4809e80c/go.mod h1:TASujm6H1LGdx43MN7Dab1xdAqR7MVU8bsS74Ywop5w=
 github.com/openebs/api v1.12.1-0.20200813113856-a86aa610f932 h1:YzV1nspFTKrCa7c2XeWPcrU9KRvV9Ul9Yu9SLHqldas=
 github.com/openebs/api v1.12.1-0.20200813113856-a86aa610f932/go.mod h1:TASujm6H1LGdx43MN7Dab1xdAqR7MVU8bsS74Ywop5w=
+github.com/openebs/api v1.12.1-0.20200908020958-c9b104663c7f h1:ebsHrWNo+Ypp5C9eKMC8lX9z9OrZxCUkWAzN2otwfek=
+github.com/openebs/api v1.12.1-0.20200908020958-c9b104663c7f/go.mod h1:TASujm6H1LGdx43MN7Dab1xdAqR7MVU8bsS74Ywop5w=
 github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k=
 github.com/pelletier/go-toml v1.1.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
 github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=

pkg/controllers/cspc-controller/util/cspc_util_test.go

@@ -5,7 +5,7 @@ you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
 
 	http://www.apache.org/licenses/LICENSE-2.0
-	
+
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

pkg/controllers/cstorvolumeconfig/volume_operations_test.go

@@ -5,7 +5,7 @@ you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
 
 	http://www.apache.org/licenses/LICENSE-2.0
-	
+
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

pkg/cspc/algorithm/select_node.go

@@ -18,6 +18,7 @@ package algorithm
 
 import (
 	"fmt"
+	"strings"
 
 	cstor "github.com/openebs/api/pkg/apis/cstor/v1"
 	openebsio "github.com/openebs/api/pkg/apis/openebs.io/v1alpha1"
@@ -162,6 +163,29 @@ func (ac *Config) ClaimBD(bdObj openebsio.BlockDevice) error {
 		return errors.Errorf("failed to get capacity from block device %s:%s", bdObj.Name, err)
 	}
 
+	// If the BD has a BD tag present then we need to decide whether
+	// cStor can use it or not.
+	// If there is no BD tag present on BD then still the BD is safe to use.
+	value, ok := bdObj.Labels[types.BlockDeviceTagLabelKey]
+	var allowedBDTags map[string]bool
+	if ok {
+		// If the BD tag value is empty -- cStor cannot use it.
+		if strings.TrimSpace(value) == "" {
+			return errors.Errorf("failed to create block device "+
+				"claim for bd {%s} as it has empty value for bd tag", bdObj.Name)
+		}
+
+		// If the BD tag in the BD is present in allowed annotations on CSPC then
+		// it means that this BD can be considered in provisioning else it should not
+		// be considered
+		allowedBDTags = getAllowedTagMap(ac.CSPC.GetAnnotations())
+		if !allowedBDTags[strings.TrimSpace(value)] {
+			return errors.Errorf("cannot use bd {%s} as it has tag %s but "+
+				"cspc has allowed bd tags as %s",
+				bdObj.Name, value, ac.CSPC.GetAnnotations()[types.OpenEBSAllowedBDTagKey])
+		}
+	}
+
 	newBDCObj := openebsio.NewBlockDeviceClaim().
 		WithName("bdc-cstor-" + string(bdObj.UID)).
 		WithNamespace(ac.Namespace).
@@ -172,6 +196,15 @@ func (ac *Config) ClaimBD(bdObj openebsio.BlockDevice) error {
 		WithCapacity(resourceList).
 		WithFinalizer(types.CSPCFinalizer)
 
+	// ToDo: Move this to openebs/api builder
+	// Create label selector to fill in BDC spec.
+	if ok {
+		ls := &metav1.LabelSelector{
+			MatchLabels: map[string]string{types.BlockDeviceTagLabelKey: value},
+		}
+		newBDCObj.Spec.Selector = ls
+	}
+
 	_, err = ac.clientset.OpenebsV1alpha1().BlockDeviceClaims(ac.Namespace).Create(newBDCObj)
 	if k8serror.IsAlreadyExists(err) {
 		klog.Infof("BDC for BD {%s} already created", bdObj.Name)
@@ -183,6 +216,29 @@ func (ac *Config) ClaimBD(bdObj openebsio.BlockDevice) error {
 	return nil
 }
 
+// getAllowedTagMap returns a map of the allowed BD tags
+// Example :
+// If the CSPC annotation is passed and following is the BD tag annotation
+//
+// cstor.openebs.io/allowed-bd-tags:fast,slow
+//
+// Then, a map {"fast":true,"slow":true} is returned.
+func getAllowedTagMap(cspcAnnotation map[string]string) map[string]bool {
+	allowedTagsMap := make(map[string]bool)
+	allowedTags := cspcAnnotation[types.OpenEBSAllowedBDTagKey]
+	if strings.TrimSpace(allowedTags) == "" {
+		return allowedTagsMap
+	}
+	allowedTagsList := strings.Split(allowedTags, ",")
+	for _, v := range allowedTagsList {
+		if strings.TrimSpace(v) == "" {
+			continue
+		}
+		allowedTagsMap[v] = true
+	}
+	return allowedTagsMap
+}
+
 // IsClaimedBDUsable returns true if the passed BD is already claimed and can be
 // used for provisioning
 func (ac *Config) IsClaimedBDUsable(bd openebsio.BlockDevice) (bool, error) {

pkg/cspc/algorithm/select_node_test.go

@@ -26,7 +26,7 @@ import (
 	"github.com/openebs/api/pkg/apis/types"
 	openebsFakeClientset "github.com/openebs/api/pkg/client/clientset/versioned/fake"
 	corev1 "k8s.io/api/core/v1"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/fake"
 	core "k8s.io/client-go/testing"
@@ -374,3 +374,85 @@ func TestConfig_ClaimBD(t *testing.T) {
 		})
 	}
 }
+
+func Test_getAllowedTagMap(t *testing.T) {
+	type args struct {
+		cspcAnnotation map[string]string
+	}
+	tests := []struct {
+		name string
+		args args
+		want map[string]bool
+	}{
+		{
+			name: "Test case #1",
+			args: args{
+				cspcAnnotation: map[string]string{types.OpenEBSAllowedBDTagKey: "fast,slow"},
+			},
+			want: map[string]bool{"fast": true, "slow": true},
+		},
+
+		{
+			name: "Test case #2",
+			args: args{
+				cspcAnnotation: map[string]string{types.OpenEBSAllowedBDTagKey: "fast,slow"},
+			},
+			want: map[string]bool{"slow": true, "fast": true},
+		},
+
+		{
+			name: "Test case #3 -- Nil Annotations",
+			args: args{
+				cspcAnnotation: nil,
+			},
+			want: map[string]bool{},
+		},
+
+		{
+			name: "Test case #4 -- No BD tag Annotations",
+			args: args{
+				cspcAnnotation: map[string]string{"some-other-annotation-key": "awesome-openebs"},
+			},
+			want: map[string]bool{},
+		},
+
+		{
+			name: "Test case #5 -- Improper format 1",
+			args: args{
+				cspcAnnotation: map[string]string{types.OpenEBSAllowedBDTagKey: ",fast,slow,,"},
+			},
+			want: map[string]bool{"fast": true, "slow": true},
+		},
+
+		{
+			name: "Test case #6 -- Improper format 2",
+			args: args{
+				cspcAnnotation: map[string]string{types.OpenEBSAllowedBDTagKey: ",fast,slow"},
+			},
+			want: map[string]bool{"fast": true, "slow": true},
+		},
+
+		{
+			name: "Test case #7 -- Improper format 2",
+			args: args{
+				cspcAnnotation: map[string]string{types.OpenEBSAllowedBDTagKey: ",fast,,slow"},
+			},
+			want: map[string]bool{"fast": true, "slow": true},
+		},
+
+		{
+			name: "Test case #7 -- Improper format 2",
+			args: args{
+				cspcAnnotation: map[string]string{types.OpenEBSAllowedBDTagKey: "this is improper"},
+			},
+			want: map[string]bool{"this is improper": true},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := getAllowedTagMap(tt.args.cspcAnnotation); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("getAllowedTagMap() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/snapshot/snapshottest/snapshot.go

@@ -5,7 +5,7 @@ you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
 
 	http://www.apache.org/licenses/LICENSE-2.0
-	
+
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

pkg/version/util.go

@@ -5,7 +5,7 @@ you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
 
 	http://www.apache.org/licenses/LICENSE-2.0
-	
+
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

pkg/volume-rpc/targetserver/config.go

@@ -5,7 +5,7 @@ you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
 
 	http://www.apache.org/licenses/LICENSE-2.0
-	
+
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

pkg/webhook/cspc.go

@@ -23,6 +23,7 @@ import (
 	"net/http"
 	"os"
 	"reflect"
+	"strings"
 
 	cstor "github.com/openebs/api/pkg/apis/cstor/v1"
 	openebsapis "github.com/openebs/api/pkg/apis/openebs.io/v1alpha1"
@@ -413,12 +414,17 @@ func validateBlockDevice(bd *openebsapis.BlockDevice, hostName string) error {
 			bd.Labels[types.HostNameLabelKey],
 		)
 	}
+
+	// If the BD tag is present on BD and the value is empty then
+	// this BD is not a valid BD for provisioning.
 	if v, found := bd.Labels[types.BlockDeviceTagLabelKey]; found {
-		return errors.Errorf(
-			"block device %s is tagged with a value %s and cannot be used",
-			bd.Name,
-			v,
-		)
+		if strings.TrimSpace(v) == "" {
+			return errors.Errorf(
+				"block device %s is tagged with a value %s and cannot be used",
+				bd.Name,
+				v,
+			)
+		}
 	}
 
 	return nil

pkg/webhook/cspc_operations.go

@@ -19,6 +19,7 @@ package webhook
 import (
 	"fmt"
 	"reflect"
+	"strings"
 
 	cstor "github.com/openebs/api/pkg/apis/cstor/v1"
 	openebsapis "github.com/openebs/api/pkg/apis/openebs.io/v1alpha1"
@@ -461,7 +462,33 @@ func getBDOwnerReference(cspc *cstor.CStorPoolCluster) []metav1.OwnerReference {
 }
 
 // ClaimBD claims a given BlockDevice
+// ToDo: The BD Claim functionality has code repetition.
+// Need to think about packaging and refactor.
 func (pOps *PoolOperations) ClaimBD(newBdObj *openebsapis.BlockDevice, oldBD string) error {
+
+	// If the BD has a BD tag present then we need to decide whether
+	// cStor can use it or not.
+	// If there is not BD tag present on BD then still the BD is safe to use.
+	value, ok := newBdObj.Labels[types.BlockDeviceTagLabelKey]
+	var allowedBDTags map[string]bool
+	if ok {
+		// If the BD tag value is empty -- cStor cannot use it.
+		if strings.TrimSpace(value) == "" {
+			return errors.Errorf("failed to create block device "+
+				"claim for bd {%s} as it has empty value for bd tag", newBdObj.Name)
+		}
+
+		// If the BD tag in the BD is present in allowed annotations on CSPC then
+		// it means that this BD can be considered in provisioning else it should not
+		// be considered
+		allowedBDTags = getAllowedTagMap(pOps.NewCSPC.GetAnnotations())
+		if !allowedBDTags[strings.TrimSpace(value)] {
+			return errors.Errorf("cannot use bd {%s} as it has tag %s but "+
+				"cspc has allowed bd tags as %s",
+				newBdObj.Name, value, pOps.NewCSPC.GetAnnotations()[types.OpenEBSAllowedBDTagKey])
+		}
+	}
+
 	newBDCObj := openebsapis.NewBlockDeviceClaim().
 		WithName("bdc-cstor-" + string(newBdObj.UID)).
 		WithNamespace(newBdObj.Namespace).
@@ -472,6 +499,14 @@ func (pOps *PoolOperations) ClaimBD(newBdObj *openebsapis.BlockDevice, oldBD str
 		WithCapacity(resource.MustParse(ByteCount(newBdObj.Spec.Capacity.Storage))).
 		WithCSPCOwnerReference(getBDOwnerReference(pOps.OldCSPC)[0]).
 		WithFinalizer(types.CSPCFinalizer)
+	// ToDo: Move this to openebs/api builder
+	// Create label selector to fill in BDC spec.
+	if ok {
+		ls := &metav1.LabelSelector{
+			MatchLabels: map[string]string{types.BlockDeviceTagLabelKey: value},
+		}
+		newBDCObj.Spec.Selector = ls
+	}
 
 	bdcClient := pOps.clientset.OpenebsV1alpha1().BlockDeviceClaims(newBdObj.Namespace)
 	bdcObj, err := bdcClient.Get(newBDCObj.Name, v1.GetOptions{})
@@ -495,6 +530,29 @@ func (pOps *PoolOperations) ClaimBD(newBdObj *openebsapis.BlockDevice, oldBD str
 	return err
 }
 
+// getAllowedTagMap returns a map of the allowed BD tags
+// Example :
+// If the CSPC annotation is passed and following is the BD tag annotation
+//
+// cstor.openebs.io/allowed-bd-tags:fast,slow
+//
+// Then, a map {"fast":true,"slow":true} is returned.
+func getAllowedTagMap(cspcAnnotation map[string]string) map[string]bool {
+	allowedTagsMap := make(map[string]bool)
+	allowedTags := cspcAnnotation[types.OpenEBSAllowedBDTagKey]
+	if strings.TrimSpace(allowedTags) == "" {
+		return allowedTagsMap
+	}
+	allowedTagsList := strings.Split(allowedTags, ",")
+	for _, v := range allowedTagsList {
+		if strings.TrimSpace(v) == "" {
+			continue
+		}
+		allowedTagsMap[v] = true
+	}
+	return allowedTagsMap
+}
+
 // GetNewBDFromRaidGroups returns a map of new successor bd to old bd for replacement in a raid group
 func GetNewBDFromRaidGroups(newRG, oldRG *cstor.RaidGroup) map[string]string {
 	newToOldBlockDeviceMap := make(map[string]string)