Bug 1319964 - "--overwrite" for oc volume is so mistakable
Summary: "--overwrite" for oc volume is so mistakable
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: OpenShift Container Platform
Classification: Red Hat
Component: oc
Version: 3.1.0
Hardware: Unspecified
OS: Unspecified
medium
low
Target Milestone: ---
: ---
Assignee: Juan Vallejo
QA Contact: Xingxing Xia
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-03-22 00:51 UTC by Kenjiro Nakayama
Modified: 2019-10-10 11:38 UTC (History)
9 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-04-12 19:05:12 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2017:0884 0 normal SHIPPED_LIVE Red Hat OpenShift Container Platform 3.5 RPM Release Advisory 2017-04-12 22:50:07 UTC

Description Kenjiro Nakayama 2016-03-22 00:51:04 UTC
Description of problem:

It is not a "bug" precisely, however the option "--overwrite" for oc volume is mistakable. (I will explain in reproduce section below.)

Version-Release number of selected component (if applicable):

- 3.1.1.6

Steps to Reproduce:

We use "--overwrite" to expect we update "test" volume to "pvc" volume, like:

~~~
$ oc volume dc/docker-registry --add --overwrite --name=test --type=emptyDir
$ oc volume dc/docker-registry --add --overwrite --name=pvc1 --type=persistentVolumeClaim --claim-name=pvc1
~~~

and try to use "pvc1". But actual result is still using "test" volume like:

~~~
        volumeMounts:
        - mountPath: /registry
          name: test
      ...
      volumes:
      - emptyDir: {}
        name: test
      - name: pvc1
        persistentVolumeClaim:
          claimName: pvc1
~~~

Expected results:

a. Prohibit --overwrite option from using for non-exsistent volume.
- or -
b. Overwrite the "test" volume by "pvc1".

Additional info:
- proposal patch: https://github.com/openshift/origin/pull/8094

Comment 1 Juan Vallejo 2017-02-01 21:15:30 UTC
It looks like we just want to make sure a user understands that by using the `--overwrite` option, they are not replacing the current volume that is being used. Maybe a simple clarification of this in the flag's description would be enough?

The current description for the "--overwrite" flag is:

```
cmd.Flags().BoolVar(&addOpts.Overwrite, "overwrite", false, "If true, replace existing volume source and/or volume mount for the given resource")
```

We could update it to:

```
cmd.Flags().BoolVar(&addOpts.Overwrite, "overwrite", false, "If true, replace existing volume source and/or volume mount of the same name for the given resource")
```

Comment 2 Kenjiro Nakayama 2017-02-02 06:16:42 UTC
You don't think that following results are more natural behavior?

  a. Prohibit --overwrite option from using for non-existent volume.
   or
  b. Overwrite the "test" volume by "pvc1".

Also, if users made a typo in volumename("--name=xxx"), they don't notice that their pod keeps using wrong(previous) volume. How do you think about it?

Comment 3 Juan Vallejo 2017-02-02 19:11:43 UTC
> You don't think that following results are more natural behavior?

>   b. Overwrite the "test" volume by "pvc1".

Although this could be interpreted as more natural based on the current description of the --override flag, it would not be a good idea to alter
the existing behavior of it; it would impact scriptability and not be a
usability improvement for users who intend to use it as it works today.

> a. Prohibit --overwrite option from using for non-existent volume.

Based on @ccoleman's comment here https://github.com/openshift/origin/pull/8094#r56688748, a negative error caused by attempting to use this flag with a volume that does not already exist might not be good in this case, as it could break existing scripts that depend on this command to return "0", for example. Maybe appending a warning, or info, to the output of a human-readable output format could work.

Comment 4 Kenjiro Nakayama 2017-02-03 02:24:03 UTC
> Maybe appending a warning, or info, to the output of a human-readable output format could work.

I see. I think that it is a good compromise. Thank you.

Comment 5 Juan Vallejo 2017-02-06 21:18:38 UTC
Related PR: https://github.com/openshift/origin/pull/12838

Comment 6 Fabiano Franz 2017-02-20 03:53:06 UTC
Fixed in https://github.com/openshift/origin/pull/12838.

Comment 7 Troy Dawson 2017-02-21 22:36:03 UTC
This has been merged into ocp and is in OCP v3.5.0.32 or newer.

Comment 9 Yanping Zhang 2017-02-23 09:48:27 UTC
Checked on oc v3.5.0.32-1+4f84c83

$ oc volume -h
……
      --overwrite=false: If true, replace existing volume source with the provided name and/or volume mount for the given resource
……
$ oc set volume -h
……
      --overwrite=false: If true, replace existing volume source with the provided name and/or volume mount for the given resource
……

Now the help info for flag --overwrite has been updated as above. The issue is fixed, so move it to Verified.

Comment 11 errata-xmlrpc 2017-04-12 19:05:12 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2017:0884


Note You need to log in before you can comment on or make changes to this bug.