feat(provisioner-localpv): Merge CAS config from PersistentVolumeClaim (

#190) * Merge CAS config from PersistentVolumeClaim Signed-off-by: Nobi <[email protected]> Signed-off-by: Niladri Halder <[email protected]> * feat(provisioner): prefer StorageClass config over PVC config In the typical use-case for the provisioner, an admin sets the config on the StorageClass, and a user sets the config on the PVC. The admin's config may set boundaries on to the volumes which the PVC config map try to override. Preferring StorageClass config when there is a conflict may be the right way to go. Signed-off-by: Niladri Halder <[email protected]> * tests: add integration test for pvc config Signed-off-by: Niladri Halder <[email protected]> * chore(changelog): add changelog entry Signed-off-by: Niladri Halder <[email protected]> --------- Signed-off-by: Nobi <[email protected]> Signed-off-by: Niladri Halder <[email protected]> Co-authored-by: Nobi <[email protected]>
openebs · Jul 15, 2024 · cab53c4 · cab53c4
1 parent 1034995
commit cab53c4
Show file tree

Hide file tree

Showing 9 changed files with 482 additions and 101 deletions.
diff --git a/Makefile b/Makefile
@@ -124,7 +124,7 @@ testv: format
 # Requires KUBECONFIG env and Ginkgo binary
 .PHONY: integration-test
 integration-test:
-	@cd tests && sudo -E env "PATH=${PATH}" ginkgo -v -failFast
+	@cd tests && sudo -E env "PATH=${PATH}" ginkgo -v --fail-fast
 
 .PHONY: format
 format:

diff --git a/changelogs/unreleased/190-nobiit b/changelogs/unreleased/190-nobiit
@@ -0,0 +1 @@
+Add feature to merge cas-config from PVC
diff --git a/cmd/provisioner-localpv/app/config.go b/cmd/provisioner-localpv/app/config.go
@@ -1,20 +1,3 @@
-/*
-Copyright 2019 The OpenEBS Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-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.
-See the License for the specific language governing permissions and
-limitations under the License.
-
-*/
-
 package app
 
 import (
@@ -26,7 +9,7 @@ import (
 	cast "github.com/openebs/maya/pkg/castemplate/v1alpha1"
 	hostpath "github.com/openebs/maya/pkg/hostpath/v1alpha1"
 	"github.com/openebs/maya/pkg/util"
-	errors "github.com/pkg/errors"
+	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog/v2"
@@ -143,7 +126,7 @@ const (
 )
 
 // GetVolumeConfig creates a new VolumeConfig struct by
-// parsing and merging the configuration provided in the PVC
+// parsing and merging the configuration provided in the PVC/SC
 // annotation - cas.openebs.io/config with the
 // default configuration of the provisioner.
 func (p *Provisioner) GetVolumeConfig(ctx context.Context, pvName string, pvc *corev1.PersistentVolumeClaim) (*VolumeConfig, error) {
@@ -169,16 +152,23 @@ func (p *Provisioner) GetVolumeConfig(ctx context.Context, pvName string, pvc *c
 		}
 	}
 
-	//TODO : extract and merge the cas volume config from pvc
-	//This block can be added once validation checks are added
-	// as to the type of config that can be passed via PVC
-	//pvcCASConfigStr := pvc.ObjectMeta.Annotations[string(mconfig.CASConfigKey)]
-	//if len(strings.TrimSpace(pvcCASConfigStr)) != 0 {
-	//	pvcCASConfig, err := cast.UnMarshallToConfig(pvcCASConfigStr)
-	//	if err == nil {
-	//		pvConfig = cast.MergeConfig(pvcCASConfig, pvConfig)
-	//	}
-	//}
+	// Extract and merge the cas config from persistentvolumeclaim.
+	// TODO: Validation checks for what all cas-config options can be
+	// set on the PVC.
+	pvcCASConfigStr := pvc.Annotations[string(mconfig.CASConfigKey)]
+	klog.V(4).Infof("PVC %v has config:%v", pvc.Name, pvcCASConfigStr)
+	if len(strings.TrimSpace(pvcCASConfigStr)) != 0 {
+		pvcCASConfig, err := cast.UnMarshallToConfig(pvcCASConfigStr)
+		if err == nil {
+			// Config keys which already exist (SC config),
+			// will be skipped
+			// i.e. SC config will have precedence over PVC config,
+			// if both have the same keys
+			pvConfig = cast.MergeConfig(pvConfig, pvcCASConfig)
+		} else {
+			return nil, errors.Wrapf(err, "failed to get config: invalid pvc config {%v}", pvcCASConfigStr)
+		}
+	}
 
 	pvConfigMap, err := cast.ConfigToMap(pvConfig)
 	if err != nil {

diff --git a/go.mod b/go.mod
@@ -16,6 +16,7 @@ require (
 	k8s.io/client-go v0.27.2
 	k8s.io/klog/v2 v2.100.1
 	sigs.k8s.io/sig-storage-lib-external-provisioner/v9 v9.0.3
+	sigs.k8s.io/yaml v1.3.0
 )
 
 require (
@@ -84,7 +85,6 @@ require (
 	k8s.io/utils v0.0.0-20230505201702-9f6742963106 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
-	sigs.k8s.io/yaml v1.3.0 // indirect
 )
 
 replace (

diff --git a/pkg/kubernetes/api/apps/v1/deployment/deployment.go b/pkg/kubernetes/api/apps/v1/deployment/deployment.go
@@ -1,24 +1,12 @@
-/*
-Copyright 2019 The OpenEBS Authors
-Licensed under the Apache License, Version 2.0 (the "License");
-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.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
 package v1alpha1
 
 import (
-	templatespec "github.com/openebs/dynamic-localpv-provisioner/pkg/kubernetes/api/core/v1/podtemplatespec"
 	stringer "github.com/openebs/maya/pkg/apis/stringer/v1alpha1"
-	errors "github.com/pkg/errors"
+	"github.com/pkg/errors"
 	appsv1 "k8s.io/api/apps/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	templatespec "github.com/openebs/dynamic-localpv-provisioner/pkg/kubernetes/api/core/v1/podtemplatespec"
 )
 
 // Predicate abstracts conditional logic w.r.t the deployment instance
@@ -96,6 +84,19 @@ func (b *Builder) WithName(name string) *Builder {
 	return b
 }
 
+// WithGenerateName sets the Name field of deployment with a random value with the provided value as a prefix.
+func (b *Builder) WithGenerateName(prefix string) *Builder {
+	if len(prefix) == 0 {
+		b.errors = append(
+			b.errors,
+			errors.New("failed to build deployment: missing prefix for generateName"),
+		)
+		return b
+	}
+	b.deployment.object.GenerateName = prefix + "-"
+	return b
+}
+
 // WithNamespace sets the Namespace field of deployment with provided value.
 func (b *Builder) WithNamespace(namespace string) *Builder {
 	if len(namespace) == 0 {

diff --git a/pkg/kubernetes/api/core/v1/persistentvolumeclaim/build.go b/pkg/kubernetes/api/core/v1/persistentvolumeclaim/build.go
@@ -1,23 +1,7 @@
-/*
-Copyright 2020 The OpenEBS Authors
-
-Licensed under the Apache License, Version 2.0 (the "License");
-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.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
 package persistentvolumeclaim
 
 import (
-	errors "github.com/pkg/errors"
+	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 )
@@ -72,7 +56,7 @@ func (b *Builder) WithGenerateName(name string) *Builder {
 		return b
 	}
 
-	b.pvc.object.GenerateName = name
+	b.pvc.object.GenerateName = name + "-"
 	return b
 }
 

diff --git a/pkg/kubernetes/api/storage/v1/storageclass/build.go b/pkg/kubernetes/api/storage/v1/storageclass/build.go
@@ -1,19 +1,3 @@
-/*
-Copyright 2019 The OpenEBS Authors
-
-Licensed under the Apache License, Version 2.0 (the "License");
-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.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
 package storageclass
 
 import (
@@ -96,6 +80,24 @@ func WithLabels(labels map[string]string) StorageClassOption {
 	}
 }
 
+func WithAnnotations(annotations map[string]string) StorageClassOption {
+	return func(s *storagev1.StorageClass) error {
+		if len(annotations) == 0 {
+			return errors.New("Failed to set Annotations. " +
+				"Input is invalid.")
+		}
+
+		if s.ObjectMeta.Annotations == nil {
+			s.ObjectMeta.Annotations = map[string]string{}
+		}
+		for key, value := range annotations {
+			s.ObjectMeta.Annotations[key] = value
+		}
+
+		return nil
+	}
+}
+
 func WithParameters(parameters map[string]string) StorageClassOption {
 	return func(s *storagev1.StorageClass) error {
 		if len(parameters) == 0 {

diff --git a/tests/operations.go b/tests/operations.go
@@ -1,26 +1,9 @@
-/*
-Copyright 2019 The OpenEBS Authors
-
-Licensed under the Apache License, Version 2.0 (the "License");
-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.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
 package tests
 
 import (
 	"bytes"
 	"context"
 	"fmt"
-
 	//"sort"
 	"strconv"
 	"strings"
@@ -50,15 +33,15 @@ import (
 	"k8s.io/client-go/tools/remotecommand"
 
 	deploy "github.com/openebs/dynamic-localpv-provisioner/pkg/kubernetes/api/apps/v1/deployment"
-	container "github.com/openebs/dynamic-localpv-provisioner/pkg/kubernetes/api/core/v1/container"
-	event "github.com/openebs/dynamic-localpv-provisioner/pkg/kubernetes/api/core/v1/event"
+	"github.com/openebs/dynamic-localpv-provisioner/pkg/kubernetes/api/core/v1/container"
+	"github.com/openebs/dynamic-localpv-provisioner/pkg/kubernetes/api/core/v1/event"
 	pv "github.com/openebs/dynamic-localpv-provisioner/pkg/kubernetes/api/core/v1/persistentvolume"
 	pvc "github.com/openebs/dynamic-localpv-provisioner/pkg/kubernetes/api/core/v1/persistentvolumeclaim"
-	pod "github.com/openebs/dynamic-localpv-provisioner/pkg/kubernetes/api/core/v1/pod"
+	"github.com/openebs/dynamic-localpv-provisioner/pkg/kubernetes/api/core/v1/pod"
 	pts "github.com/openebs/dynamic-localpv-provisioner/pkg/kubernetes/api/core/v1/podtemplatespec"
 	k8svolume "github.com/openebs/dynamic-localpv-provisioner/pkg/kubernetes/api/core/v1/volume"
 	sc "github.com/openebs/dynamic-localpv-provisioner/pkg/kubernetes/api/storage/v1/storageclass"
-	ndmconfig "github.com/openebs/dynamic-localpv-provisioner/pkg/kubernetes/ndmconfig"
+	"github.com/openebs/dynamic-localpv-provisioner/pkg/kubernetes/ndmconfig"
 )
 
 const (
@@ -476,6 +459,24 @@ func (ops *Operations) GetPVNameFromPVCName(namespace, pvcName string) string {
 	return p.Spec.VolumeName
 }
 
+// GetNodeAffinityLabelKeysFromPv returns the label keys for NodeSelector MatchExpressions in a PV.
+func (ops *Operations) GetNodeAffinityLabelKeysFromPv(pvName string) ([]string, error) {
+	pv, err := ops.PVClient.Get(context.TODO(), pvName, metav1.GetOptions{})
+	if err != nil {
+		return nil, err
+	}
+
+	var nodeAffinityLabelKeys []string
+
+	for _, selectorTerm := range pv.Spec.NodeAffinity.Required.NodeSelectorTerms {
+		for _, matchExpression := range selectorTerm.MatchExpressions {
+			nodeAffinityLabelKeys = append(nodeAffinityLabelKeys, matchExpression.Key)
+		}
+	}
+
+	return nodeAffinityLabelKeys, nil
+}
+
 // isNotFound returns true if the original
 // cause of error was due to castemplate's
 // not found error or kubernetes not found