Bug 1351576 - Update cluster scheduling policy always set default properties
Summary: Update cluster scheduling policy always set default properties
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: ovirt-engine
Classification: oVirt
Component: RestAPI
Version: 4.0.0
Hardware: x86_64
OS: Linux
high
high
Target Milestone: ovirt-4.0.5
: 4.0.5
Assignee: Yanir Quinn
QA Contact: Artyom
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-06-30 11:26 UTC by Artyom
Modified: 2017-01-18 07:36 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Cause: No capability in rest API to directly set and override scheduling policy properties for a specific cluster via rest API Consequence: Scheduling policy properties could not be set and overridden for a specific cluster via rest API calls. Fix: Custom scheduling policy properties added to cluster in order to support update/add/get operations of these properties and store them correctly at the back end. Result: New capability in rest api is available for getting and setting(overriding defaults) a cluster's custom scheduling policy properties. For example, to update the custom properties of the cluster send a request: [source] ---- PUT /ovirt-engine/api/clusters/123 ---- With a request body: [source,xml] ---- <cluster> <custom_scheduling_policy_properties> <property> <name>HighUtilization</name> <value>70</value> </property> </custom_scheduling_policy_properties> </cluster> ---- NOTE: Update operations using `custom_scheduling_policy_properties` attribute will not update the the properties of the scheduling policy specified by the `scheduling_policy` attribute, they will only be reflected on this specific cluster.
Clone Of:
Environment:
Last Closed: 2017-01-18 07:36:04 UTC
oVirt Team: SLA
Embargoed:
rule-engine: ovirt-4.0.z+
rule-engine: ovirt-4.1+
rule-engine: blocker+
mgoldboi: planning_ack+
dfediuck: devel_ack+
mavital: testing_ack+


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
oVirt gerrit 62840 0 master MERGED Added scheduling properties to cluster 2016-08-30 10:57:34 UTC
oVirt gerrit 62845 0 master MERGED restapi: Added scheduling properties to cluster 2016-09-05 15:03:36 UTC
oVirt gerrit 63003 0 model_4.0 MERGED Added scheduling properties to cluster 2016-08-31 12:47:11 UTC
oVirt gerrit 63380 0 ovirt-engine-4.0 MERGED restapi: Added scheduling properties to cluster 2016-09-06 10:30:28 UTC

Description Artyom 2016-06-30 11:26:03 UTC
Description of problem:
Update cluster scheduling policy always set default properties

Version-Release number of selected component (if applicable):
rhevm-4.0.0.6-0.1.el7ev.noarch

How reproducible:
Always

Steps to Reproduce:
1. Update cluster scheduling policy via REST
<cluster>
  <scheduling_policy id="scheduling_policy_id">
    <properties>
      <property>
        <name>HighUtilization</name>
        <value>70</value>
      </property>
      <property>
        <name>LowUtilization</name>
        <value>15</value>
      </property>
    </properties>
  </scheduling_policy>
</cluster>
2.
3.

Actual results:
Update cluster scheduling policy to desired but with default parameters(in case of power_saving policy HighUtilization: 80, LowUtilization:20)

Expected results:
Update cluster scheduling policy to desired but with user parameters(HighUtilization: 70, LowUtilization:15)

Additional info:

Comment 1 Juan Hernández 2016-06-30 12:12:32 UTC
Since version 3.5 of the engine the correct way to change the scheduling policy of a cluster is to create/edit the policy object using the /schedulingpolicies collection, and then assign the policy to the cluster:
    
  PUT /clusters/{cluster:id}
  <cluster>
    <scheduling_policy id="123"/>
  </cluster>

If you need to change the properties of the policy then you should send a PUT request to the /schedulingpolicies collection:

  PUT /schedulingpolicies/{policy:id}
  <scheduling_policy>
    <properties>
      <property>
        <name>HighUtilization</name>
        <value>70</value>
      </property>
      <property>
        <name>LowUtilization</name>
        <value>15</value>
      </property>
    </properties>
  </scheduling_policy>

The request that you are sending is only supported if doesn't include the "id" and includes the old and deprecated "policy" element:

  PUT /clusters/{cluster:id}
  <cluster>
    <scheduling_policy>
      <policy>power_saving</policy>
      <properties>
        ...
      </properties>
    </scheduling_policy>
  </cluster>

In this case, the system will try to assign a policy that is named "power_saving" (or whatever to provide) and that has compatible properties.

Please don't use this backwards compatibility mechanism, use the /schedulingpolicies collection instead.

Comment 2 Artyom 2016-06-30 12:26:36 UTC
So we receive some inconsistency between REST and UI, because in UI I can change the property of scheduling policy when I change cluster scheduling policy.
Also, if I change property via UI I do not have any possibility to see this change under REST because REST shows only
<scheduling_policyhref="/ovirt-engine/api/schedulingpolicies/5a2b0939-7d46-4b73-a469-e9c2c7fc6a53"id="5a2b0939-7d46-4b73-a469-e9c2c7fc6a53"/>
without any additioal info.
I believe it really strange behaviour, also properties that you can see under 
schedulingpolicies/scheduling_policy_id is just default one and must be applied just in case if user not specified his own values for this properties

Comment 3 Artyom 2016-06-30 12:35:15 UTC
This how looks the same actions under 3.6
PUT:
<cluster>
<scheduling_policy id="5a2b0939-7d46-4b73-a469-e9c2c7fc6a53">
<properties>
<property>
<name>HighUtilization</name>
<value>65</value>
</property>
<property>
<name>LowUtilization</name>
<value>15</value>
</property>        
</properties>
</scheduling_policy>
</cluster>

Response:
<clusterhref="/ovirt-engine/api/clusters/00000002-0002-0002-0002-0000000002ea"id="00000002-0002-0002-0002-0000000002ea">
<actions>
<linkhref="/ovirt-engine/api/clusters/00000002-0002-0002-0002-0000000002ea/resetemulatedmachine"rel="resetemulatedmachine"/>
</actions>
<name>Default</name>
<description>The default server cluster</description>
<linkhref="/ovirt-engine/api/clusters/00000002-0002-0002-0002-0000000002ea/networks"rel="networks"/>
<linkhref="/ovirt-engine/api/clusters/00000002-0002-0002-0002-0000000002ea/permissions"rel="permissions"/>
<linkhref="/ovirt-engine/api/clusters/00000002-0002-0002-0002-0000000002ea/glustervolumes"rel="glustervolumes"/>
<linkhref="/ovirt-engine/api/clusters/00000002-0002-0002-0002-0000000002ea/glusterhooks"rel="glusterhooks"/>
<linkhref="/ovirt-engine/api/clusters/00000002-0002-0002-0002-0000000002ea/affinitygroups"rel="affinitygroups"/>
<linkhref="/ovirt-engine/api/clusters/00000002-0002-0002-0002-0000000002ea/cpuprofiles"rel="cpuprofiles"/>
<data_centerhref="/ovirt-engine/api/datacenters/00000001-0001-0001-0001-0000000003ca"id="00000001-0001-0001-0001-0000000003ca"/>
<memory_policy>
<overcommitpercent="100"/>
<transparent_hugepages>
<enabled>true</enabled>
</transparent_hugepages>
</memory_policy>
<scheduling_policyhref="/ovirt-engine/api/schedulingpolicies/5a2b0939-7d46-4b73-a469-e9c2c7fc6a53"id="5a2b0939-7d46-4b73-a469-e9c2c7fc6a53">
<name>power_saving</name>
<policy>power_saving</policy>
<thresholdslow="15"high="65"duration="120"/>
<properties>
<property>
<name>LowUtilization</name>
<value>15</value>
</property>
<property>
<name>HighUtilization</name>
<value>65</value>
</property>
<property>
<name>CpuOverCommitDurationMinutes</name>
<value>2</value>
</property>
</properties>
</scheduling_policy>
...

So I expect the same thing under 4.0 and I also think it is correct behaviour

Comment 4 Juan Hernández 2016-06-30 12:44:50 UTC
We can't have 1 to 1 correspondence between the API and the UI, in general. In this particular case it was decided that scheduling policies are top level entities, independent of clusters, so the correct way to represent them is to use its own collection. The UI presents this in a simpler way, for humans.

If you change the properties in the UI, then you should be able to see the properties using the API. For example, you will get the cluster like this:

  GET /clusters/123
  <cluster id="123" href="/clusters/123">
    <scheduling_policy id="456" href="/schedulingpolicies/456"/>
    ...
  </cluster>

To see the properties of the scheduling policy you have then to follow the link:

  GET /schedulingpolicies/456
  <scheduling_policy id="456" href="/schedulingpolicies/456">
    <name>mypolicy</name>
    <properties>
      ...
    </properties>
  </scheduling_policies>

If that doesn't work, then it is certainly a bug.

Roy, can you take a look, please?

Comment 5 Artyom 2016-06-30 13:01:17 UTC
I believe this bug can be duplicate of this one https://bugzilla.redhat.com/show_bug.cgi?id=1316456

Also in case if a user will have two clusters that he wants to use the same scheduler policy but with different properties, it can be a problem.

Comment 6 Roy Golan 2016-07-03 08:09:52 UTC
Juan maybe its time, in 4.0, to deprecate the old usage and return an error?

And keep supporting the PUT /schedulingpolicies/ids way

Comment 7 Juan Hernández 2016-07-04 08:05:11 UTC
(In reply to Roy Golan from comment #6)
> Juan maybe its time, in 4.0, to deprecate the old usage and return an error?
> 
> And keep supporting the PUT /schedulingpolicies/ids way

The old usage doesn't work at all in version 4 of the API, it has been completely removed already, we preserved it only in version 3 of the API.

But the problem, if I understand correctly, is that this was never implemented correctly, or at least I always understood it incorrectly. My assumption was that a cluster has a *reference* to a scheduling policy, but it looks like that a cluster actually has two properties: a reference to a *default* scheduling policy, and a set of properties, specific to that cluster, that override the default scheduling policy.

I see two alternatives here:

1. Restore the API capability to set the values of the properties specific to the cluster.

2. Modify the backend (and maybe the GUI) so that clusters only have a reference to the cluster, and not a default and a set of overriding properties.

Number 1 is clear in my opinion. What do you prefer?

Comment 8 Juan Hernández 2016-07-04 08:07:47 UTC
Sorry, hit enter too early.

Option number 1 means restoring the behaviour we had in 3.6, which is clearer for users that are already using this area of the API. But option number 2 is conceptually clearer, so better for the long term. I prefer option number 2.

Comment 9 Roy Golan 2016-07-14 10:52:34 UTC
There is no explicit instance for a cluster. Instead there is a private  cluster_policy_custom_properties per each cluster which is essentially the overrided properties of the policy.

example:

1. create a copy of power_saving policy. A record would be created under cluster_policies with its custom properties

id                | f77382d0-c376-4671-85e2-86de8dce4a80
name              | Copy_of_power_saving
description       | 
is_locked         | f
is_default        | f
custom_properties | {
                  |   "HighUtilization" : "80",
                  |   "LowUtilization" : "20",
                  |   "CpuOverCommitDurationMinutes" : "2"


2. go to a cluster and use that policy, and override the value to 60. Under the cluster table you would see:

cluster_policy_id                | f77382d0-c376-4671-85e2-86de8dce4a80
cluster_policy_custom_properties | {
                                 |   "HighUtilization" : "60",
                                 |   "LowUtilization" : "20",
                                 |   "CpuOverCommitDurationMinutes" : "2"
                                 | }

3. if you change, via the ui the copy_of_power_saving properties under 'Configure', the change won't override the cluster properties.


We can fix it with:

cluster_policy_id = 456
cluster_policy_parent_id =  f77382d0-c376-4671-85e2-86de8dce4a80
cluster_policy_custom_properties = { "HighUtilization" : "60", ...}


And fix REST to work with PUT /cluster/123/policy/456

Comment 10 Juan Hernández 2016-07-14 11:24:30 UTC
I'd prefer if we change the database and the business logic to have just one reference to the policy from the cluster. If that isn't doable, then we should explicitly represent in the API what we actually do. So I'd suggest that we add to the API "cluster" type a new "custom_scheduling_policy_properties" attribute. When retrieving a cluster that doesn't have overridden properties we should get something like this:

  GET /clusters/{cluster:id}
  <cluster id="123" href="/clusters/123">
    <scheduling_policy id="456" href="/schedulingpolicies/456"/>
    <-- No custom properties here, as nothing is overridden. -->
  </cluster>

When the properties have been overridden:

  GET /clusters/{cluster:id}
  <cluster id="123" href="/clusters/123">
    <scheduling_policy id="456" href="/schedulingpolicies/456"/>
    <custom_scheduling_policy_properties/>
      <property>
        <name>HighUtilization</name>
        <value>60</value>
      </property>
      ...
    </custom_scheduling_policy_properties>
  </cluster>

To modify the overridden properties:

  PUT /clusters/{cluster:id}
  <cluster>
    <custom_scheduling_policy_properties/>
      <property>
        <name>HighUtilization</name>
        <value>60</value>
      </property>
      ...
    </custom_scheduling_policy_properties>
  </cluster>

Comment 11 Roy Golan 2016-07-17 08:33:01 UTC
If we will only support policies under cluster we will loose manageability. How will we let user view all various policies and attributes in one place? its similar to instance type in a sense.

Comment 12 Juan Hernández 2016-07-18 07:54:18 UTC
What I am proposing is to have just one reference from cluster to scheduling policy. I think that is clear, and it doesn't reduce flexibility or manageability, as you can still have as many policies as you want, and users can follow the link included in the cluster to find the details of the policy.

If that isn't possible, or we want to maintain the current design of the backend, then we can use the suggested "custom_scheduling_policy_properties" element, so that the current backend behavour is explicitly reflected in the API.

Comment 13 Red Hat Bugzilla Rules Engine 2016-07-27 11:05:44 UTC
This bug report has Keywords: Regression or TestBlocker.
Since no regressions or test blockers are allowed between releases, it is also being identified as a blocker for this release. Please resolve ASAP.

Comment 14 Roy Golan 2016-08-24 09:17:04 UTC
(In reply to Juan Hernández from comment #10)
> I'd prefer if we change the database and the business logic to have just one
> reference to the policy from the cluster. If that isn't doable, then we
> should explicitly represent in the API what we actually do. So I'd suggest
> that we add to the API "cluster" type a new
> "custom_scheduling_policy_properties" attribute. When retrieving a cluster
> that doesn't have overridden properties we should get something like this:
> 
>   GET /clusters/{cluster:id}
>   <cluster id="123" href="/clusters/123">
>     <scheduling_policy id="456" href="/schedulingpolicies/456"/>
>     <-- No custom properties here, as nothing is overridden. -->
>   </cluster>
> 
> When the properties have been overridden:
> 
>   GET /clusters/{cluster:id}
>   <cluster id="123" href="/clusters/123">
>     <scheduling_policy id="456" href="/schedulingpolicies/456"/>
>     <custom_scheduling_policy_properties/>
>       <property>
>         <name>HighUtilization</name>
>         <value>60</value>
>       </property>
>       ...
>     </custom_scheduling_policy_properties>
>   </cluster>
> 
> To modify the overridden properties:
> 
>   PUT /clusters/{cluster:id}
>   <cluster>
>     <custom_scheduling_policy_properties/>
>       <property>
>         <name>HighUtilization</name>
>         <value>60</value>
>       </property>
>       ...
>     </custom_scheduling_policy_properties>
>   </cluster>


Juan I am going with the proposal to set cluster scheduling policy like you noted in comment 10 so ignore comment 11 its not adding real value.

Comment 15 Artyom 2016-10-05 14:04:15 UTC
Verified on rhevm-4.0.5-0.1.el7ev.noarch

PUT action to cluster:
<cluster>
   <custom_scheduling_policy_properties>
     <property>
       <name>HighUtilization</name>
       <value>70</value>
     </property>
   </custom_scheduling_policy_properties>
 </cluster>

update cluster scheduling properties


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