From bfafb340a48e7936ebeb9bea9c1843ff715d38a9 Mon Sep 17 00:00:00 2001 From: Mikhail Borisov <38663549+mishoyama@users.noreply.github.com> Date: Mon, 24 Jan 2022 14:43:50 +0300 Subject: [PATCH] [ISSUE-707] volume release label added (#708) Signed-off-by: Mikhail Borisov --- pkg/base/k8s/kubeclient.go | 21 ++++---- pkg/base/k8s/kubeclient_test.go | 16 +++---- pkg/common/volume_operations.go | 25 +++++++--- pkg/common/volume_operations_test.go | 71 ++++++++++++++++++++++++---- pkg/controller/controller_test.go | 14 +++--- 5 files changed, 105 insertions(+), 42 deletions(-) diff --git a/pkg/base/k8s/kubeclient.go b/pkg/base/k8s/kubeclient.go index 82d8a4e24..39fe5e4ae 100644 --- a/pkg/base/k8s/kubeclient.go +++ b/pkg/base/k8s/kubeclient.go @@ -58,6 +58,8 @@ const ( AppLabelKey = "app.kubernetes.io/name" // AppLabelShortKey matches CSI CRs with csi-baremetal app AppLabelShortKey = "app" + // ReleaseLabelKey matches CSI CRs with the helm release + ReleaseLabelKey = "release" // AppLabelValue matches CSI CRs with csi-baremetal app AppLabelValue = "csi-baremetal" ) @@ -227,7 +229,8 @@ func (k *KubeClient) ConstructLVGCR(name string, apiLVG api.LogicalVolumeGroup) // ConstructVolumeCR constructs Volume custom resource from api.Volume struct // Receives a name for k8s ObjectMeta and an instance of api.Volume struct // Returns an instance of Volume CR struct -func (k *KubeClient) ConstructVolumeCR(name, namespace, appName string, apiVolume api.Volume) *volumecrd.Volume { +func (k *KubeClient) ConstructVolumeCR(name, namespace string, labels map[string]string, + apiVolume api.Volume) *volumecrd.Volume { return &volumecrd.Volume{ TypeMeta: apisV1.TypeMeta{ Kind: crdV1.VolumeKind, @@ -236,7 +239,7 @@ func (k *KubeClient) ConstructVolumeCR(name, namespace, appName string, apiVolum ObjectMeta: apisV1.ObjectMeta{ Name: name, Namespace: namespace, - Labels: constructCustomAppMap(appName), + Labels: labels, }, Spec: apiVolume, } @@ -452,16 +455,10 @@ func PrepareScheme() (*runtime.Scheme, error) { } // constructAppMap creates the map contains app labels -func constructDefaultAppMap() map[string]string { - return constructCustomAppMap(AppLabelValue) -} - -func constructCustomAppMap(appName string) (labels map[string]string) { - if appName != "" { - labels = map[string]string{ - AppLabelKey: appName, - AppLabelShortKey: appName, - } +func constructDefaultAppMap() (labels map[string]string) { + labels = map[string]string{ + AppLabelKey: AppLabelValue, + AppLabelShortKey: AppLabelValue, } return } diff --git a/pkg/base/k8s/kubeclient_test.go b/pkg/base/k8s/kubeclient_test.go index d65b30209..ca67c1083 100644 --- a/pkg/base/k8s/kubeclient_test.go +++ b/pkg/base/k8s/kubeclient_test.go @@ -44,15 +44,15 @@ const ( testID = "someID" testNode1Name = "node1" testDriveLocation1 = "drive" - testApp = "app" ) var ( - testLogger = logrus.New() - testCtx = context.Background() - testUUID = uuid.New().String() - testUUID2 = uuid.New().String() - testVolume = vcrd.Volume{ + testLogger = logrus.New() + testCtx = context.Background() + testUUID = uuid.New().String() + testUUID2 = uuid.New().String() + testAppLabels = map[string]string{AppLabelKey: "app", ReleaseLabelKey: "release"} + testVolume = vcrd.Volume{ TypeMeta: k8smetav1.TypeMeta{Kind: "Volume", APIVersion: apiV1.APIV1Version}, ObjectMeta: k8smetav1.ObjectMeta{Name: testID, Namespace: testNs}, Spec: api.Volume{ @@ -458,13 +458,13 @@ var _ = Describe("Constructor methods", func() { }) Context("ConstructVolumeCR", func() { It("Should return right Volume CR", func() { - constructedCR := k8sclient.ConstructVolumeCR(testApiVolume.Id, testNs, testApp, testApiVolume) + constructedCR := k8sclient.ConstructVolumeCR(testApiVolume.Id, testNs, testAppLabels, testApiVolume) Expect(constructedCR.TypeMeta.Kind).To(Equal(testVolumeCR.TypeMeta.Kind)) Expect(constructedCR.TypeMeta.APIVersion).To(Equal(testVolumeCR.TypeMeta.APIVersion)) Expect(constructedCR.ObjectMeta.Name).To(Equal(testVolumeCR.ObjectMeta.Name)) Expect(constructedCR.ObjectMeta.Namespace).To(Equal(testVolumeCR.ObjectMeta.Namespace)) Expect(constructedCR.Spec).To(Equal(testVolumeCR.Spec)) - Expect(constructedCR.Labels).To(Equal(constructCustomAppMap(testApp))) + Expect(constructedCR.Labels).To(Equal(testAppLabels)) }) }) Context("ConstructLVGCR", func() { diff --git a/pkg/common/volume_operations.go b/pkg/common/volume_operations.go index 449c8d8c4..3dc33d590 100644 --- a/pkg/common/volume_operations.go +++ b/pkg/common/volume_operations.go @@ -158,7 +158,7 @@ func (vo *VolumeOperationsImpl) handleVolumeCreation(ctx context.Context, log *l isFound = false ac = &accrd.AvailableCapacity{} volumeReservation = podReservation.Spec.ReservationRequests[volumeReservationNum] - appLabel string + claimLabels map[string]string ) // scheduler extender reserves capacity on different nodes during filter stage since 'reserve' API is not available // need to find capacity on requested node @@ -207,7 +207,7 @@ func (vo *VolumeOperationsImpl) handleVolumeCreation(ctx context.Context, log *l } if !v.Ephemeral { - appLabel, err = vo.getVolumeAppLabel(ctx, reservationName, podNamespace) + claimLabels, err = vo.getPersistentVolumeClaimLabels(ctx, reservationName, podNamespace) if err != nil { log.Errorf("Unable to get related PVC, error: %v", err) return nil, status.Errorf(codes.Internal, "unable to get related PVC") @@ -230,7 +230,7 @@ func (vo *VolumeOperationsImpl) handleVolumeCreation(ctx context.Context, log *l Mode: v.Mode, Type: v.Type, } - volumeCR := vo.k8sClient.ConstructVolumeCR(v.Id, podNamespace, appLabel, apiVolume) + volumeCR := vo.k8sClient.ConstructVolumeCR(v.Id, podNamespace, claimLabels, apiVolume) if err = vo.k8sClient.CreateCR(ctx, v.Id, volumeCR); err != nil { log.Errorf("Unable to create CR, error: %v", err) @@ -674,8 +674,9 @@ func (vo *VolumeOperationsImpl) fillCache() { } } -// getVolumeAppLabel returns App name related with PVC -func (vo *VolumeOperationsImpl) getVolumeAppLabel(ctx context.Context, pvcName, pvcNamespace string) (string, error) { +// VolumeOperationsImpl returns PVC labels: release, app.kubernetes.io/name and adds short app label +func (vo *VolumeOperationsImpl) getPersistentVolumeClaimLabels(ctx context.Context, pvcName, pvcNamespace string) ( + map[string]string, error) { ll := vo.log.WithFields(logrus.Fields{ "method": "getVolumeAppLabel", }) @@ -686,8 +687,18 @@ func (vo *VolumeOperationsImpl) getVolumeAppLabel(ctx context.Context, pvcName, Namespace: pvcNamespace, }, pvc); err != nil { ll.Errorf("Failed to get PVC %s in namespace %s, error %v", pvcName, pvcNamespace, err) - return "", err + return nil, err + } + + // need to get release and app labels only + labels := map[string]string{} + if value, ok := pvc.GetLabels()[k8s.ReleaseLabelKey]; ok { + labels[k8s.ReleaseLabelKey] = value + } + if value, ok := pvc.GetLabels()[k8s.AppLabelKey]; ok { + labels[k8s.AppLabelKey] = value + labels[k8s.AppLabelShortKey] = value } - return pvc.GetLabels()[k8s.AppLabelKey], nil + return labels, nil } diff --git a/pkg/common/volume_operations_test.go b/pkg/common/volume_operations_test.go index 30fd2c7c5..778b0cbee 100644 --- a/pkg/common/volume_operations_test.go +++ b/pkg/common/volume_operations_test.go @@ -16,6 +16,68 @@ limitations under the License. package common +import ( + "context" + "github.com/sirupsen/logrus" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "testing" + + "github.com/dell/csi-baremetal/pkg/base/cache" + "github.com/dell/csi-baremetal/pkg/base/featureconfig" + "github.com/dell/csi-baremetal/pkg/base/k8s" + "github.com/stretchr/testify/assert" +) + +var ( + namespace = "my-namespace" + testLogger = logrus.New() +) + +// creates fake k8s client and creates AC CRs based on provided acs +// returns instance of ACOperationsImpl based on created k8s client +func setupVOOperationsTest(t *testing.T) *VolumeOperationsImpl { + k8sClient, err := k8s.GetFakeKubeClient(namespace, testLogger) + assert.Nil(t, err) + assert.NotNil(t, k8sClient) + + return NewVolumeOperationsImpl(k8sClient, testLogger, cache.NewMemCache(), featureconfig.NewFeatureConfig()) +} + +func Test_getPersistentVolumeClaimLabels(t *testing.T) { + var ( + svc = setupVOOperationsTest(t) + ctx = context.TODO() + pvcName = "my-pvc" + ) + // no PVC + labels, err := svc.getPersistentVolumeClaimLabels(ctx, pvcName, namespace) + assert.Nil(t, labels) + assert.NotNil(t, err) + + // create PVC + var ( + appName = "my-app" + releaseName = "my-release" + pvcLabels = map[string]string{ + k8s.AppLabelKey: appName, + k8s.ReleaseLabelKey: releaseName, + } + pvc = &v1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: pvcName, Namespace: namespace, + Labels: pvcLabels}} + ) + err = svc.k8sClient.Create(ctx, pvc) + assert.Nil(t, err) + + // check labels + labels, err = svc.getPersistentVolumeClaimLabels(ctx, pvcName, namespace) + assert.NotNil(t, labels) + assert.Nil(t, err) + assert.Equal(t, labels[k8s.AppLabelKey], appName) + assert.Equal(t, labels[k8s.AppLabelShortKey], appName) + assert.Equal(t, labels[k8s.ReleaseLabelKey], releaseName) +} + // TODO - refactor UTs https://github.com/dell/csi-baremetal/issues/371 /*func TestVolumeOperationsImpl_CreateVolume_VolumeExists(t *testing.T) { // 1. Volume CR has already exist @@ -597,15 +659,6 @@ func TestVolumeOperationsImpl_deleteLVGIfVolumesNotExistOrUpdate(t *testing.T) { assert.True(t, k8sError.IsNotFound(err)) } -// creates fake k8s client and creates AC CRs based on provided acs -// returns instance of ACOperationsImpl based on created k8s client -func setupVOOperationsTest(t *testing.T) *VolumeOperationsImpl { - k8sClient, err := k8s.GetFakeKubeClient(testNS, testLogger) - assert.Nil(t, err) - assert.NotNil(t, k8sClient) - - return NewVolumeOperationsImpl(k8sClient, testLogger, cache.NewMemCache(), featureconfig.NewFeatureConfig()) -} func buildVolumePlacingPlan(node string, vol *api.Volume, ac *accrd.AvailableCapacity) *capacityplanner.VolumesPlacingPlan { diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 88d8cc345..0e779d3cb 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -54,11 +54,12 @@ import ( ) var ( - testLogger = logrus.New() - testID = "someID" - testNs = "default" - testApp = "app" - testPod = "pod" + testLogger = logrus.New() + testID = "someID" + testNs = "default" + testApp = "app" + testAppLabels = map[string]string{} + testPod = "pod" testCtx = context.Background() testNode1Name = "node1" @@ -374,7 +375,8 @@ var _ = Describe("CSIControllerService DeleteVolume", func() { err error ) // create volume crd to delete - volumeCrd = controller.k8sclient.ConstructVolumeCR(volumeID, testNs, testApp, api.Volume{Id: volumeID, CSIStatus: apiV1.Created}) + volumeCrd = controller.k8sclient.ConstructVolumeCR(volumeID, testNs, testAppLabels, api.Volume{Id: volumeID, + CSIStatus: apiV1.Created}) err = controller.k8sclient.CreateCR(testCtx, volumeID, volumeCrd) Expect(err).To(BeNil()) fillCache(controller, volumeID, testNs)