From 6a2e5612469667ba7ad3681a42167122b397c3e9 Mon Sep 17 00:00:00 2001 From: Rahul M Chheda <53308066+rahulchheda@users.noreply.github.com> Date: Fri, 6 Dec 2019 19:15:02 +0530 Subject: [PATCH] feat(localPV): add Raw Block Volume support (#1536) - Added Builder Function for support & checking Block Device - Returned true, for support of the Block Device - Provided the path for the raw Block device, if provided - Added containerBuilder function (funcName: WithVolumeDevices Builder) - Added pvcBuilder function (funcname: WithVolumeMode Builder) - Added 2 BDD TestCases : - [+ve] Basic Raw Block Volume Support - [-ve] If PVC volumeMode is "Block", but mountPath is provided in Deployment PodSpecTemplate Signed-off-by: Rahul M Chheda --- .../app/helper_blockdevice.go | 15 +- cmd/provisioner-localpv/app/provisioner.go | 7 +- .../app/provisioner_blockdevice.go | 22 +- pkg/blockdeviceclaim/v1alpha1/build.go | 16 + .../container/v1alpha1/container.go | 22 ++ .../persistentvolumeclaim/v1alpha1/build.go | 7 + tests/localpv/hostdevice_test.go | 318 +++++++++++++++++- 7 files changed, 392 insertions(+), 15 deletions(-) diff --git a/cmd/provisioner-localpv/app/helper_blockdevice.go b/cmd/provisioner-localpv/app/helper_blockdevice.go index ae78125204..9669ae4e02 100644 --- a/cmd/provisioner-localpv/app/helper_blockdevice.go +++ b/cmd/provisioner-localpv/app/helper_blockdevice.go @@ -63,6 +63,8 @@ type HelperBlockDeviceOptions struct { capacity string // deviceType string bdcName string + // volumeMode of PVC + volumeMode corev1.PersistentVolumeMode } // validate checks that the required fields to create BDC @@ -98,7 +100,6 @@ func (p *Provisioner) createBlockDeviceClaim(blkDevOpts *HelperBlockDeviceOption if err := blkDevOpts.validate(); err != nil { return err } - //Create a BDC for this PV (of type device). NDM will //look for the device matching the capacity and node on which //pod is being scheduled. Since this BDC is specific to a PV @@ -125,6 +126,7 @@ func (p *Provisioner) createBlockDeviceClaim(blkDevOpts *HelperBlockDeviceOption WithHostName(blkDevOpts.nodeHostname). WithCapacity(blkDevOpts.capacity). WithFinalizer(LocalPVFinalizer). + WithBlockVolumeMode(blkDevOpts.volumeMode). Build() if err != nil { @@ -192,12 +194,19 @@ func (p *Provisioner) getBlockDevicePath(blkDevOpts *HelperBlockDeviceOptions) ( //If the error is about BDC being already present, then return nil return "", "", errors.Errorf("unable to find BD:%v for BDC:%v associated with PV:%v", bdName, blkDevOpts.bdcName, blkDevOpts.name) } - path := bd.Spec.FileSystem.Mountpoint + blkPath := bd.Spec.Path if len(bd.Spec.DevLinks) > 0 { - //TODO : Iterate and get the first path by id. + blkPath = bd.Spec.DevLinks[0].Links[0] + + //Iterate and get the first path by id. + for _, v := range bd.Spec.DevLinks { + if v.Kind == "by-id" { + blkPath = v.Links[0] + } + } } return path, blkPath, nil diff --git a/cmd/provisioner-localpv/app/provisioner.go b/cmd/provisioner-localpv/app/provisioner.go index f2c7aadaf8..b60ed35d60 100644 --- a/cmd/provisioner-localpv/app/provisioner.go +++ b/cmd/provisioner-localpv/app/provisioner.go @@ -78,9 +78,9 @@ func NewProvisioner(stopCh chan struct{}, kubeClient *clientset.Clientset) (*Pro } // SupportsBlock will be used by controller to determine if block mode is -// supported by the host path provisioner. Return false. +// supported by the host path provisioner. func (p *Provisioner) SupportsBlock() bool { - return false + return true } // Provision is invoked by the PVC controller which expect the PV @@ -128,6 +128,9 @@ func (p *Provisioner) Provision(opts pvController.VolumeOptions) (*v1.Persistent if stgType == "device" { return p.ProvisionBlockDevice(opts, pvCASConfig) } + if *opts.PVC.Spec.VolumeMode == v1.PersistentVolumeBlock && stgType != "device" { + return nil, fmt.Errorf("PV with BlockMode is not supported with StorageType %v", stgType) + } alertlog.Logger.Errorw("", "eventcode", "cstor.local.pv.provision.failure", "msg", "Failed to provision CStor Local PV", diff --git a/cmd/provisioner-localpv/app/provisioner_blockdevice.go b/cmd/provisioner-localpv/app/provisioner_blockdevice.go index 0ee188a626..f7436ae39c 100644 --- a/cmd/provisioner-localpv/app/provisioner_blockdevice.go +++ b/cmd/provisioner-localpv/app/provisioner_blockdevice.go @@ -44,6 +44,7 @@ func (p *Provisioner) ProvisionBlockDevice(opts pvController.VolumeOptions, volu nodeHostname: nodeHostname, name: name, capacity: capacity.String(), + volumeMode: *opts.PVC.Spec.VolumeMode, } path, blkPath, err := p.getBlockDevicePath(blkDevOpts) @@ -59,16 +60,13 @@ func (p *Provisioner) ProvisionBlockDevice(opts pvController.VolumeOptions, volu return nil, err } klog.Infof("Creating volume %v on %v at %v(%v)", name, nodeHostname, path, blkPath) + + // Over-ride the path, with the blockPath, when path is empty. if path == "" { path = blkPath klog.Infof("Using block device{%v} with fs{%v}", blkPath, fsType) } - // TODO - // VolumeMode will always be specified as Filesystem for host path volume, - // and the value passed in from the PVC spec will be ignored. - fs := v1.PersistentVolumeFilesystem - // It is possible that the HostPath doesn't already exist on the node. // Set the Local PV to create it. //hostPathType := v1.HostPathDirectoryOrCreate @@ -84,17 +82,23 @@ func (p *Provisioner) ProvisionBlockDevice(opts pvController.VolumeOptions, volu //labels[string(v1alpha1.StorageClassKey)] = *className //TODO Change the following to a builder pattern - pvObj, err := mPV.NewBuilder(). + pvObjBuilder := mPV.NewBuilder(). WithName(name). WithLabels(labels). WithAnnotations(volAnnotations). WithReclaimPolicy(opts.PersistentVolumeReclaimPolicy). WithAccessModes(pvc.Spec.AccessModes). - WithVolumeMode(fs). WithCapacityQty(pvc.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)]). WithLocalHostPathFormat(path, fsType). - WithNodeAffinity(nodeHostname). - Build() + WithNodeAffinity(nodeHostname) + + // If volumeMode set to "Block", then provide the appropriate volumeMode, to pvObj + if *opts.PVC.Spec.VolumeMode == v1.PersistentVolumeBlock { + pvObjBuilder.WithVolumeMode(v1.PersistentVolumeBlock) + } + + //Build the pvObject + pvObj, err := pvObjBuilder.Build() if err != nil { alertlog.Logger.Errorw("", diff --git a/pkg/blockdeviceclaim/v1alpha1/build.go b/pkg/blockdeviceclaim/v1alpha1/build.go index 968b25062c..de50bba7e8 100644 --- a/pkg/blockdeviceclaim/v1alpha1/build.go +++ b/pkg/blockdeviceclaim/v1alpha1/build.go @@ -307,3 +307,19 @@ func (b *Builder) WithFinalizer(finalizers ...string) *Builder { b.BDC.Object.Finalizers = append(b.BDC.Object.Finalizers, finalizers...) return b } + +// WithBlockVolumeMode sets the volumeMode as volumeModeBlock, +// if persistentVolumeMode is set to "Block" +func (b *Builder) WithBlockVolumeMode(mode corev1.PersistentVolumeMode) *Builder { + if len(mode) == 0 { + b.errs = append( + b.errs, + errors.New("failed to build BDC object: missing PersistentVolumeMode"), + ) + } + if mode == corev1.PersistentVolumeBlock { + b.BDC.Object.Spec.Details.BlockVolumeMode = ndm.VolumeModeBlock + } + + return b +} diff --git a/pkg/kubernetes/container/v1alpha1/container.go b/pkg/kubernetes/container/v1alpha1/container.go index 0a98701580..7de57373a5 100644 --- a/pkg/kubernetes/container/v1alpha1/container.go +++ b/pkg/kubernetes/container/v1alpha1/container.go @@ -254,6 +254,28 @@ func (b *Builder) WithVolumeMountsNew(volumeMounts []corev1.VolumeMount) *Builde return b } +// WithVolumeDevices builds the containers with the appropriate volumeDevices +func (b *Builder) WithVolumeDevices(volumeDevices []corev1.VolumeDevice) *Builder { + if volumeDevices == nil { + b.errors = append( + b.errors, + errors.New("failed to build container object: nil volumedevices"), + ) + return b + } + if len(volumeDevices) == 0 { + b.errors = append( + b.errors, + errors.New("failed to build container object: missing volumedevices"), + ) + return b + } + newVolumeDevices := []corev1.VolumeDevice{} + newVolumeDevices = append(newVolumeDevices, volumeDevices...) + b.con.VolumeDevices = newVolumeDevices + return b +} + // WithImagePullPolicy sets the image pull policy of the container func (b *Builder) WithImagePullPolicy(policy corev1.PullPolicy) *Builder { if len(policy) == 0 { diff --git a/pkg/kubernetes/persistentvolumeclaim/v1alpha1/build.go b/pkg/kubernetes/persistentvolumeclaim/v1alpha1/build.go index 50575da6e9..9cf512c597 100644 --- a/pkg/kubernetes/persistentvolumeclaim/v1alpha1/build.go +++ b/pkg/kubernetes/persistentvolumeclaim/v1alpha1/build.go @@ -172,6 +172,13 @@ func (b *Builder) WithCapacity(capacity string) *Builder { return b } +// WithVolumeMode sets the volumeMode field in PVC with provided arguments +func (b *Builder) WithVolumeMode(vM corev1.PersistentVolumeMode) *Builder { + + b.pvc.object.Spec.VolumeMode = &vM + return b +} + // Build returns the PVC API instance func (b *Builder) Build() (*corev1.PersistentVolumeClaim, error) { if len(b.errs) > 0 { diff --git a/tests/localpv/hostdevice_test.go b/tests/localpv/hostdevice_test.go index 81f03de57b..fbd86d2bfd 100644 --- a/tests/localpv/hostdevice_test.go +++ b/tests/localpv/hostdevice_test.go @@ -195,6 +195,171 @@ var _ = Describe("TEST HOSTDEVICE LOCAL PV", func() { }) +var _ = Describe("TEST HOSTDEVICE LOCAL PV WITH VOLUMEMODE AS BLOCK", func() { + var ( + pvcObj *corev1.PersistentVolumeClaim + accessModes = []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce} + capacity = "2Gi" + deployName = "busybox-device" + label = "demo=hostdevice-deployment" + pvcName = "pvc-hd-block" + deployObj *appsv1.Deployment + labelselector = map[string]string{ + "demo": "hostdevice-deployment", + } + ) + + When("pvc with storageclass openebs-device, and volumeMode as Block, is created", func() { + It("should create a pvc ", func() { + var ( + scName = "openebs-device" + blockVolumeMode = corev1.PersistentVolumeBlock + ) + + By("building a pvc") + pvcObj, err = pvc.NewBuilder(). + WithName(pvcName). + WithNamespace(namespaceObj.Name). + WithStorageClass(scName). + WithAccessModes(accessModes). + WithVolumeMode(blockVolumeMode). + WithCapacity(capacity).Build() + Expect(err).ShouldNot( + HaveOccurred(), + "while building pvc {%s} in namespace {%s}", + pvcName, + namespaceObj.Name, + ) + + By("creating above pvc") + pvcObj, err = ops.PVCClient.WithNamespace(namespaceObj.Name).Create(pvcObj) + Expect(err).To( + BeNil(), + "while creating pvc {%s} in namespace {%s}", + pvcName, + namespaceObj.Name, + ) + }) + }) + + When("deployment with busybox image is created", func() { + It("should create a deployment and a running pod", func() { + + By("building a deployment") + deployObj, err = deploy.NewBuilder(). + WithName(deployName). + WithNamespace(namespaceObj.Name). + WithLabelsNew(labelselector). + WithSelectorMatchLabelsNew(labelselector). + WithPodTemplateSpecBuilder( + pts.NewBuilder(). + WithLabelsNew(labelselector). + WithContainerBuildersNew( + container.NewBuilder(). + WithName("busybox"). + WithImage("busybox"). + WithCommandNew( + []string{ + "sleep", + "3600", + }, + ). + WithVolumeDevices( + []corev1.VolumeDevice{ + corev1.VolumeDevice{ + Name: "demo-block-vol1", + DevicePath: "/dev/sdc", + }, + }, + ), + ). + WithVolumeBuildersNew( + volume.NewBuilder(). + WithName("demo-block-vol1"). + WithPVCSource(pvcName), + ), + ). + Build() + Expect(err).ShouldNot( + HaveOccurred(), + "while building delpoyment {%s} in namespace {%s}", + deployName, + namespaceObj.Name, + ) + + By("creating above deployment") + _, err = ops.DeployClient.WithNamespace(namespaceObj.Name). + Create(deployObj) + Expect(err).To( + BeNil(), + "while creating deployment {%s} in namespace {%s}", + deployName, + namespaceObj.Name, + ) + + By("verifying pod count as 1") + podCount := ops.GetPodRunningCountEventually(namespaceObj.Name, label, 1) + Expect(podCount).To(Equal(1), "while verifying pod count") + + }) + }) + + When("remove finalizer", func() { + It("finalizer should come back after provisioner restart", func() { + bdcName := "bdc-pvc-" + string(pvcObj.GetUID()) + bdcObj, err := ops.BDCClient.WithNamespace(string(artifacts.OpenebsNamespace)).Get(bdcName, + metav1.GetOptions{}) + Expect(err).To(BeNil()) + + _, err = blockdeviceclaim.BuilderForAPIObject(bdcObj).WithConfigPath(ops.KubeConfigPath). + BDC.RemoveFinalizer(localpv_app.LocalPVFinalizer) + Expect(err).To(BeNil()) + + podList, err := ops.PodClient. + WithNamespace(string(artifacts.OpenebsNamespace)). + List(metav1.ListOptions{LabelSelector: LocalPVProvisionerLabelSelector}) + Expect(err).To(BeNil()) + err = ops.PodClient.Delete(podList.Items[0].Name, &metav1.DeleteOptions{}) + Expect(err).To(BeNil()) + + Expect(ops.IsFinalizerExistsOnBDC(bdcName, localpv_app.LocalPVFinalizer)).To(BeTrue()) + }) + }) + When("deployment is deleted", func() { + It("should not have any deployment or running pod", func() { + + By("deleting above deployment") + err = ops.DeployClient.WithNamespace(namespaceObj.Name).Delete(deployName, &metav1.DeleteOptions{}) + Expect(err).To( + BeNil(), + "while deleting deployment {%s} in namespace {%s}", + deployName, + namespaceObj.Name, + ) + + By("verifying pod count as 0") + podCount := ops.GetPodRunningCountEventually(namespaceObj.Name, label, 0) + Expect(podCount).To(Equal(0), "while verifying pod count") + + }) + }) + + When("pvc with storageclass openebs-device is deleted ", func() { + It("should delete the pvc", func() { + + By("deleting above pvc") + err = ops.PVCClient.Delete(pvcName, &metav1.DeleteOptions{}) + Expect(err).To( + BeNil(), + "while deleting pvc {%s} in namespace {%s}", + pvcName, + namespaceObj.Name, + ) + + }) + }) + +}) var _ = Describe("[-ve] TEST HOSTDEVICE LOCAL PV", func() { var ( pvcObj *corev1.PersistentVolumeClaim @@ -285,7 +450,7 @@ var _ = Describe("[-ve] TEST HOSTDEVICE LOCAL PV", func() { Build() Expect(err).ShouldNot( HaveOccurred(), - "while building delpoyment {%s} in namespace {%s}", + "while building deployment {%s} in namespace {%s}", existingDeployName, namespaceObj.Name, ) @@ -444,6 +609,157 @@ var _ = Describe("[-ve] TEST HOSTDEVICE LOCAL PV", func() { namespaceObj.Name, ) + By("verifying pod count as 0") + podCount := ops.GetPodRunningCountEventually(namespaceObj.Name, existinglabel, 1) + Expect(podCount).To(Equal(1), "while verifying pod count") + + }) + }) + + When("existing pvc with storageclass openebs-device is deleted ", func() { + It("should delete the pvc", func() { + + By("deleting above pvc") + err = ops.PVCClient.Delete(existingPVCName, &metav1.DeleteOptions{}) + Expect(err).To( + BeNil(), + "while deleting pvc {%s} in namespace {%s}", + existingPVCName, + namespaceObj.Name, + ) + + }) + }) +}) + +var _ = Describe("[-ve] TEST HOSTDEVICE LOCAL PV WITH VOLUMEMODE AS BLOCK ", func() { + var ( + //pvcObj *corev1.PersistentVolumeClaim + accessModes = []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce} + capacity = "2Gi" + //deployName = "busybox-device" + //label = "demo=hostdevice-deployment" + //pvcName = "pvc-hd-block" + //deployObj *appsv1.Deployment + blockVolumeMode = corev1.PersistentVolumeBlock + //labelselector = map[string]string{ + // "demo": "hostdevice-deployment", + //} + scName = "openebs-device" + existingPVCObj *corev1.PersistentVolumeClaim + existingDeployName = "existing-busybox-device" + existinglabel = "demo=existing-hostdevice-deployment" + existingPVCName = "existing-pvc-hd-block" + existingDeployObj *appsv1.Deployment + existingLabelselector = map[string]string{ + "demo": "existing-hostdevice-deployment", + } + ) + When("existing pvc with storageclass openebs-device is created", func() { + It("should create a pvc", func() { + + By("building a pvc") + existingPVCObj, err = pvc.NewBuilder(). + WithName(existingPVCName). + WithNamespace(namespaceObj.Name). + WithStorageClass(scName). + WithAccessModes(accessModes). + WithVolumeMode(blockVolumeMode). + WithCapacity(capacity).Build() + Expect(err).ShouldNot( + HaveOccurred(), + "while building pvc {%s} in namespace {%s}", + existingPVCName, + namespaceObj.Name, + ) + + By("creating above pvc") + _, err = ops.PVCClient.WithNamespace(namespaceObj.Name).Create(existingPVCObj) + Expect(err).To( + BeNil(), + "while creating pvc {%s} in namespace {%s}", + existingPVCName, + namespaceObj.Name, + ) + }) + }) + When("existing deployment with busybox image is created", func() { + It("should create a deployment but should be unable to get a running pod, with PVC volumeMode set to Block,but added as volumeMount in Deployment", func() { + + By("building a deployment, with volume Mount for a Block volumeMode PVC") + existingDeployObj, err = deploy.NewBuilder(). + WithName(existingDeployName). + WithNamespace(namespaceObj.Name). + WithLabelsNew(existingLabelselector). + WithSelectorMatchLabelsNew(existingLabelselector). + WithPodTemplateSpecBuilder( + pts.NewBuilder(). + WithLabelsNew(existingLabelselector). + WithContainerBuildersNew( + container.NewBuilder(). + WithName("busybox"). + WithImage("busybox"). + WithCommandNew( + []string{ + "sleep", + "3600", + }, + ). + WithVolumeMountsNew( + []corev1.VolumeMount{ + corev1.VolumeMount{ + Name: "demo-block-vol2", + MountPath: "/mnt/store1", + }, + }, + ), + ). + WithVolumeBuildersNew( + volume.NewBuilder(). + WithName("demo-block-vol2"). + WithPVCSource(existingPVCName), + ), + ). + Build() + Expect(err).ShouldNot( + HaveOccurred(), + "while building deployment {%s} in namespace {%s}", + existingDeployName, + namespaceObj.Name, + ) + + By("creating above deployment") + _, err = ops.DeployClient.WithNamespace(namespaceObj.Name). + Create(existingDeployObj) + Expect(err).To( + BeNil(), + "while creating deployment {%s} in namespace {%s}", + existingDeployName, + namespaceObj.Name, + ) + + By("verifying pvc status as bound") + status := ops.IsPVCBoundEventually(existingPVCName) + Expect(status).To(Equal(true), "while checking status equal to bound") + + By("verifying pod count as 0") + podCount := ops.GetPodRunningCountEventually(namespaceObj.Name, existinglabel, 1) + Expect(podCount).To(Equal(0), "while verifying pod count") + + }) + }) + When("above deployment is deleted", func() { + It("should not have any deployment or running pod", func() { + + By("deleting above deployment") + err = ops.DeployClient.WithNamespace(namespaceObj.Name).Delete(existingDeployName, &metav1.DeleteOptions{}) + Expect(err).To( + BeNil(), + "while deleting deployment {%s} in namespace {%s}", + existingDeployName, + namespaceObj.Name, + ) + By("verifying pod count as 0") podCount := ops.GetPodRunningCountEventually(namespaceObj.Name, existinglabel, 0) Expect(podCount).To(Equal(0), "while verifying pod count")