]> git.apps.os.sepia.ceph.com Git - ceph-ci.git/commitdiff
cephfs-shell: get cmd must get both path and should validate them
authordparmar18 <dparmar@redhat.com>
Fri, 8 Apr 2022 08:58:48 +0000 (14:28 +0530)
committerdparmar18 <dparmar@redhat.com>
Wed, 11 May 2022 11:46:28 +0000 (17:16 +0530)
Description:
- While using `get` command, `local_path` parameter is optional. Changing it
  to mandatory.
  - Rationale: Till now, there used to be a default path of `local_path` as
               `default='.'` but wasn't mentioned anywhere. It led to confusion.
               On top of it, considering get command to be a ssh inspired utlity,
               or any other CLI tool that copies file between filesystems, source
               and destination path are always mandatory. Therefore in order to
               simulate this behavior in cephfs-shell`s command(s), my opinion is
               to make get command accept both the paths.

- Added checks to make sure:
1) File does exist at `remote_path`
2) File with the same name doesn't exist in `local_path`
3) Removed code that would run through the directory and if it finds
   nothing in `root_src_dir`, then it will try to do:
   `os.makedirs(root_dst_dir + b'/' + root_src_dir)`, but it will
   never be empty as 1) takes care of it.

Fixes: https://tracker.ceph.com/issues/55216
Signed-off-by: dparmar18 <dparmar@redhat.com>
src/tools/cephfs/cephfs-shell

index 8ab719422dbdd1709d5c3b2b4303e80a069df47c..cc1c6be25d37fd93287ab64e1bb0dcb995c878ef 100755 (executable)
@@ -716,12 +716,11 @@ class CephFSShell(Cmd):
         return self.complete_filenames(text, line, begidx, endidx)
 
     get_parser = argparse.ArgumentParser(
-        description='Copy a file from Ceph File System from Local Directory.')
+        description='Copy a file from Ceph File System to Local Directory.')
     get_parser.add_argument('remote_path', type=str, action=path_to_bytes,
                             help='Path of the file in the remote system')
     get_parser.add_argument('local_path', type=str, action=path_to_bytes,
-                            help='Path of the file in the local system',
-                            nargs='?', default='.')
+                            help='Path of the file in the local system')
     get_parser.add_argument('-f', '--force', action='store_true',
                             help='Overwrites the destination if it already exists.')
 
@@ -730,6 +729,19 @@ class CephFSShell(Cmd):
         """
         Copy a file/directory from CephFS to given path.
         """
+        if not is_file_exists(args.remote_path) and not \
+                is_dir_exists(args.remote_path):
+            set_exit_code_msg(errno.ENOENT, "error: no file/directory"
+                                            " found at specified remote "
+                                            "path")
+            return
+        if (os.path.isfile(args.local_path) or os.path.isdir(
+                args.local_path)) and not args.force:
+            set_exit_code_msg(msg=f"error: file/directory "
+                                  f"{args.local_path.decode('utf-8')}"
+                                  f" already exists, use --force to "
+                                  f"overwrite")
+            return
         root_src_dir = args.remote_path
         root_dst_dir = args.local_path
         fname = root_src_dir.rsplit(b'/', 1)
@@ -748,17 +760,6 @@ class CephFSShell(Cmd):
             copy_to_local(root_src_dir, root_dst_dir)
         else:
             files = list(reversed(sorted(dirwalk(root_src_dir))))
-            if len(files) == 0:
-                try:
-                    os.makedirs(root_dst_dir + b'/' + root_src_dir)
-                except OSError as e:
-                    if args.force:
-                        pass
-                    else:
-                        set_exit_code_msg(e.errno, f"{root_src_dir.decode('utf-8')}: "
-                                          "already exists! use --force to overwrite")
-                        return
-
             for file_ in files:
                 dst_dirpath, dst_file = file_.rsplit(b'/', 1)
                 if dst_dirpath in files:
@@ -771,16 +772,7 @@ class CephFSShell(Cmd):
                     except OSError:
                         pass
                 else:
-                    if not args.force:
-                        try:
-                            os.stat(dst_path)
-                            set_exit_code_msg(errno.EEXIST, f"{file_.decode('utf-8')}: "
-                                              "file already exists! use --force to override")
-                            return
-                        except OSError:
-                            copy_to_local(file_, dst_path)
-                    else:
-                        copy_to_local(file_, dst_path)
+                    copy_to_local(file_, dst_path)
 
         return 0