quantum subnet-update can't update allocation-pool

[Mathieu Rohon]

the upadte of the allocation-pool doesn't work :

# quantum subnet-create --allocation-pool start=10.0.13.200,end=10.0.13.254 net3 10.0.13.0/24
Created a new subnet:
+------------------+------------------------------------------------+
| Field | Value |
+------------------+------------------------------------------------+
| allocation_pools | {"start": "10.0.13.200", "end": "10.0.13.254"} |
| cidr | 10.0.13.0/24 |
| dns_nameservers | |
| enable_dhcp | True |
| gateway_ip | 10.0.13.1 |
| host_routes | |
| id | cb4f7e7e-3152-4c5b-a620-9b9bd5b75ac3 |
| ip_version | 4 |
| name | |
| network_id | d76bff4a-dff1-436c-845b-b72c9bce8a96 |
| tenant_id | 57ffb85101824a73ae4872ab0c6780cf |
+------------------+------------------------------------------------+

# quantum subnet-update cb4f7e7e-3152-4c5b-a620-9b9bd5b75ac3 --allocation-pool start=10.0.13.210,end=10.0.13.254
Updated subnet: cb4f7e7e-3152-4c5b-a620-9b9bd5b75ac3

# quantum subnet-show cb4f7e7e-3152-4c5b-a620-9b9bd5b75ac3
+------------------+------------------------------------------------+
| Field | Value |
+------------------+------------------------------------------------+
| allocation_pools | {"start": "10.0.13.200", "end": "10.0.13.254"} |
| cidr | 10.0.13.0/24 |
| dns_nameservers | |
| enable_dhcp | True |
| gateway_ip | 10.0.13.1 |
| host_routes | |
| id | cb4f7e7e-3152-4c5b-a620-9b9bd5b75ac3 |
| ip_version | 4 |
| name | |
| network_id | d76bff4a-dff1-436c-845b-b72c9bce8a96 |
| tenant_id | 57ffb85101824a73ae4872ab0c6780cf |
+------------------+------------------------------------------------+

[Mathieu Rohon (mathieu-rohon)]

I have tested it on Folsom.
It seems to be the same on Grizzly with devstack

[dan wendlandt (danwent)]

Hi Mathieu,

Just an FYI, you only need to add the tag 'folsom-backport-potential', which indicates that the problem exists in folsom, and thus this bug might be backported. The tag 'in-stable-folsom' actually means that the change has been backported and merged into folsom/stable, which it has not.

[Mathieu Rohon (mathieu-rohon)]

ho sorry,

I've been too fast in the copy/paste ;-)

thanks

[dan wendlandt (danwent)]

so I wrote a patch to fix this issue, but it wasn't until I started running the unit tests that I realized that the command in the bug request is actually wrong. It should be allocation_pools (with an "s").

When you run that, you get an error from Quantum saying that allocation_pools is read-only and can't be updated.

quantum subnet-update foo --allocation-pools start=10.1.0.10,end=10.1.0.254
Cannot update read-only attribute allocation_pools

This is actually already documented here: http://docs.openstack.org/api/openstack-network/2.0/content/Concepts-d1e369.html

I have the code to make allocation pools writable similar to what we already do for host_routes or dns_nameservers, but its more of a spec question whether we would want to make that change.

[dan wendlandt (danwent)]

Note: what made this so tricky to detect was that we still dont' provide good errors if a key is passed in that is completely ignored. this is a major usability (unusability?) issue for CLI and API users.

[Mathieu Rohon (mathieu-rohon)]

HI,

another issue come from python-quantumclient library.
the python-quantumclient requires "allocation-pool" (without "s") as argument for the creation :
https://github.com/openstack/python-quantumclient/blob/master/quantumclient/quantum/v2_0/subnet.py

I'll post it in the python-quantumclient bug list.

[Mathieu Rohon (mathieu-rohon)]

On folsom, even with the "s", the update is still accepted :

# quantum subnet-update cb4f7e7e-3152-4c5b-a620-9b9bd5b75ac3 --allocation-pools start=10.0.13.210,end=10.0.13.254
Updated subnet: cb4f7e7e-3152-4c5b-a620-9b9bd5b75ac3

May be we should split this bug in two bugs :
1 : folsom bug on subnet-update --allocation-pools with a grizzly backport
2 : this bug about errors in keys

[Salvatore Orlando (salvatore-orlando)]

Actually quantum subnet-update will accept anything. Even allocation-poops. This is because it's been implemented in a way that it's extension friendly.

On the server side we have a long standing issue that it never complains if you send an unknown attribute. When you do
--allocation-pools I am not sure this is going to be translated into allocation_pools (dash replaced by underscore) in the api request which is sent. In that case, the server will just ignore 'allocation-pools'.

So the update is accepted, but it's actually not doing anything (indeed data are not updated).

For the quantum subnet-create operation instead I believe --allocation-pool is used in singular form because it can be repeated multiple times for distinct allocation pools. The issue here is that actually we should have the same sets of options for create and update operations.

Summarizing:
* I will triage for checking whether there are issues concerning specifically subnet_update operation on the API server. If not I will mark this bug as invalid
* I will file a quantum bug for forcing the server to return 400 for unknown attributes.
* I will file a python-quantumclient bug for allowing to specify the same options as create for update operations.

I think we need to do these improvements as this is not the first time this confusion occurs,

[dan wendlandt (danwent)]

Yeah, that is my same conclusion. Perhaps we give people a flag where they can turn off the unknown attributes checking if there are some half-assed extensions that aren't properly declared, but now that extensions actually add attributes to the attributes dictionary, adding this check should be trivial.

[dan wendlandt (danwent)]

btw, i'm happy to make that change, and will file an issue for enforcing known attributes.

[dan wendlandt (danwent)]

Marking as invalid, as this is not supported by the spec.

[Robert Collins (lifeless)]

It seems like a desirable thing to be able to evolve dhcp pools - every other dhcp system I've used permits that; should we update the spec then?

[Paul Collins (pjdc)]

I recently found myself needing to reduce the space covered by an allocation pool on a network managed by havana neutron. It's not always possible to plan for such cases, so it would be helpful to operators if the attribute could be made updatable and safe to update.

[Marios Andreou (marios-b)]

I just pinged Dan to see if he still has the code around to make this work. I was planning on submitting the code to get the discussion going around changing the spec to accommodate this,

thanks, marios

[Marios Andreou (marios-b)]

OK - after irc chat with Salvatore it seems like indeed this is something we'd like to have. I'll assign to myself for now and start investigating. If Dan still wants to take it then no problem. Once code is in, I will have to look at updating the spec too right?

thanks! marios

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/62042

[Mark McClain (markmcclain)]

This is not a release blocker, but if it merges before Icehouse window closes I will retarget this bug back to Icehouse.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/62042
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=c9077f6219584b9ace4a43806f69d23efea0d504
Submitter: Jenkins
Branch: master

commit c9077f6219584b9ace4a43806f69d23efea0d504
Author: marios <email address hidden>
Date: Fri Dec 13 18:57:28 2013 +0200

    Make allocation_pools attribute of subnet updateable by PUT

    Bug 1111572 was filed about a failed update (PUT) on
    'allocation_pools' of subnet. This is currently not allowed by the
    neutron API (hence DocImpact below). Following discussion on the
    bug and subsequently, it seems this is a desirable feature.

    This review makes the allocation_pools attribute of subnet
    updateable by PUT. The semantics are that the entire allocation
    pools attribute is replaced by the provided parameter (see
    provided tests for details).

    Unit tests added that exercise successful update of
    allocation_pools with sane params and update using erroneous
    allocation_pools that fall outside the subnet cidr.

    DocImpact

    Closes-Bug: 1111572
    Change-Id: I47a3a71d0d196b76eda46b1d960193fb60417ba9
    Co-Authored-By: Robert Collins <email address hidden>