Allow ignoring unreadable files when snapshotting volumes

Snapshots are implemented using `tar` for file system volumes. With this
patch, users can set the `ignoreFailedRead` parameter in the
VolumeSnapshotClass parameter to `true`, which results in the
`--ignore-failed-read` option to be passed to `tar`.

This enhancement is particularly useful for end-to-end testing of
applications that may delete files initially included when the tar
process started and retrieved the file list. By ignoring unreadable
files, the snapshot process can finish successfully.

Signed-off-by: Leonardo Cecchi <[email protected]>

Author: leonardoce

pkg/hostpath/README.md

@@ -21,12 +21,25 @@ $ csc identity plugin-info --endpoint tcp://127.0.0.1:10000
 "csi-hostpath"  "0.1.0"
 ```
 
-#### Create a volume
+#### Create a block volume
 ```
-$ csc controller new --endpoint tcp://127.0.0.1:10000 --cap 1,block CSIVolumeName
+$ csc controller new --endpoint tcp://127.0.0.1:10000 --cap 1,block --req-bytes 1048576 --lim-bytes 1048576 CSIVolumeName
 CSIVolumeID
 ```
 
+#### Create mounted volume
+```
+$ csc controller new --endpoint tcp://127.0.0.1:10000 --cap MULTI_NODE_MULTI_WRITER,mount,xfs,uid=500,gid=500 CSIVolumeName
+CSIVolumeID
+```
+
+#### List volumes
+```
+csc controller list-volumes --endpoint tcp://127.0.0.1:10000
+CSIVolumeID  0
+CSIVolumeID  0
+```
+
 #### Delete a volume
 ```
 $ csc controller del --endpoint tcp://127.0.0.1:10000 CSIVolumeID
@@ -56,3 +69,19 @@ CSIVolumeID
 $ csc node get-info --endpoint tcp://127.0.0.1:10000
 CSINode
 ```
+
+### Create snapshot
+```
+$ csc controller create-snapshot --endpoint tcp://127.0.0.1:10000 --params ignoreFailedRead=true --source-volume CSIVolumeID CSISnapshotName
+CSISnapshotID
+```
+
+### Delete snapshot
+```
+csc controller delete-snapshot --endpoint tcp://127.0.0.1:10000 CSISnapshotID
+```
+
+### List snapshots
+```
+csc controller list-snapshots --endpoint tcp://127.0.0.1:10000
+```

pkg/hostpath/controllerserver.go

@@ -586,8 +586,12 @@ func (hp *hostPath) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotR
 	snapshotID := uuid.NewUUID().String()
 	creationTime := ptypes.TimestampNow()
 	file := hp.getSnapshotPath(snapshotID)
+	opts, err := optionsFromParameters(hostPathVolume, req.Parameters)
+	if err != nil {
+		return nil, status.Errorf(codes.InvalidArgument, "invalid volume snapshot class parameters: %s", err.Error())
+	}
 
-	if err := hp.createSnapshotFromVolume(hostPathVolume, file); err != nil {
+	if err := hp.createSnapshotFromVolume(hostPathVolume, file, opts...); err != nil {
 		return nil, err
 	}
 

pkg/hostpath/groupcontrollerserver.go

@@ -122,10 +122,15 @@ func (hp *hostPath) CreateVolumeGroupSnapshot(ctx context.Context, req *csi.Crea
 			return nil, err
 		}
 
+		opts, err := optionsFromParameters(hostPathVolume, req.Parameters)
+		if err != nil {
+			return nil, status.Errorf(codes.InvalidArgument, "invalid volume group snapshot class parameters: %s", err.Error())
+		}
+
 		snapshotID := uuid.NewUUID().String()
 		file := hp.getSnapshotPath(snapshotID)
 
-		if err := hp.createSnapshotFromVolume(hostPathVolume, file); err != nil {
+		if err := hp.createSnapshotFromVolume(hostPathVolume, file, opts...); err != nil {
 			return nil, err
 		}
 

pkg/hostpath/hostpath.go

@@ -380,17 +380,25 @@ func (hp *hostPath) getAttachCount() int64 {
 	return count
 }
 
-func (hp *hostPath) createSnapshotFromVolume(vol state.Volume, file string) error {
-	var cmd []string
+func (hp *hostPath) createSnapshotFromVolume(vol state.Volume, file string, opts ...string) error {
+	var args []string
+	var cmdName string
 	if vol.VolAccessType == state.BlockAccess {
 		klog.V(4).Infof("Creating snapshot of Raw Block Mode Volume")
-		cmd = []string{"cp", vol.VolPath, file}
+		cmdName = "cp"
+		args = []string{vol.VolPath, file}
 	} else {
-		klog.V(4).Infof("Creating snapshot of Filsystem Mode Volume")
-		cmd = []string{"tar", "czf", file, "-C", vol.VolPath, "."}
+		klog.V(4).Infof("Creating snapshot of Filesystem Mode Volume")
+		cmdName = "tar"
+		opts = append(
+			[]string{"czf", file, "-C", vol.VolPath},
+			opts...,
+		)
+		args = []string{"."}
 	}
 	executor := utilexec.New()
-	out, err := executor.Command(cmd[0], cmd[1:]...).CombinedOutput()
+	optsAndArgs := append(opts, args...)
+	out, err := executor.Command(cmdName, optsAndArgs...).CombinedOutput()
 	if err != nil {
 		return fmt.Errorf("failed create snapshot: %w: %s", err, out)
 	}

pkg/hostpath/options.go

@@ -0,0 +1,52 @@
+/*
+Copyright 2024 The Kubernetes 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 hostpath
+
+import (
+	"fmt"
+	"strconv"
+
+	"github.com/kubernetes-csi/csi-driver-host-path/pkg/state"
+)
+
+// ignoreFailedReadParameterName is a parameter that, when set to true,
+// causes the `--ignore-failed-read` option to be passed to `tar`.
+const ignoreFailedReadParameterName = "ignoreFailedRead"
+
+func optionsFromParameters(vol state.Volume, parameters map[string]string) ([]string, error) {
+	// We do not support options for snapshots of block volumes
+	if vol.VolAccessType == state.BlockAccess {
+		return nil, nil
+	}
+
+	ignoreFailedReadString := parameters[ignoreFailedReadParameterName]
+	if len(ignoreFailedReadString) == 0 {
+		return nil, nil
+	}
+
+	if ok, err := strconv.ParseBool(ignoreFailedReadString); err != nil {
+		return nil, fmt.Errorf(
+			"invalid value for %q, expected boolean but was %q",
+			ignoreFailedReadParameterName,
+			ignoreFailedReadString,
+		)
+	} else if ok {
+		return []string{"--ignore-failed-read"}, nil
+	}
+
+	return nil, nil
+}

pkg/hostpath/options_test.go

@@ -0,0 +1,119 @@
+/*
+Copyright 2024 The Kubernetes 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 hostpath
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/kubernetes-csi/csi-driver-host-path/pkg/state"
+)
+
+func TestOptionsFromParameters(t *testing.T) {
+	mountedVolume := state.Volume{
+		VolAccessType: state.MountAccess,
+	}
+	blockVolume := state.Volume{
+		VolAccessType: state.BlockAccess,
+	}
+
+	cases := []struct {
+		testName string
+		volume   state.Volume
+		params   map[string]string
+		result   []string
+		success  bool
+	}{
+		{
+			testName: "no options passed (mounted)",
+			params:   nil,
+			result:   nil,
+			success:  true,
+			volume:   mountedVolume,
+		},
+		{
+			testName: "no options passed (block)",
+			params:   nil,
+			result:   nil,
+			success:  true,
+			volume:   blockVolume,
+		},
+		{
+			testName: "ignoreFailedRead=false (mounted)",
+			params: map[string]string{
+				ignoreFailedReadParameterName: "false",
+			},
+			result:  nil,
+			success: true,
+			volume:  mountedVolume,
+		},
+		{
+			testName: "ignoreFailedRead=true (mounted)",
+			params: map[string]string{
+				ignoreFailedReadParameterName: "true",
+			},
+			result: []string{
+				"--ignore-failed-read",
+			},
+			success: true,
+			volume:  mountedVolume,
+		},
+		{
+			testName: "invalid ignoreFailedRead (mounted)",
+			params: map[string]string{
+				ignoreFailedReadParameterName: "ABC",
+			},
+			result:  nil,
+			success: false,
+			volume:  mountedVolume,
+		},
+		{
+			testName: "ignoreFailedRead=true (block)",
+			params: map[string]string{
+				ignoreFailedReadParameterName: "true",
+			},
+			result:  nil,
+			success: true,
+			volume:  blockVolume,
+		},
+		{
+			testName: "invalid ignoreFailedRead (block)",
+			params: map[string]string{
+				ignoreFailedReadParameterName: "ABC",
+			},
+			result:  nil,
+			success: true,
+			volume:  blockVolume,
+		},
+	}
+
+	for _, tt := range cases {
+		t.Run(tt.testName, func(t *testing.T) {
+			result, err := optionsFromParameters(tt.volume, tt.params)
+			if tt.success && err != nil {
+				t.Fatalf("expected success, found error: %s", err.Error())
+			}
+			if !tt.success && err == nil {
+				t.Fatalf("expected failure, but succeeded with %q", result)
+			}
+
+			if tt.success && !reflect.DeepEqual(tt.result, result) {
+				t.Fatalf("expected %q but received %q", tt.result, result)
+			}
+		})
+	}
+}

pkg/state/strings.go

@@ -40,7 +40,7 @@ func (s *Strings) Empty() bool {
 	return len(*s) == 0
 }
 
-// Remove removes the first occurence of the string, if present.
+// Remove removes the first occurrence of the string, if present.
 func (s *Strings) Remove(str string) {
 	for i, str2 := range *s {
 		if str == str2 {