Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[blobfuse2] return code should be non-zero when mount parameter is wrong #735

Closed
andyzhangx opened this issue Mar 13, 2022 · 8 comments
Closed

Comments

@andyzhangx
Copy link
Contributor

Which version of the blobfuse was used?

blobfuse v2 preview

Which OS (please include version) are you using?

Ubuntu 18.04

What problem was encountered?

It seems -o parameters are not supported any more in v2, while it still returns 0 which means mount is successful, and csi driver would regard it as successful mount. return code should be non-zero when mount parameter is wrong

mkdir blob3
blobfuse2 mount blob3 --container-name=public --tmp-path=/tmp/blobfuse/ --config-file=/usr/share/blobfuse2/config.yaml -o allow_other --file-cache-timeout-in-seconds=120 --use-attr-cache=true --cancel-list-on-mount-seconds=10 -o attr_timeout=120 -o entry_timeout=120 -o negative_timeout=120 --log-level=LOG_WARNING --cache-size-mb=1000 -o ro --streaming=true# #
Error: unknown shorthand flag: 'o' in -o
Usage:
  blobfuse2 mount [path] [flags]
  blobfuse2 mount [command]

Available Commands:
  all         Mounts all azure blob container for a given account as a filesystem
  list        List all blobfuse2 mountpoints

Flags:
      --allow-other                Allow other users to access this mount point.
      --attr-cache-timeout int32   attribute cache timeout (default 120)
      --attr-timeout uint32         The attribute timeout in seconds
      --config-file string         Configures the path for the file where the account credentials are provided. Default is config.yaml (default "config.yaml")
      --container-name string      Configures the name of the container to be mounted
      --entry-timeout uint32       The entry timeout in seconds.
      --foreground                 Mount the system in foreground mode. Default value false.
  -h, --help                       help for mount
      --log-file-path string       Configures the path for log files. Default is $HOME/.blobfuse2/blobfuse2.log (default "$HOME/.blobfuse2/blobfuse2.log")
      --log-level string           Enables logs written to syslog. Set to LOG_WARNING by default. Allowed values are LOG_OFF|LOG_CRIT|LOG_ERR|LOG_WARNING|LOG_INFO|LOG_DEBUG (default "LOG_WARNING")
      --negative-timeout uint32    The negative entry timeout in seconds.
      --no-symlinks                whether or not symlinks should be supported
      --passphrase string          Key to decrypt config file. Can also be specified by env-variable BLOBFUSE2_SECURE_CONFIG_PASSPHRASE.
                                   Key length shall be 16 (AES-128), 24 (AES-192), or 32 (AES-256) bytes in length.
      --read-only                  Mount the system in read only mode. Default value false.
      --secure-config              Encrypt auto generated config file for each container
      --tmp-path string            Configures the tmp location for the cache. Configure the fastest disk (SSD or ramdisk) for best performance.

Global Flags:
      --disable-version-check   To disable version check that is performed automatically

Use "blobfuse2 mount [command] --help" for more information about a command.

# echo $?
0

Have you found a mitigation/solution?

By default, blobfuse logs errors to syslog. If this is relevant, is there anything in the syslog that might be helpful?

If relevant, please share your mount command.

@vibhansa-msft
Copy link
Member

-o cli inputs are not supported in v2. If you wish you use v2 with same semantics, in terms of cli, as v1 then you can use "mountv1" option instead.
"blobfuse mountv1 "
this will allow you to use v2 binary with exactly the same cli options that you were using with v1. This is created for easy migration path to v2.

If you wish to use "mount" command of v2 then "--read-only, --attr-timeout, --negative-timeout, --entry-timeout" are the replacements for the same.

@andyzhangx
Copy link
Contributor Author

@vibhansa-msft -o inputs are not the point here, if mount failed due to unsupported parameters, it should return non-zero error code to indicate it failed instead of return as success.

@tasherif-msft tasherif-msft self-assigned this Mar 14, 2022
@tasherif-msft
Copy link
Contributor

Thanks for opening the issue @andyzhangx we'll get that fixed :)

@tasherif-msft
Copy link
Contributor

Hi @andyzhangx this issue appears to be from the command library that we're using - they don't give us the ability to exit on errors. I have opened up an issue against their repo here spf13/cobra#1633. I will keep you posted!

@vibhansa-msft vibhansa-msft changed the title [blobfuse v2] return code should be non-zero when mount parameter is wrong [blobfuse2] return code should be non-zero when mount parameter is wrong Mar 23, 2022
@andyzhangx
Copy link
Contributor Author

andyzhangx commented Apr 6, 2022

Hi @andyzhangx this issue appears to be from the command library that we're using - they don't give us the ability to exit on errors. I have opened up an issue against their repo here spf13/cobra#1633. I will keep you posted!

I think this is a blocking issue, maybe you could switch to flag: Package flag implements command-line flag parsing. here is the example: https://github.com/kubernetes-sigs/blob-csi-driver/blob/0fb2adf84dc16912ee559d37ffdb275e471123f3/pkg/blobplugin/main.go#L39-L64

~/go/src/sigs.k8s.io/blob-csi-driver# _output/amd64/blobplugin -a
flag provided but not defined: -a
Usage of _output/amd64/blobplugin:
  -add_dir_header
        If true, adds the file directory to the header of the log messages
~/go/src/sigs.k8s.io/blob-csi-driver# echo $?
2

@andyzhangx
Copy link
Contributor Author

here is the example I switch from cobra to flag package:
kubernetes-csi/csi-driver-iscsi@8d7ca50

@gapra-msft
Copy link
Member

@andyzhangx it might not be as simple as that example since we parse inputs from flags, config file and env var. So we have some other logic that does this. I will investigate the feasibility of migrating to flags. If not, we may just have to fork the cobra library and set the error handling to ExitOnError manually

@vibhansa-msft
Copy link
Member

Required code changes merged into main branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants