Merge pull request #543 from leonardoce/tar-opts

Allow ignoring unreadable files in `tar`-based snapshots

Author: k8s-ci-robot

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

@@ -585,8 +585,12 @@ func (hp *hostPath) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotR
 	snapshotID := uuid.NewUUID().String()
 	creationTime := timestamppb.Now()
 	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

@@ -385,17 +385,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 {