Merge pull request #1054 from pohly/sector-mode

support traditional file IO

Dockerfile

@@ -157,8 +157,7 @@ RUN set -x && \
     mkdir -p /usr/local/bin && \
     mv _output/pmem-csi-driver${BIN_SUFFIX} /usr/local/bin/pmem-csi-driver && \
     mv _output/pmem-csi-operator${BIN_SUFFIX} /usr/local/bin/pmem-csi-operator && \
-    if [ "$BIN_SUFFIX" = "-test" ]; then GOOS=linux GO111MODULE=on \
-        go build -o /usr/local/bin/pmem-dax-check ./test/cmd/pmem-dax-check; fi && \
+    go build -o /usr/local/bin/pmem-dax-check ./test/cmd/pmem-dax-check && \
     mkdir -p /usr/local/share/package-licenses && \
     hack/copy-modules-license.sh /usr/local/share/package-licenses ./cmd/pmem-csi-driver ./cmd/pmem-csi-operator && \
     cp /go/LICENSE /usr/local/share/package-licenses/go.LICENSE && \

Jenkinsfile

@@ -515,9 +515,9 @@ void TestInVM(worker, distro, distroVersion, kubernetesVersion, skipIfPR) {
            "
     } } finally {
         echo "Writing cluster state and kubelet logs into files."
-        sh "_work/${env.CLUSTER}/ssh.0 kubectl get nodes -o wide > joblog-${BUILD_TAG}-nodestate-${kubernetesVersion}.log"
-        sh "_work/${env.CLUSTER}/ssh.0 kubectl get pods --all-namespaces -o wide > joblog-${BUILD_TAG}-podstate-${kubernetesVersion}.log"
-        sh "for cmd in `ls _work/${env.CLUSTER}/ssh.*`; do \$cmd sudo journalctl -u kubelet >> joblog-${BUILD_TAG}-kubeletlogs-${kubernetesVersion}.log; done"
+        sh "_work/${env.CLUSTER}/ssh.0 kubectl get nodes -o wide > joblog-${BUILD_TAG}-${kubernetesVersion}-nodestate.log"
+        sh "_work/${env.CLUSTER}/ssh.0 kubectl get pods --all-namespaces -o wide > joblog-${BUILD_TAG}-${kubernetesVersion}-podstate.log"
+        sh "for cmd in `ls _work/${env.CLUSTER}/ssh.*`; do suffix=`basename \$cmd | sed -e s/^ssh.//`; \$cmd sudo journalctl -u kubelet > joblog-${BUILD_TAG}-${kubernetesVersion}-kubelet.\${suffix}.log; \$cmd sudo journalctl > joblog-${BUILD_TAG}-${kubernetesVersion}-journal-\${suffix}.log; done"
         // Each test run produces junit_*.xml files with testsuite name="PMEM E2E suite".
         // To make test names unique in the Jenkins UI, we rename that test suite per run,
         // mangle the <testcase name="..." classname="..."> such that

Makefile

@@ -186,6 +186,8 @@ KUSTOMIZE += $(subst kubernetes-base,kubernetes-1.21,$(subst X.XX,1.22,$(KUSTOMI
 KUSTOMIZE += deploy/common/pmem-storageclass-default.yaml=deploy/kustomize/storageclass
 KUSTOMIZE += deploy/common/pmem-storageclass-ext4.yaml=deploy/kustomize/storageclass-ext4
 KUSTOMIZE += deploy/common/pmem-storageclass-xfs.yaml=deploy/kustomize/storageclass-xfs
+KUSTOMIZE += deploy/common/pmem-storageclass-ext4-fileio.yaml=deploy/kustomize/storageclass-ext4-fileio
+KUSTOMIZE += deploy/common/pmem-storageclass-xfs-fileio.yaml=deploy/kustomize/storageclass-xfs-fileio
 KUSTOMIZE += deploy/common/pmem-storageclass-late-binding.yaml=deploy/kustomize/storageclass-late-binding
 KUSTOMIZE += deploy/operator/pmem-csi-operator.yaml=deploy/kustomize/operator
 

deploy/common/pmem-storageclass-ext4-fileio.yaml

@@ -0,0 +1,11 @@
+# Generated with "make kustomize", do not edit!
+
+apiVersion: storage.k8s.io/v1
+kind: StorageClass
+metadata:
+  name: pmem-csi-sc-ext4-fileio
+parameters:
+  csi.storage.k8s.io/fstype: ext4
+  eraseafter: "true"
+  usage: FileIO
+provisioner: pmem-csi.intel.com

deploy/common/pmem-storageclass-xfs-fileio.yaml

@@ -0,0 +1,11 @@
+# Generated with "make kustomize", do not edit!
+
+apiVersion: storage.k8s.io/v1
+kind: StorageClass
+metadata:
+  name: pmem-csi-sc-xfs-fileio
+parameters:
+  csi.storage.k8s.io/fstype: xfs
+  eraseafter: "false"
+  usage: FileIO
+provisioner: pmem-csi.intel.com

deploy/kustomize/patches/usage-fileio.yaml

@@ -0,0 +1,6 @@
+apiVersion: storage.k8s.io/v1
+kind: StorageClass
+metadata:
+  name: pmem-csi-sc
+parameters:
+  usage: FileIO

deploy/kustomize/storageclass-ext4-fileio/kustomization.yaml

@@ -0,0 +1,7 @@
+nameSuffix: -fileio
+
+bases:
+- ../storageclass-ext4
+
+patchesStrategicMerge:
+- ../patches/usage-fileio.yaml

deploy/kustomize/storageclass-xfs-fileio/kustomization.yaml

@@ -0,0 +1,7 @@
+nameSuffix: -fileio
+
+bases:
+- ../storageclass-xfs
+
+patchesStrategicMerge:
+- ../patches/usage-fileio.yaml

docs/install.md

@@ -576,7 +576,30 @@ following custom parameters in a storage class:
 |---|-------|--------|-------------|
 |`eraseAfter`|Clear all data by overwriting with zeroes after use and before deleting the volume|Yes|`true` (default), `false`|
 |`kataContainers`|Prepare volume for use with DAX in Kata Containers.|Yes|`false/0/f/FALSE` (default), `true/1/t/TRUE`|
-
+|`usage`|Determine how a volume is going to be used.|Yes|`AppDirect` (default), `FileIO`|
+
+By default, volumes are created for AppDirect enabled applications:
+- The [namespace
+  mode](https://docs.pmem.io/ndctl-user-guide/concepts/nvdimm-namespaces) is
+  `fsdax`.
+- Mount parameters include `-o dax` (= [`-o dax=always` on newer kernels](https://www.kernel.org/doc/Documentation/filesystems/dax.txt))
+  which ensures that all files are automatically opened in DAX mode, i.e.
+  reads and writes directly access the underlying PMEM.
+
+This might not be ideal for traditional file IO because the page cache is
+bypassed, which may affect performance, and because applications have to be
+prepared to deal with partially written data sectors in case of crashes. When
+the goal is to run traditional applications, then `usage=FileIO` may be better:
+- In direct mode, the [namespace
+  mode](https://docs.pmem.io/ndctl-user-guide/concepts/nvdimm-namespaces) is
+  `sector`.
+- In LVM mode, the namespace mode is `fsdax` because currently
+  PMEM-CSI doesn't support LVM on top of other namespaces.
+- Mount parameters do not include `-o dax`.
+
+`kataContainers` and `usage=FileIO` are mutually exclusive because the former
+is about making AppDirect available in Kata Containers. The normal volume
+passthrough can be used for `usage=FileIO`.
 
 ### Creating volumes
 

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

@@ -272,7 +272,7 @@ func (cs *nodeControllerServer) createVolumeInternal(ctx context.Context,
 			}
 		}()
 	}
-	actualSize, err := cs.dm.CreateDevice(ctx, volumeID, uint64(asked))
+	actualSize, err := cs.dm.CreateDevice(ctx, volumeID, uint64(asked), p.GetUsage())
 	if err != nil {
 		code := codes.Internal
 		if errors.Is(err, pmemerr.NotEnoughSpace) {

pkg/pmem-csi-driver/nodeserver.go

@@ -183,7 +183,9 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 			return nil, err
 		}
 		srcPath = device.Path
-		mountFlags = append(mountFlags, daxMountFlag)
+		if v.GetUsage() == parameters.UsageAppDirect {
+			mountFlags = append(mountFlags, daxMountFlag)
+		}
 	} else {
 		// Validate parameters.
 		v, err := parameters.Parse(parameters.PersistentVolumeOrigin, req.GetVolumeContext())
@@ -527,6 +529,11 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 		requestedFsType = defaultFilesystem
 	}
 
+	v, err := parameters.Parse(parameters.PersistentVolumeOrigin, req.GetVolumeContext())
+	if err != nil {
+		return nil, status.Error(codes.InvalidArgument, "persistent volume context: "+err.Error())
+	}
+
 	// Serialize by VolumeId
 	volumeMutex.LockKey(req.GetVolumeId())
 	defer func() {
@@ -572,7 +579,9 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 		}
 	}
 
-	mountOptions = append(mountOptions, daxMountFlag)
+	if v.GetUsage() == parameters.UsageAppDirect {
+		mountOptions = append(mountOptions, daxMountFlag)
+	}
 
 	if err = ns.mount(ctx, device.Path, stagingtargetPath, mountOptions, false /* raw block */); err != nil {
 		return nil, status.Error(codes.Internal, err.Error())

pkg/pmem-csi-driver/parameters/parameters.go

@@ -17,6 +17,7 @@ import (
 
 type Persistency string
 type Origin int
+type Usage string
 
 // Beware of API and backwards-compatibility breaking when changing these string constants!
 const (
@@ -27,6 +28,11 @@ const (
 	Size             = "size"
 	DeviceMode       = "deviceMode"
 
+	// Added in PMEM-CSI 1.1.0.
+	UsageModel           = "usage"
+	UsageAppDirect Usage = "AppDirect"
+	UsageFileIO    Usage = "FileIO"
+
 	// Kubernetes v1.16+ adds this key to NodePublishRequest.VolumeContext
 	// while provisioning ephemeral volume.
 	Ephemeral = "csi.storage.k8s.io/ephemeral"
@@ -58,13 +64,15 @@ var valid = map[Origin][]string{
 	CreateVolumeOrigin: []string{
 		EraseAfter,
 		KataContainers,
+		UsageModel,
 		PersistencyModel,
 	},
 
 	// Parameters from Kubernetes and users.
 	EphemeralVolumeOrigin: []string{
 		EraseAfter,
 		KataContainers,
+		UsageModel,
 		PodInfoPrefix,
 		Size,
 	},
@@ -78,6 +86,7 @@ var valid = map[Origin][]string{
 		EraseAfter,
 		KataContainers,
 		PersistencyModel,
+		UsageModel,
 
 		Name,
 		PodInfoPrefix,
@@ -89,6 +98,7 @@ var valid = map[Origin][]string{
 	NodeVolumeOrigin: []string{
 		EraseAfter,
 		KataContainers,
+		UsageModel,
 		Name,
 		PersistencyModel,
 		Size,
@@ -107,6 +117,7 @@ type Volume struct {
 	Persistency    *Persistency
 	Size           *int64
 	DeviceMode     *api.DeviceMode
+	Usage          *Usage
 }
 
 // VolumeContext represents the same settings as a string map.
@@ -160,6 +171,15 @@ func Parse(origin Origin, stringmap map[string]string) (Volume, error) {
 				return result, fmt.Errorf("parameter %q: failed to parse %q as boolean: %v", key, value, err)
 			}
 			result.KataContainers = &b
+		case UsageModel:
+			u := Usage(value)
+			switch u {
+			case UsageAppDirect, UsageFileIO:
+				result.Usage = &u
+			case "":
+			default:
+				return result, fmt.Errorf("parameter %q: unknown value: %s", key, value)
+			}
 		case Size:
 			quantity, err := resource.ParseQuantity(value)
 			if err != nil {
@@ -201,6 +221,10 @@ func Parse(origin Origin, stringmap map[string]string) (Volume, error) {
 		return result, fmt.Errorf("required parameter %q not specified", Size)
 	}
 
+	if result.GetKataContainers() && result.GetUsage() != UsageAppDirect {
+		return result, fmt.Errorf("Kata Container support and usage %q are mutually exclusive", result.GetUsage())
+	}
+
 	return result, nil
 }
 
@@ -235,6 +259,9 @@ func (v Volume) ToContext() VolumeContext {
 	if v.DeviceMode != nil {
 		result[DeviceMode] = string(*v.DeviceMode)
 	}
+	if v.Usage != nil {
+		result[UsageModel] = string(*v.Usage)
+	}
 
 	return result
 }
@@ -281,3 +308,10 @@ func (v Volume) GetDeviceMode() api.DeviceMode {
 
 	return ""
 }
+
+func (v Volume) GetUsage() Usage {
+	if v.Usage != nil {
+		return *v.Usage
+	}
+	return UsageAppDirect
+}

pkg/pmem-csi-driver/parameters/parameters_test.go

@@ -21,6 +21,8 @@ func TestParameters(t *testing.T) {
 	normal := PersistencyNormal
 	gig := "1Gi"
 	gigNum := int64(1 * 1024 * 1024 * 1024)
+	appDirect := UsageAppDirect
+	fileIO := UsageFileIO
 
 	tests := []struct {
 		name       string
@@ -69,6 +71,45 @@ func TestParameters(t *testing.T) {
 			err: "parameter \"foo\" invalid in this context",
 		},
 
+		// Usage values.
+		{
+			name:   "invalid-usage",
+			origin: CreateVolumeOrigin,
+			stringmap: VolumeContext{
+				UsageModel: "Foo",
+			},
+			err: "parameter \"usage\": unknown value: Foo",
+		},
+		{
+			name:   "invalid-kata-containers",
+			origin: CreateVolumeOrigin,
+			stringmap: VolumeContext{
+				UsageModel:     "FileIO",
+				KataContainers: "true",
+			},
+			err: "Kata Container support and usage \"FileIO\" are mutually exclusive",
+		},
+		{
+			name:   "valid-usage-app-direct",
+			origin: CreateVolumeOrigin,
+			stringmap: VolumeContext{
+				UsageModel: "AppDirect",
+			},
+			parameters: Volume{
+				Usage: &appDirect,
+			},
+		},
+		{
+			name:   "valid-usage-file-io",
+			origin: CreateVolumeOrigin,
+			stringmap: VolumeContext{
+				UsageModel: "FileIO",
+			},
+			parameters: Volume{
+				Usage: &fileIO,
+			},
+		},
+
 		// Parse errors for size.
 		{
 			name:   "invalid-size-suffix",

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

@@ -12,6 +12,7 @@ import (
 	"sync"
 
 	api "github.com/intel/pmem-csi/pkg/apis/pmemcsi/v1beta1"
+	"github.com/intel/pmem-csi/pkg/pmem-csi-driver/parameters"
 
 	pmemerr "github.com/intel/pmem-csi/pkg/errors"
 )
@@ -66,7 +67,7 @@ func (dm *fakeDM) getCapacity() Capacity {
 	}
 }
 
-func (dm *fakeDM) CreateDevice(ctx context.Context, volumeId string, size uint64) (uint64, error) {
+func (dm *fakeDM) CreateDevice(ctx context.Context, volumeId string, size uint64, usage parameters.Usage) (uint64, error) {
 	dm.mutex.Lock()
 	defer dm.mutex.Unlock()
 

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

@@ -14,6 +14,7 @@ import (
 	pmemlog "github.com/intel/pmem-csi/pkg/logger"
 	"github.com/intel/pmem-csi/pkg/ndctl"
 	pmemcommon "github.com/intel/pmem-csi/pkg/pmem-common"
+	"github.com/intel/pmem-csi/pkg/pmem-csi-driver/parameters"
 )
 
 const (
@@ -132,7 +133,7 @@ func (lvm *pmemLvm) GetCapacity(ctx context.Context) (capacity Capacity, err err
 	return capacity, nil
 }
 
-func (lvm *pmemLvm) CreateDevice(ctx context.Context, volumeId string, size uint64) (uint64, error) {
+func (lvm *pmemLvm) CreateDevice(ctx context.Context, volumeId string, size uint64, usage parameters.Usage) (uint64, error) {
 	ctx, logger := pmemlog.WithName(ctx, "LVM-CreateDevice")
 
 	lvmMutex.Lock()

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

@@ -6,6 +6,8 @@ import (
 
 	api "github.com/intel/pmem-csi/pkg/apis/pmemcsi/v1beta1"
 	"k8s.io/apimachinery/pkg/api/resource"
+
+	"github.com/intel/pmem-csi/pkg/pmem-csi-driver/parameters"
 )
 
 const (
@@ -79,7 +81,7 @@ type PmemDeviceManager interface {
 	// CreateDevice creates a new block device with give name, size and namespace mode.
 	// It returns the actual volume size which will always be at least as large as requested.
 	// Possible errors: ErrNotEnoughSpace, ErrDeviceExists
-	CreateDevice(ctx context.Context, name string, size uint64) (uint64, error)
+	CreateDevice(ctx context.Context, name string, size uint64, usage parameters.Usage) (uint64, error)
 
 	// GetDevice returns the block device information for given name
 	// Possible errors: ErrDeviceNotFound