Bug 1376045

Summary: [SDK] Valid response when setting negative overcommit percent on cluster
Product: [oVirt] ovirt-engine Reporter: Gonza <grafuls>
Component: RestAPIAssignee: Martin Sivák <msivak>
Status: CLOSED WONTFIX QA Contact: Pavel Stehlik <pstehlik>
Severity: unspecified Docs Contact:
Priority: low    
Version: ---CC: bugs, dfediuck, juan.hernandez, oourfali
Target Milestone: ---Flags: rule-engine: planning_ack?
rule-engine: devel_ack?
rule-engine: testing_ack?
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2016-09-15 07:47:19 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: SLA RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:

Description Gonza 2016-09-14 14:01:40 UTC
Description of problem:
If trying to update overcommit percent via python SDK with a negative value, cluster is not updated but SDK returns a valid response (code 200).

Version-Release number of selected component (if applicable):
ovirt-engine-sdk-python-4.0.0.0-0.1.20150908.gitceb901a.fc22.noarch

How reproducible:
100%

Steps to Reproduce:
    cluster = api.clusters.get(name="Default")
    mp = cluster.get_memory_policy()
    mp.overcommit.set_percent(-7)
    cluster.set_memory_policy(mp)
    cluster.update()

Actual results:
Response code is not valid, expected is: [400, 409, 500], actual is: 200 

Expected results:
Response code is 400 and validation error message is returned

Additional info:
Issue occurring as well on:
rhevm-sdk-python-3.6.9.1-1.el6ev.noarch

Comment 1 Gonza 2016-09-14 14:25:42 UTC
This occurred while using version 3 of the API

Comment 2 Oved Ourfali 2016-09-15 05:39:38 UTC
Moving to SLA.
If infra's assistance is needed, please reach out.

Comment 3 Roy Golan 2016-09-15 06:15:41 UTC
This is a dup of Bug 1301353 . The suggested patch caused a regression and eventually it was decided to forget this. 

IIRC the very bad piece of code we have today at ClusterOperationCommandBase.java is very loose and allows REST api calls to send 0 or less and fix that immediatly to 200 

```java

    protected void checkMaxMemoryOverCommitValue() {
        if (getCluster().getMaxVdsMemoryOverCommit() <= 0) {
            getCluster().setMaxVdsMemoryOverCommit(
                    Config.<Integer>getValue(ConfigValues.MaxVdsMemOverCommit));
        }
    }
```

The suggested fix removed that and raised a validation error, but didn't address backward compatibility of REST api clients, that simply send 0 today rather than the real value.

Comment 4 Doron Fediuck 2016-09-15 07:47:19 UTC
Traditionally we never validated the provided input other than numeric sanity.
Fixing this may actually break working setups as we've seen before.
Hence we prefer not to fix it as long as there's no urgent reason to do so.