feature/CP-4869: Pass 'storageos.com/dataplane.' labels to CP via Upd…

…ateVolume API (#154) * Pass 'storageos.com/dataplane.' labels to CP via UpdateVolume API * Run go fmt ./... Co-authored-by: Alex Reid <[email protected]>
storageos · Sep 28, 2022 · 48660e6 · 48660e6
1 parent 24524e6
commit 48660e6
Show file tree

Hide file tree

Showing 14 changed files with 258 additions and 5 deletions.
diff --git a/controllers/fencer/reconciler.go b/controllers/fencer/reconciler.go
@@ -37,6 +37,7 @@ var (
 )
 
 // NodeFencer provides access to nodes and the volumes running on them.
+//
 //go:generate mockgen -build_flags=--mod=vendor -source=reconciler.go -destination=mocks/mock_node_fencer.go -package=mocks . NodeFencer
 type NodeFencer interface {
 	ListNodes(ctx context.Context) ([]client.Object, error)

diff --git a/controllers/namespace-delete/reconciler.go b/controllers/namespace-delete/reconciler.go
@@ -16,7 +16,8 @@ import (
 	"github.com/storageos/api-manager/internal/pkg/storageos"
 )
 
-//NamespaceDeleter provides access to removing namespaces from StorageOS.
+// NamespaceDeleter provides access to removing namespaces from StorageOS.
+//
 //go:generate mockgen -build_flags=--mod=vendor -source=reconciler.go -destination=mocks/mock_namespace_deleter.go -package=mocks . NamespaceDeleter
 type NamespaceDeleter interface {
 	DeleteNamespace(ctx context.Context, key client.ObjectKey) error

diff --git a/controllers/node-delete/reconciler.go b/controllers/node-delete/reconciler.go
@@ -14,7 +14,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 )
 
-//NodeDeleter provides access to removing nodes from StorageOS.
+// NodeDeleter provides access to removing nodes from StorageOS.
+//
 //go:generate mockgen -build_flags=--mod=vendor -source=reconciler.go -destination=mocks/mock_node_deleter.go -package=mocks . NodeDeleter
 type NodeDeleter interface {
 	DeleteNode(ctx context.Context, key client.ObjectKey) error

diff --git a/controllers/node-label/reconciler.go b/controllers/node-label/reconciler.go
@@ -17,6 +17,7 @@ import (
 )
 
 // NodeLabeller provides access to update node labels.
+//
 //go:generate mockgen -build_flags=--mod=vendor -source=reconciler.go -destination=mocks/mock_node_labeller.go -package=mocks . NodeLabeller
 type NodeLabeller interface {
 	EnsureNodeLabels(ctx context.Context, key client.ObjectKey, labels map[string]string) error

diff --git a/controllers/pvc-label/reconciler.go b/controllers/pvc-label/reconciler.go
@@ -17,6 +17,7 @@ import (
 )
 
 // VolumeLabeller provides access to update volume labels.
+//
 //go:generate mockgen -build_flags=--mod=vendor -source=reconciler.go -destination=mocks/mock_volume_labeller.go -package=mocks . VolumeLabeller
 type VolumeLabeller interface {
 	EnsureVolumeLabels(ctx context.Context, key client.ObjectKey, labels map[string]string) error

diff --git a/internal/controllers/pool/controller.go b/internal/controllers/pool/controller.go
@@ -57,6 +57,7 @@ const (
 )
 
 // Pool provides access to StorageOS Pools.
+//
 //go:generate mockgen -build_flags=--mod=vendor -source=controller.go -destination=mocks/mock_pool.go -package=mocks . Pool
 type Pool interface {
 	GetPool(ctx context.Context, key client.ObjectKey) (*stosv1.Pool, error)

diff --git a/internal/controllers/sharedvolume/controller.go b/internal/controllers/sharedvolume/controller.go
@@ -50,6 +50,7 @@ import (
 )
 
 // VolumeSharer provides access to StorageOS SharedVolumes.
+//
 //go:generate mockgen -build_flags=--mod=vendor -source=controller.go -destination=mocks/mock_volume_sharer.go -package=mocks . VolumeSharer
 type VolumeSharer interface {
 	SetExternalEndpoint(ctx context.Context, volID string, namespace string, endpoint string) error

diff --git a/internal/controllers/sync-node/controller.go b/internal/controllers/sync-node/controller.go
@@ -52,6 +52,7 @@ const (
 )
 
 // Lister provides access to StorageOS Nodes.
+//
 //go:generate mockgen -build_flags=--mod=vendor -source=controller.go -destination=mocks/mock_lister.go -package=mocks . Lister
 type Lister interface {
 	NodeObjects(context.Context) (map[client.ObjectKey]storageos.Object, error)

diff --git a/internal/controllers/sync-volume/controller.go b/internal/controllers/sync-volume/controller.go
@@ -51,6 +51,7 @@ const (
 )
 
 // Lister provides access to StorageOS Volumes.
+//
 //go:generate mockgen -build_flags=--mod=vendor -source=controller.go -destination=mocks/mock_lister.go -package=mocks . Lister
 type Lister interface {
 	ListVolumes(context.Context) ([]storageos.Object, error)

diff --git a/internal/pkg/storageos/labels.go b/internal/pkg/storageos/labels.go
@@ -11,6 +11,12 @@ const (
 	// apply to the StorageOS object.
 	ReservedLabelPrefix = "storageos.com/"
 
+	// DataplaneReservedLabelPrefix is the reserved prefix shared by
+	// all special labels which pertain to the dataplane. The CP need not
+	// interpret these labels, but it must pass them to the dataplane in
+	// any RPC calls it makes which pertain to the volume.
+	DataplaneReservedLabelPrefix = "storageos.com/dataplane."
+
 	// ReservedLabelComputeOnly is the node label used to indicate that a node
 	// should not store data and should instead access it remotely.
 	ReservedLabelComputeOnly = ReservedLabelPrefix + "computeonly"
@@ -89,9 +95,20 @@ var (
 	ErrReservedLabelFixed = errors.New("behaviour can't be changed after creation")
 )
 
+// isDataplaneReservedLabel returns whether a label has the prefix "storageos.com/dataplane."
+// in which case the label is for the dataplane and does not need to be interpreterd by the
+// control plane
+func isDataplaneReservedLabel(labelKey string) bool {
+	return strings.HasPrefix(labelKey, DataplaneReservedLabelPrefix)
+}
+
 // IsReservedLabel returns true if the key is a StorageOS reserved label name.
 // It does not validate whether the key is valid.
 func IsReservedLabel(key string) bool {
+	if isDataplaneReservedLabel(key) {
+		return false
+	}
+
 	if strings.HasPrefix(key, ReservedLabelPrefix) {
 		return true
 	}

diff --git a/internal/pkg/storageos/labels_test.go b/internal/pkg/storageos/labels_test.go
@@ -11,6 +11,10 @@ func TestIsReservedLabel(t *testing.T) {
 			key:          "not-storage-os",
 			wantReserved: false,
 		},
+		{
+			key:          "storageos.com/dataplane.foo",
+			wantReserved: false,
+		},
 		{
 			key:          ReservedLabelPrefix,
 			wantReserved: true,
@@ -44,3 +48,54 @@ func TestIsReservedLabel(t *testing.T) {
 		})
 	}
 }
+
+func TestIsDataplaneReservedLabel(t *testing.T) {
+	testcases := []struct {
+		key          string
+		wantReserved bool
+	}{
+		{
+			key:          "not-storage-os",
+			wantReserved: false,
+		},
+		{
+			key:          "storageos.com/dataplane.foo",
+			wantReserved: true,
+		},
+		{
+			key:          "storageos.com/dataplane.foo.bar.gaz",
+			wantReserved: true,
+		},
+		{
+			key:          ReservedLabelPrefix,
+			wantReserved: false,
+		},
+		{
+			key:          ReservedLabelPrefix + "foo",
+			wantReserved: false,
+		},
+		{
+			key:          ReservedLabelK8sPVCNamespace,
+			wantReserved: false,
+		},
+		{
+			key:          ReservedLabelK8sPVCName,
+			wantReserved: false,
+		},
+		{
+			key:          ReservedLabelK8sPVName,
+			wantReserved: false,
+		},
+	}
+
+	for _, tc := range testcases {
+		tc := tc
+		t.Run(tc.key, func(t *testing.T) {
+			reserved := isDataplaneReservedLabel(tc.key)
+
+			if reserved != tc.wantReserved {
+				t.Errorf("reserved must match got:\n%t\n, want:\n%t", reserved, tc.wantReserved)
+			}
+		})
+	}
+}
diff --git a/internal/pkg/storageos/node.go b/internal/pkg/storageos/node.go
@@ -139,9 +139,10 @@ func nodeToCR(n stosapi.Node) *stosv1.Node {
 
 // DeleteNode removes a node from the StorageOS cluster.  Delete will fail if
 // pre-requisites are not met:
-// 		(1) The node appears offline
-// 		(2) No node lock is held
-//      (3) No master deployments live on the node (i.e. node must be detected as no active volumes).
+//
+//			(1) The node appears offline
+//			(2) No node lock is held
+//	     (3) No master deployments live on the node (i.e. node must be detected as no active volumes).
 func (c *Client) DeleteNode(ctx context.Context, key client.ObjectKey) error {
 	funcName := "delete_node"
 	start := time.Now()

diff --git a/internal/pkg/storageos/volume_labels.go b/internal/pkg/storageos/volume_labels.go
@@ -54,6 +54,11 @@ func (c *Client) EnsureVolumeLabels(ctx context.Context, key client.ObjectKey, l
 
 	for k, v := range labels {
 		switch {
+		case isDataplaneReservedLabel(k):
+			// The dataplane labels are passed to the control-plane in the same
+			// way unreserved labels are. The control-plane does not interpret
+			// these labels but simply passes them onto the dataplane.
+			unreservedLabels[k] = v
 		case !IsReservedLabel(k):
 			unreservedLabels[k] = v
 		case k == ReservedLabelComputeOnly: