Skip to content

Commit 1f78614

Browse files
authored
Merge pull request #1268 from uriel-guzman/automated-cherry-pick-of-#1231-upstream-release-1.22
Automated cherry pick of #1231: Fix boolean flags formatting bug
2 parents ebf6e29 + b46b60a commit 1f78614

File tree

6 files changed

+15
-28
lines changed

6 files changed

+15
-28
lines changed

pkg/csi_driver/utils.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,9 @@ var (
9595
managedSidecarRegexGCR = regexp.MustCompile(managedSidecarPatternGCR)
9696
// Regex to detect deprecated flag error messages from gcsfuse. Should match the flags using .MarkDeprecated() in https://github.com/GoogleCloudPlatform/gcsfuse/blob/master/cfg/config.go
9797
deprecatedFlagPatterns = regexp.MustCompile(`Flag .*? has been deprecated`)
98+
// Regex to detect invalid argument error messages from gcsfuse. Should match the flags using InvalidValueError in https://github.com/spf13/pflag/blob/b85eb9e15911a41cd7c05d955503542e9befadf4/errors.go#L116,
99+
// imported by GCSFuse: https://github.com/GoogleCloudPlatform/gcsfuse/blob/fc54ba2287dba1ae4be0888686902f72e2f16f8b/go.mod#L34
100+
invalidArgumentPatterns = regexp.MustCompile(`invalid argument .* for .* flag`)
98101
)
99102

100103
func NewVolumeCapabilityAccessMode(mode csi.VolumeCapability_AccessMode_Mode) *csi.VolumeCapability_AccessMode {
@@ -471,7 +474,7 @@ func extractErrorFromGcsFuseErrorFile(errMsg []byte, err error) (codes.Code, err
471474
errMsgStr := string(errMsg)
472475
code := codes.Internal
473476
if strings.Contains(errMsgStr, "Incorrect Usage") ||
474-
strings.Contains(errMsgStr, "unknown flag") {
477+
strings.Contains(errMsgStr, "unknown flag") || invalidArgumentPatterns.MatchString(errMsgStr) {
475478
code = codes.InvalidArgument
476479
}
477480

pkg/csi_driver/utils_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@ func TestExtractErrorFromGcsFuseErrorFile(t *testing.T) {
6060
errorMessage: []byte("Flag --max-retry-duration has been deprecated, This is currently unused."),
6161
expectedCode: codes.Unavailable,
6262
},
63+
{
64+
name: "invalid argument flag error",
65+
errorMessage: []byte(`Error: invalid argument "trve" for "--file-cache-cache-file-for-range-read" flag: strconv.ParseBool: parsing "trve": invalid syntax`),
66+
expectedCode: codes.InvalidArgument,
67+
},
6368
{
6469
name: "bucket doesn't exist error",
6570
errorMessage: []byte("something went wrong: bucket doesn't exist"),

pkg/sidecar_mounter/sidecar_mounter.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,11 @@ func (m *Mounter) Mount(ctx context.Context, mc *MountConfig) error {
111111

112112
args := []string{}
113113
for k, v := range mc.FlagMap {
114-
args = append(args, "--"+k)
114+
arg := k
115115
if v != "" {
116-
args = append(args, v)
116+
arg = fmt.Sprintf("%s=%s", k, v)
117117
}
118+
args = append(args, fmt.Sprintf("--%s", arg))
118119
}
119120

120121
args = append(args, mc.BucketName)

pkg/sidecar_mounter/sidecar_mounter_config.go

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -96,19 +96,6 @@ var disallowedFlags = map[string]bool{
9696
KernelParamsFileConfigFlag: true,
9797
}
9898

99-
var boolFlags = map[string]bool{
100-
"implicit-dirs": true,
101-
"enable-nonexistent-type-cache": true,
102-
"debug_fuse_errors": true,
103-
"debug_fuse": true,
104-
"debug_fs": true,
105-
"debug_gcs": true,
106-
"debug_http": true,
107-
"debug_invariants": true,
108-
"debug_mutex": true,
109-
"disable-autoconfig": true,
110-
}
111-
11299
// Fetch the following information from a given socket path:
113100
// 1. Pod volume name
114101
// 2. The file descriptor
@@ -307,17 +294,7 @@ func (mc *MountConfig) prepareMountArgs() {
307294
continue
308295
}
309296

310-
switch {
311-
case boolFlags[flag] && value != "":
312-
flag = flag + "=" + value
313-
if value == util.TrueStr || value == util.FalseStr {
314-
value = ""
315-
} else {
316-
invalidArgs = append(invalidArgs, flag)
317-
318-
continue
319-
}
320-
case flag == "app-name":
297+
if flag == util.GCSFuseAppNameArg {
321298
value = GCSFuseAppName + "-" + value
322299
}
323300

pkg/sidecar_mounter/sidecar_mounter_config_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func TestPrepareMountArgs(t *testing.T) {
120120
Options: []string{"uid=100", "gid=200", "debug_gcs", "max-conns-per-host=10", "implicit-dirs=true"},
121121
},
122122
expectedArgs: map[string]string{
123-
"implicit-dirs=true": "",
123+
"implicit-dirs": "true",
124124
"app-name": GCSFuseAppName,
125125
"temp-dir": "test-buffer-dir/temp-dir",
126126
"config-file": "test-config-file",

pkg/util/util.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ const (
6060
StorageServiceErrorStr = "failed to setup storage service"
6161
GCSFuseCsiDriverName = "gcsfuse.csi.storage.gke.io"
6262
GCSFuseNumaNodeArg = "gcs-fuse-numa-node"
63+
GCSFuseAppNameArg = "app-name"
6364
)
6465

6566
var (

0 commit comments

Comments
 (0)