Bug 1270807 - Adding IP(manually) to new network that was created via rest api and attached to host is not marked as out-of-sync
Adding IP(manually) to new network that was created via rest api and attached...
Status: CLOSED CURRENTRELEASE
Product: ovirt-engine
Classification: oVirt
Component: BLL.Network (Show other bugs)
3.6.0
x86_64 Linux
unspecified Severity high (vote)
: ovirt-3.6.2
: 3.6.2.5
Assigned To: Dan Kenigsberg
Michael Burman
:
Depends On: 1298534
Blocks:
  Show dependency treegraph
 
Reported: 2015-10-12 08:31 EDT by Michael Burman
Modified: 2016-02-18 06:02 EST (History)
8 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-02-18 06:02:09 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: Network
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rule-engine: ovirt‑3.6.z+
ylavi: planning_ack+
rule-engine: devel_ack+
rule-engine: testing_ack+


Attachments (Terms of Use)


External Trackers
Tracker ID Priority Status Summary Last Updated
oVirt gerrit 49589 master MERGED engine: Introudce NetworkAttachmentIpConfigurationValidator 2016-01-03 05:37 EST
oVirt gerrit 49590 master MERGED engine: Integrate NetworkAttachmentIpConfigurationValidator 2016-01-03 05:38 EST
oVirt gerrit 50582 master MERGED webadmin: fix HostSetupNetworkModel ipConfiguration param 2016-01-03 05:36 EST
oVirt gerrit 51232 ovirt-engine-3.6 MERGED webadmin: fix HostSetupNetworkModel ipConfiguration param 2016-01-04 05:16 EST
oVirt gerrit 51233 ovirt-engine-3.6 MERGED engine: Introudce NetworkAttachmentIpConfigurationValidator 2016-01-04 05:17 EST
oVirt gerrit 51234 ovirt-engine-3.6 MERGED engine: Integrate NetworkAttachmentIpConfigurationValidator 2016-01-04 05:18 EST
oVirt gerrit 51236 ovirt-engine-3.6.2 MERGED webadmin: fix HostSetupNetworkModel ipConfiguration param 2016-01-04 06:14 EST
oVirt gerrit 51237 ovirt-engine-3.6.2 MERGED engine: Introudce NetworkAttachmentIpConfigurationValidator 2016-01-04 06:15 EST
oVirt gerrit 51238 ovirt-engine-3.6.2 MERGED engine: Integrate NetworkAttachmentIpConfigurationValidator 2016-01-04 06:16 EST

  None (edit)
Description Michael Burman 2015-10-12 08:31:31 EDT
Description of problem:
Adding IP(manually) to new network that was created via rest api and attached to host is not marked as out-of-sync.

Network that was created via rest api, has no ip parameters and when adding IP to such network(manually) while this network attached to host and refreshing caps, engine is not recognize the difference and not marking this network as out-of-sync as it should.

Version-Release number of selected component (if applicable):
3.6.0-0.18.el6

How reproducible:
100

Steps to Reproduce:
1. Create new network via rest api and attach to host
2. Add static ip to this network manually via the host
3. refresh caps

Actual results:
Network is not marked as out-of-sync

Expected results:
Network should be marked as out-of-sync
Comment 1 Yaniv Lavi 2015-10-29 08:41:19 EDT
In oVirt testing is done on single release by default. Therefore I'm removing the 4.0 flag. If you think this bug must be tested in 4.0 as well, please re-add the flag. Please note we might not have testing resources to handle the 4.0 clone.
Comment 2 Eliraz Levi 2015-11-23 07:49:09 EST
The problem is as followed:
The api.xsd is not demanding from the client to give any of the network_attachment elements.
Specifically it is not demanding the ip_address_assignments element to be filled out.

For example the following rest request considered to be valid according to the api.xsd definitions:

Method: POST
Url: http://localhost:8080/ovirt-engine/api/hosts/<host_id>/setupnetworks
Body:
<action>
    <modified_network_attachments>
            <network_attachment>
                <network id="<network_id>"/>
                <host_nic id="<interface_id>"/>
            </network_attachment>
    </modified_network_attachments>
</action>

As a result, a NetworkAttachment is later being created, but it's IpConfiguration value is null as there is no demand on ip_address_assignments.

The (host)setup network command succeeded, following by a database changes persist of the new networkAttachment, again , with null IpConfiguration value.

This causes problems when trying to calculate the ReportedConfiguration.

================================================================================

The suggested solution will be as followed:

====================Alona's ACK NEEDED========================
1. Add validation to canDoAction() of HostSetupNetwokCommand.
the validation will assert IPConfiguration is not null and valid.
(One like the QoS has.)

2. Add a default IPConfiguration initialization with NONE set as boot protocol only for 
new networkAttachment (already exist one are loaded from the database).

this behavior is required in order to be coherent with the web admin client.
(None is the default boot protocol of a network attachment)

3. The current implementation of ReportedConfiguration calculation assuming that IPConfiguration is not null.
Should it be modified to handle a null NetworkAttachment's IPConfiguration?
maybe throw exception?
====================Alona's ACK NEEDED \=======================


=====================Juan or Ori INFO ACK ======================
4. We should consider modifying the min occurrences requirement of ip_address_assignment's elements as it has no meaning without at least one element initialized.
=====================Juan or Ori INFO ACK \======================
Comment 3 Juan Hernández 2015-11-23 08:21:00 EST
The list type definitions of the RESTAPI don't allow this kind of restriction, as they may be used in multiple places, and in some of those places they may be empty.

In the future we will be able to specify constraints (for the "setupnetworks" operation) to indicate, for example, that lists must have certain number of elements. Meanwhile the validation should go in the code, either in the RESTAPI or in the backend. In addition we should document it, using the Javadoc comments of the "setupnetworks" operation.
Comment 4 Alona Kaplan 2015-11-24 07:48:00 EST
(In reply to Eliraz Levi from comment #2)
> The problem is as followed:
> The api.xsd is not demanding from the client to give any of the
> network_attachment elements.
> Specifically it is not demanding the ip_address_assignments element to be
> filled out.
> 
> For example the following rest request considered to be valid according to
> the api.xsd definitions:
> 
> Method: POST
> Url: http://localhost:8080/ovirt-engine/api/hosts/<host_id>/setupnetworks
> Body:
> <action>
>     <modified_network_attachments>
>             <network_attachment>
>                 <network id="<network_id>"/>
>                 <host_nic id="<interface_id>"/>
>             </network_attachment>
>     </modified_network_attachments>
> </action>
> 
> As a result, a NetworkAttachment is later being created, but it's
> IpConfiguration value is null as there is no demand on
> ip_address_assignments.
> 
> The (host)setup network command succeeded, following by a database changes
> persist of the new networkAttachment, again , with null IpConfiguration
> value.
> 
> This causes problems when trying to calculate the ReportedConfiguration.
> 
> =============================================================================
> ===
> 
> The suggested solution will be as followed:
> 
> ====================Alona's ACK NEEDED========================
> 1. Add validation to canDoAction() of HostSetupNetwokCommand.
> the validation will assert IPConfiguration is not null and valid.
> (One like the QoS has.)
> 
> 2. Add a default IPConfiguration initialization with NONE set as boot
> protocol only for 
> new networkAttachment (already exist one are loaded from the database).
> 

+1

> this behavior is required in order to be coherent with the web admin client.
> (None is the default boot protocol of a network attachment)
> 
> 3. The current implementation of ReportedConfiguration calculation assuming
> that IPConfiguration is not null.
> Should it be modified to handle a null NetworkAttachment's IPConfiguration?
> maybe throw exception?
> ====================Alona's ACK NEEDED \=======================
> 
> 
> =====================Juan or Ori INFO ACK ======================
> 4. We should consider modifying the min occurrences requirement of
> ip_address_assignment's elements as it has no meaning without at least one
> element initialized.
> =====================Juan or Ori INFO ACK \======================
Comment 5 Michael Burman 2016-01-21 05:45:08 EST
Verified on - 3.6.2.6-0.1.el6

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