Bug 1443920

+++ This bug was initially created as a clone of Bug #1443126 +++
This bug tracks the concern raised in bug #1443126 that during the validation of the ReduceLuns operations, the version of the domain is checked before its type.
This is a bit misleading, as it may prompt a user to think that upgrading the domain will make it a viable candidate for reducing its luns, which is, of course, wrong.

Original bug:
------------------------------------------------------------------------------
Description of problem:
In REST API, storage domains which are not data block (FC/iSCSI) have the reduceluns action.

Version-Release number of selected component (if applicable):
rhevm-4.1.1.8-0.1.el7.noarch
ovirt-engine-restapi-4.1.1.8-0.1.el7.noarch

How reproducible:
Always

Steps to Reproduce:
1. GET to api/storagedomains

Actual results:
Non block data domains (file/data, export, ISO) have the reduceluns link action:


<storage_domain href= "/ovirt-engine/api/storagedomains/a5c82f5e-6549-473d-9bb6-94b52c834b23" id="a5c82f5e-6549-473d-9bb6-94b52c834b23">
<actions>
.
.
.
<link href= "/ovirt-engine/api/storagedomains/a5c82f5e-6549-473d-9bb6-94b52c834b23/reduceluns" rel="reduceluns"/>
</actions>
<name>nfs_0</name>
.
.
.
<type>nfs</type>



Expected results:
Non block data domains should not have the reduceluns link action

--- Additional comment from Allon Mureinik on 2017-04-19 09:03:37 IDT ---

IMHO, this is too much logic for the REST API. As far as I understand, all the members of the same collection always have the same operations, and its up to the backend to fail operations that don't make sense (e.g., reducing a file storage domain).
E.g., a VM in "up" status still has a "start" action, even though it will definitely fail.

I propose as closing NOTABUG.
Juan - your two cents?

--- Additional comment from Juan Hernández on 2017-04-19 10:25:24 IDT ---

I'd suggest the following:

1. Verify that the operation returns a reasonable error message when invoked for a non block storage domain, and fix if it doesn't.

2. Add a note to the API documentation clearly stating that this operation doesn't make sense for non-block storage domains, and that an error will be returned if invoked for that kind of storage domains.

Other than that I agree with closing the bug.

--- Additional comment from Yaniv Dary on 2017-04-19 11:08:35 IDT ---

Do you get a reasonable error?

--- Additional comment from Elad on 2017-04-20 10:13:43 IDT ---

(In reply to Yaniv Dary from comment #3)
> Do you get a reasonable error?


<fault>
<detail>
[Cannot edit Storage. Storage Domain format V1 is illegal.]
</detail>
<reason>Operation Failed</reason>
</fault>


2017-04-20 10:11:55,097+03 ERROR [org.ovirt.engine.api.restapi.resource.AbstractBackendResource] (default task-16) [] Operation Failed: [Cannot edit Storage. Storage Domain format V1 is illegal.]

--- Additional comment from Juan Hernández on 2017-04-20 10:52:24 IDT ---

In my opinion that message isn't clear enough. It should explicitly say that the operation doesn't make sense for non-block storage domains.

--- Additional comment from Elad on 2017-04-20 11:42:45 IDT ---

(In reply to Juan Hernández from comment #5)
> In my opinion that message isn't clear enough. It should explicitly say that
> the operation doesn't make sense for non-block storage domains.

Agree

--- Additional comment from Allon Mureinik on 2017-04-20 12:09:29 IDT ---

(In reply to Elad from comment #6)
> (In reply to Juan Hernández from comment #5)
> > In my opinion that message isn't clear enough. It should explicitly say that
> > the operation doesn't make sense for non-block storage domains.
> 
> Agree

Indeed.
While this error message is technically correct (V1 domains really don't support reducing LUNs), the main issue here is the domain's storage type (i.e., upgrading the domain to V4 won't resolve the issue, as the error message might suggest).
The problem here is the order of the validations in the backend - the domain's version is checked first, and only then its type (in other words, if you'd try this on a new V4 file data domain, you should get a more appropriate error).

Let's keep this BZ for improving the REST's documentation, and I'll clone it to another BZ for the backend's error message.