Devicemanager: New error codes + Device Manager unit tests and few other fixes.

Jenkinsfile

@@ -159,6 +159,26 @@ pipeline {
             }
         }
 
+        stage('make dm-test') {
+            options {
+                timeout(time: 30, unit: "MINUTES")
+            }
+
+            steps {
+                sh "docker run --rm ${DockerBuildArgs()} \
+                               --privileged=true \
+                               -e TEST_CHECK_SIGNED_FILES=false \
+                               -e TEST_DISTRO=clear \
+                               -e TEST_DISTRO_VERSION=${env.CLEAR_LINUX_VERSION_1_15} \
+                               -v `pwd`:`pwd`:rshared \
+                               -w `pwd` \
+                               ${env.BUILD_IMAGE} bash -c 'set -x; \
+                                   swupd bundle-add openssh-client && \
+                                   make run_dm_tests; \
+                                   make stop'"
+            }
+        }
+
         stage('Build test image') {
             options {
                 timeout(time: 60, unit: "MINUTES")

go.mod

@@ -33,11 +33,12 @@ require (
 	go.uber.org/zap v1.11.0 // indirect
 	golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 // indirect
 	golang.org/x/net v0.0.0-20191027233614-53de4c7853b5
-	golang.org/x/sys v0.0.0-20191105231009-c1f44814a5cd // indirect
+	golang.org/x/sys v0.0.0-20191105231009-c1f44814a5cd
 	golang.org/x/time v0.0.0-20191024005414-555d28b269f0 // indirect
 	google.golang.org/appengine v1.6.5 // indirect
 	google.golang.org/genproto v0.0.0-20191009194640-548a555dbc03 // indirect
 	google.golang.org/grpc v1.24.0
+	gopkg.in/freddierice/go-losetup.v1 v1.0.0-20170407175016-fc9adea44124
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/square/go-jose.v2 v2.4.0 // indirect
 	k8s.io/api v0.0.0

go.sum

@@ -622,6 +622,8 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
+gopkg.in/freddierice/go-losetup.v1 v1.0.0-20170407175016-fc9adea44124 h1:aPcd9iBdqpFyYkoGRQbQd+asp162GIRDvAVB0FhLxhc=
+gopkg.in/freddierice/go-losetup.v1 v1.0.0-20170407175016-fc9adea44124/go.mod h1:6LXpUYtVsrx91XiupFRJ8jVKOqLZf5PrbEVSGHta/84=
 gopkg.in/fsnotify.v1 v1.4.7 h1:xOHLXZwVvI9hhs+cLKq5+I5onOuwQLhQwiu63xxlHs4=
 gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys=
 gopkg.in/gcfg.v1 v1.2.0 h1:0HIbH907iBTAntm+88IJV2qmJALDAh8sPekI9Vc1fm0=

pkg/ndctl/ndctl.go

@@ -8,10 +8,9 @@ package ndctl
 import "C"
 
 import (
+	"errors"
 	"fmt"
-	"os"
 
-	"github.com/pkg/errors"
 	"k8s.io/klog"
 )
 
@@ -24,6 +23,10 @@ const (
 	tib  uint64 = gib * 1024
 )
 
+var (
+	ErrNotExist = errors.New("namespace not found")
+)
+
 //CreateNamespaceOpts options to create a namespace
 type CreateNamespaceOpts struct {
 	Name       string
@@ -81,7 +84,7 @@ func (ctx *Context) CreateNamespace(opts CreateNamespaceOpts) (*Namespace, error
 			}
 		}
 	}
-	return nil, errors.Wrap(err, "failed to create namespace")
+	return nil, err
 }
 
 //DestroyNamespaceByName deletes namespace with given name
@@ -106,7 +109,7 @@ func (ctx *Context) GetNamespaceByName(name string) (*Namespace, error) {
 			}
 		}
 	}
-	return nil, errors.Wrapf(os.ErrNotExist, "namespace '%s' not found", name)
+	return nil, ErrNotExist
 }
 
 //GetActiveNamespaces returns list of all active namespaces in all regions

pkg/ndctl/region.go

@@ -93,7 +93,9 @@ func (r *Region) ActiveNamespaces() []*Namespace {
 	return r.namespaces(true)
 }
 
-//AllNamespaces returns all namespaces in the region
+//AllNamespaces returns all non-zero sized namespaces in the region
+//as sometime a deleted namespace also lies around with size zero, we can ignore
+//such namespace
 func (r *Region) AllNamespaces() []*Namespace {
 	return r.namespaces(false)
 }
@@ -318,8 +320,15 @@ func (r *Region) namespaces(onlyActive bool) []*Namespace {
 	ndr := (*C.struct_ndctl_region)(r)
 
 	for ndns := C.ndctl_namespace_get_first(ndr); ndns != nil; ndns = C.ndctl_namespace_get_next(ndns) {
-		if !onlyActive || C.ndctl_namespace_is_active(ndns) == true {
-			namespaces = append(namespaces, (*Namespace)(ndns))
+		ns := (*Namespace)(ndns)
+		// If asked for only active namespaces return it regardless of it size
+		// if not, return only valid namespaces, i.e, non-zero sized.
+		if onlyActive {
+			if ns.Active() {
+				namespaces = append(namespaces, ns)
+			}
+		} else if ns.Size() > 0 {
+			namespaces = append(namespaces, ns)
 		}
 	}
 

pkg/pmem-csi-driver/controllerserver-master.go

@@ -305,7 +305,7 @@ func (cs *masterController) DeleteVolume(ctx context.Context, req *csi.DeleteVol
 			defer conn.Close() // nolint:errcheck
 			klog.V(4).Infof("Asking node %s to delete volume name:%s id:%s", node, vol.name, vol.id)
 			if _, err := csi.NewControllerClient(conn).DeleteVolume(ctx, req); err != nil {
-				return nil, status.Error(codes.Internal, "Failed to delete volume name:"+vol.name+" id:"+vol.id+" on "+node+": "+err.Error())
+				return nil, err
 			}
 		}
 		cs.mutex.Lock()

pkg/pmem-csi-driver/controllerserver-node.go

@@ -9,6 +9,7 @@ package pmemcsidriver
 import (
 	"crypto/sha1"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"strconv"
 	"sync"
@@ -273,6 +274,9 @@ func (cs *nodeControllerServer) DeleteVolume(ctx context.Context, req *csi.Delet
 	}
 
 	if err := cs.dm.DeleteDevice(req.VolumeId, eraseafter); err != nil {
+		if errors.Is(err, pmdmanager.ErrDeviceInUse) {
+			return nil, status.Errorf(codes.FailedPrecondition, err.Error())
+		}
 		return nil, status.Errorf(codes.Internal, "Failed to delete volume: %s", err.Error())
 	}
 	if cs.sm != nil {

pkg/pmem-csi-driver/nodeserver.go

@@ -7,6 +7,7 @@ SPDX-License-Identifier: Apache-2.0
 package pmemcsidriver
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -127,9 +128,9 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 		// 1) No Volume found with given volume id
 		// 2) No provisioner info found in VolumeContext "storage.kubernetes.io/csiProvisionerIdentity"
 		// 3) No StagingPath in the request
-		// TODO(avalluri): Inspect the error returned by GetDevice() once we
-		// implement the error codes in DeviceManager
-		device, _ = ns.cs.dm.GetDevice(req.VolumeId)
+		if device, err = ns.cs.dm.GetDevice(req.VolumeId); err != nil && !errors.Is(err, pmdmanager.ErrDeviceNotFound) {
+			return nil, status.Errorf(codes.Internal, "failed to get device details for volume id '%s': %v", req.VolumeId, err)
+		}
 		_, ok := req.GetVolumeContext()[volumeProvisionerIdentity]
 		ephemeral = device == nil && !ok && len(srcPath) == 0
 	}
@@ -146,11 +147,11 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 			mountOptions = []string{"dax"}
 		}
 	} else {
-		device, err = ns.cs.dm.GetDevice(req.VolumeId)
-		// TODO(avalluri): Inspect the error returned by GetDevice() once we
-		// implement the error codes in DeviceManager
-		if err != nil {
-			return nil, status.Error(codes.NotFound, "No device found with volume id "+req.VolumeId+": "+err.Error())
+		if device, err = ns.cs.dm.GetDevice(req.VolumeId); err != nil {
+			if errors.Is(err, pmdmanager.ErrDeviceNotFound) {
+				return nil, status.Errorf(codes.NotFound, "no device found with volume id %q: %v", req.VolumeId, err)
+			}
+			return nil, status.Errorf(codes.Internal, "failed to get device details for volume id %q: %v", req.VolumeId, err)
 		}
 
 		mountOptions = []string{"bind"}
@@ -303,8 +304,10 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 
 	device, err := ns.cs.dm.GetDevice(req.VolumeId)
 	if err != nil {
-		klog.Errorf("NodeStageVolume: did not find volume %s", req.VolumeId)
-		return nil, status.Error(codes.InvalidArgument, err.Error())
+		if errors.Is(err, pmdmanager.ErrDeviceNotFound) {
+			return nil, status.Errorf(codes.NotFound, "no device found with volume id %q: %v", req.VolumeId, err)
+		}
+		return nil, status.Errorf(codes.Internal, "failed to get device details for volume id %q: %v", req.VolumeId, err)
 	}
 
 	if err = ns.provisionDevice(device, req.GetVolumeCapability().GetMount().GetFsType()); err != nil {
@@ -350,10 +353,11 @@ func (ns *nodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstag
 	// by spec, we have to return OK if asked volume is not mounted on asked path,
 	// so we look up the current device by volumeID and see is that device
 	// mounted on staging target path
-	_, err := ns.cs.dm.GetDevice(req.VolumeId)
-	if err != nil {
-		klog.Errorf("NodeUnstageVolume: did not find volume %s", req.GetVolumeId())
-		return nil, status.Error(codes.InvalidArgument, err.Error())
+	if _, err := ns.cs.dm.GetDevice(req.VolumeId); err != nil {
+		if errors.Is(err, pmdmanager.ErrDeviceNotFound) {
+			return nil, status.Errorf(codes.NotFound, "no device found with volume id '%s': %s", req.VolumeId, err.Error())
+		}
+		return nil, status.Errorf(codes.Internal, "failed to get device details for volume id '%s': %s", req.VolumeId, err.Error())
 	}
 
 	// Find out device name for mounted path

pkg/pmem-device-manager/pmd-lvm.go

@@ -1,15 +1,14 @@
 package pmdmanager
 
 import (
+	"errors"
 	"fmt"
-	"os"
 	"strconv"
 	"strings"
 	"sync"
 
 	"github.com/intel/pmem-csi/pkg/ndctl"
 	pmemexec "github.com/intel/pmem-csi/pkg/pmem-exec"
-	"github.com/pkg/errors"
 	"k8s.io/klog"
 )
 
@@ -43,7 +42,7 @@ func NewPmemDeviceManagerLVM() (PmemDeviceManager, error) {
 
 	ctx, err := ndctl.NewContext()
 	if err != nil {
-		return nil, fmt.Errorf("Failed to initialize pmem context: %s", err.Error())
+		return nil, err
 	}
 	volumeGroups := []string{}
 	for _, bus := range ctx.GetBuses() {
@@ -61,6 +60,10 @@ func NewPmemDeviceManagerLVM() (PmemDeviceManager, error) {
 	}
 	ctx.Free()
 
+	return NewPmemDeviceManagerLVMForVGs(volumeGroups)
+}
+
+func NewPmemDeviceManagerLVMForVGs(volumeGroups []string) (PmemDeviceManager, error) {
 	devices, err := listDevices(volumeGroups...)
 	if err != nil {
 		return nil, err
@@ -88,7 +91,7 @@ func (lvm *pmemLvm) GetCapacity() (map[string]uint64, error) {
 // nsmode is expected to be either "fsdax" or "sector"
 func (lvm *pmemLvm) CreateDevice(volumeId string, size uint64, nsmode string) error {
 	if nsmode != string(ndctl.FsdaxMode) && nsmode != string(ndctl.SectorMode) {
-		return fmt.Errorf("Unknown nsmode(%v)", nsmode)
+		return fmt.Errorf("unknown namespace mode %q: %w", nsmode, ErrInvalid)
 	}
 	lvmMutex.Lock()
 	defer lvmMutex.Unlock()
@@ -97,9 +100,8 @@ func (lvm *pmemLvm) CreateDevice(volumeId string, size uint64, nsmode string) er
 	// this function is asked to create new devices repeatedly, forcing running out of space.
 	// Avoid device filling with garbage entries by returning error.
 	// Overall, no point having more than one namespace with same volumeId.
-	_, err := lvm.getDevice(volumeId)
-	if err == nil {
-		return fmt.Errorf("CreateDevice: Failed: volume with that name '%s' exists", volumeId)
+	if _, err := lvm.getDevice(volumeId); err == nil {
+		return ErrDeviceExists
 	}
 	vgs, err := getVolumeGroups(lvm.volumeGroups, nsmode)
 	if err != nil {
@@ -131,13 +133,11 @@ func (lvm *pmemLvm) CreateDevice(volumeId string, size uint64, nsmode string) er
 				if err != nil {
 					return err
 				}
-				err = WaitDeviceAppears(device)
-				if err != nil {
+				if err := waitDeviceAppears(device); err != nil {
 					return err
 				}
-				err = ClearDevice(device, false)
-				if err != nil {
-					return err
+				if err := clearDevice(device, false); err != nil {
+					return fmt.Errorf("clear device %q: %v", volumeId, err)
 				}
 
 				lvm.devices[device.VolumeId] = device
@@ -146,18 +146,28 @@ func (lvm *pmemLvm) CreateDevice(volumeId string, size uint64, nsmode string) er
 			}
 		}
 	}
-	return fmt.Errorf("No region is having enough space required(%v)", size)
+	return ErrNotEnoughSpace
 }
 
 func (lvm *pmemLvm) DeleteDevice(volumeId string, flush bool) error {
 	lvmMutex.Lock()
 	defer lvmMutex.Unlock()
 
-	device, err := lvm.getDevice(volumeId)
-	if err != nil {
+	var err error
+	var device *PmemDeviceInfo
+
+	if device, err = lvm.getDevice(volumeId); err != nil {
+		if errors.Is(err, ErrDeviceNotFound) {
+			return nil
+		}
 		return err
 	}
-	if err := ClearDevice(device, flush); err != nil {
+	if err := clearDevice(device, flush); err != nil {
+		if errors.Is(err, ErrDeviceNotFound) {
+			// Remove device from cache
+			delete(lvm.devices, volumeId)
+			return nil
+		}
 		return err
 	}
 
@@ -171,18 +181,6 @@ func (lvm *pmemLvm) DeleteDevice(volumeId string, flush bool) error {
 	return nil
 }
 
-func (lvm *pmemLvm) FlushDeviceData(volumeId string) error {
-	lvmMutex.Lock()
-	defer lvmMutex.Unlock()
-
-	device, err := lvm.getDevice(volumeId)
-	if err != nil {
-		return err
-	}
-
-	return ClearDevice(device, true)
-}
-
 func (lvm *pmemLvm) ListDevices() ([]*PmemDeviceInfo, error) {
 	lvmMutex.Lock()
 	defer lvmMutex.Unlock()
@@ -207,7 +205,7 @@ func (lvm *pmemLvm) getDevice(volumeId string) (*PmemDeviceInfo, error) {
 		return dev, nil
 	}
 
-	return nil, errors.Wrapf(os.ErrNotExist, "no device found with id '%s'", volumeId)
+	return nil, ErrDeviceNotFound
 }
 
 func getUncachedDevice(volumeId string, volumeGroup string) (*PmemDeviceInfo, error) {
@@ -219,27 +217,28 @@ func getUncachedDevice(volumeId string, volumeGroup string) (*PmemDeviceInfo, er
 	if dev, ok := devices[volumeId]; ok {
 		return dev, nil
 	}
-	return nil, errors.Wrapf(os.ErrNotExist, "no device found with id '%s' in group '%s'", volumeId, volumeGroup)
+
+	return nil, ErrDeviceNotFound
 }
 
 // listDevices Lists available logical devices in given volume groups
 func listDevices(volumeGroups ...string) (map[string]*PmemDeviceInfo, error) {
 	args := append(lvsArgs, volumeGroups...)
 	output, err := pmemexec.RunCommand("lvs", args...)
 	if err != nil {
-		return nil, fmt.Errorf("list volumes failed : %s(lvs output: %s)", err.Error(), output)
+		return nil, fmt.Errorf("lvs failure : %v", err)
 	}
-	return parseLVSOuput(output)
+	return parseLVSOutput(output)
 }
 
 func vgName(bus *ndctl.Bus, region *ndctl.Region, nsmode ndctl.NamespaceMode) string {
 	return bus.DeviceName() + region.DeviceName() + string(nsmode)
 }
 
 //lvs options "lv_name,lv_path,lv_size,lv_free"
-func parseLVSOuput(output string) (map[string]*PmemDeviceInfo, error) {
+func parseLVSOutput(output string) (map[string]*PmemDeviceInfo, error) {
 	devices := map[string]*PmemDeviceInfo{}
-	lines := strings.Split(string(output), "\n")
+	lines := strings.Split(output, "\n")
 	for _, line := range lines {
 		fields := strings.Fields(strings.TrimSpace(line))
 		if len(fields) != 3 {
@@ -281,12 +280,12 @@ func getVolumeGroups(groups []string, wantedTag string) ([]vgInfo, error) {
 	args := append(vgsArgs, groups...)
 	output, err := pmemexec.RunCommand("vgs", args...)
 	if err != nil {
-		return vgs, fmt.Errorf("vgs failure: %s", err.Error())
+		return vgs, fmt.Errorf("vgs failure: %v", err)
 	}
 	for _, line := range strings.SplitN(output, "\n", len(groups)) {
 		fields := strings.Fields(strings.TrimSpace(line))
 		if len(fields) != 4 {
-			return vgs, fmt.Errorf("Failed to parse vgs output line: %s", line)
+			return vgs, fmt.Errorf("failed to parse vgs output: %q", line)
 		}
 		tag := fields[3]
 		if tag == wantedTag {