Bug 1376045 - [SDK] Valid response when setting negative overcommit percent on cluster
Summary: [SDK] Valid response when setting negative overcommit percent on cluster
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: ovirt-engine
Classification: oVirt
Component: RestAPI
Version: ---
Hardware: Unspecified
OS: Unspecified
low
unspecified vote
Target Milestone: ---
: ---
Assignee: Martin Sivák
QA Contact: Pavel Stehlik
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-09-14 14:01 UTC by Gonza
Modified: 2016-09-15 07:47 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-09-15 07:47:19 UTC
oVirt Team: SLA
rule-engine: planning_ack?
rule-engine: devel_ack?
rule-engine: testing_ack?


Attachments (Terms of Use)

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.


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