Following the discussions on #2004004, we notice that there is an error
message in the Neutron code that is misleading.
This patch intends to improve the exception message, and provide a
little bit of documentation regarding the gateway IP address
validation for subnets.
Related-bug: #2004004
Change-Id: Ibf09254b5b2fee6efd3de5e5dc6f013424831db9
Running with a stricter .pylintrc generates a lot of
C0330 warnings (hanging/continued indentation). Fix
some remaining ones in miscellaneous directories.
Also cleanup any remaining code that I missed in this
series, or has changed since I started.
Trivialfix
Change-Id: I17b4779020a7bfb369c3e721ab6638cd4a6ab50c
Running with a stricter .pylintrc generates a lot of
C0330 warnings (hanging/continued indentation). Fix
the ones in neutron/ipam.
Trivialfix
Change-Id: I8312c9a0835ee9bdae9e3b2c565bf899899349c3
Use "value > constant" syntax and not vice-versa. Also
removed disable of misplaced-comparison-constant in
.pylintrc so future ones are caught.
Trivialfix
Change-Id: I733864e7437213bfb6edde24f207b2c9861998c6
This patch removes the ``IpAddressAllocationNotFound`` exception. This
exception was raised when a IPAM register was called to be deleted
but not found.
As reported in the LP bug, this IPAM register deletion can be called
several times if a port fails during the creation. The IPAM register
deletion calls the DB deletion but doesn't raise any exception if the
register does not exist. The code ensures the IPAM register is
deleted and there is no need to fail if it is not present anymore.
This patch also removes the exception catch and try in "update_port",
that was added in [0] as a fix for [1]. That was added because the
subnet deletion code involved a port update call [2] during the
IP allocation deletion, if any port was still present in the subnet.
Since [3], this code is not needed because the subnet deletion does
not call a port update anymore.
[0]https://review.opendev.org/c/openstack/neutron/+/373536
[1]https://bugs.launchpad.net/neutron/+bug/1622616
[2]https://github.com/openstack/neutron/blob/pike-em/neutron/db/db_base_plugin_v2.py#L1017-L1018
[3]https://review.opendev.org/c/openstack/neutron/+/713045
Closes-Bug: #1965807
Related-Bug: #1954763
Related-Bug: #1622616
Change-Id: I5b96b3a91aacffe118ddbb91a75c4892818ba97a
When needing to create a point to point connection via a subnet,
generally and /31 is the recommended cidr. Neutron supports /31
disabling dhcp and gateway on a subnet. /32 is also supported in
openstack.
Closes-Bug: #1580927
Change-Id: I3bfa3efb9fb8076656b16c89d2f35d74efde12b7
Now is possible to define a gateway IP when creating a subnet using a
subnet pool. The IPAM subnet generator retrieves the available IP
ranges in the subnet pool and generates a list of candidate subnets
with the prefix lenght defined. If the gateway IP can be allocated in
one of those candidate subnets, the IPAM returns a valid IpamSubnet
that will be used to create a Neutron subnet.
Closes-Bug: #1904436
Change-Id: Ib1d1f591c4d0f59ebff3ddcb3be7b10b0b5e67dc
As spotted in Focal testing patch [0], pep8 test fails with many
C0321 false-positives, reported in pylint as current version does not
support python 3.8 [1]
Use a newer version of pylint and astroid, fixing or disabling some of
the new checks: no-else-*, unnecessary-comprehension, import-outside-toplevel
[0] https://review.opendev.org/#/c/738163/
[1] https://github.com/PyCQA/pylint/issues/2737
Change-Id: Ie646b7093aa8634fd950c136a0eba9adcf56591c
OVN distributed services like Metadata and DHCP uses now
DEVICE_OWNER_DHCP device_owner which isn't distributed by its nature.
To fully use benefits of OVN Distributed ports (localports) [1]
and to not overlap with Neutron logic created for not-distributed
ports we should use new device_owner.
In this change we need also to bump minimum required
neutron-lib to 2.4.0.
[1] https://www.ovn.org/support/dist-docs/ovn-nb.5.txt
Change-Id: I0a69f1bddaa7030c7287216e62ec1ac6dd381475
With python 3.x, classes can use the metaclass= logic
to not require usage of the six library.
One step in removing all of six usage from neutron.
Change-Id: I2f815e412d9a96eb5faf2b3bb3a1e393a9db9309
When a subnet is created and allocated, the gateway IP is created
based on the subnet CIDR. In case of IPv6 prefix delegation, this
CIDR is a temporary one. In this case the gateway IP cannot be
assigned yet and the value stored in the DB should be "None".
IpamBackendMixin._gateway_ip_str must read properly the IP version
stored in the "subnet" variable, under the key "ip_version"
instead of "version".
Closes-Bug: #1856726
Closes-Bug: #1856675
Change-Id: I9313c880cc458f08dc3a1b0ff13187b764ba7042
The new hacking release found some pep8 errors, fix them.
Unfortunately, moving to 2.0.0 pulled a thread with other
dependencies causing issues, so for now pin it below
1.2.0 to get the gate functioning again.
Change-Id: Ic8ee670e03dde0669231477a4209b01eb8fba7fd
All of the externally consumed variables from neutron.common.constants
now live in neutron-lib. This patch removes neutron.common.constants
and switches all uses over to lib.
NeutronLibImpact
Depends-On: https://review.openstack.org/#/c/647836/
Change-Id: I3c2f28ecd18996a1cee1ae3af399166defe9da87
Reduces E128 warnings by ~260 to just ~900,
no way we're getting rid of all of them at once (or ever).
Files under neutron/tests still have a ton of E128 warnings.
Change-Id: I9137150ccf129bf443e33428267cd4bc9c323b54
Co-Authored-By: Akihiro Motoki <amotoki@gmail.com>
1.It is best not to use 'netaddr.IPSet.add',
because _compact_single_network in 'IPSet.add' is quite time consuming
2.When the current address pool does not have enough addresses,
all addresses are allocated from the current pool,
and allocations are continued from the next address pool
until all addresses are assigned.
Change-Id: I804a95fdaa3552c785e85ffab7b8ac849c634a87
Closes-Bug: #1813253
Today the neutron common exceptions already live in neutron-lib and are
shimmed from neutron. This patch removes the neutron.common.exceptions
module and changes neutron's imports over to use their respective
neutron-lib exception module instead.
NeutronLibImpact
Change-Id: I9704f20eb21da85d2cf024d83338b3d94593671e
Some L3 ports can directly modify the IP address now,
but there are some types of device_owner, for instance
network:router_centralized_snat, should not allow to
change the IP address, otherwise it will increase L3
agent code complexity.
Since router public gateway port is related to the
external network, and this port can be used for some
service, like VPN. So after this patch we will only
allow the gateway port to update the IP address
directly, aka device_owner network:router_gateway.
The following l3 router ports will not be allowed to
directly update the IP address:
network:ha_router_replicated_interface
network:router_centralized_snat
network:router_ha_interface
network:router_interface
network:router_interface_distributed
Remove one unit test case since it will not occur in
neutron.
Change-Id: I2890a71abc82cb781b601afb879bdcca10e39257
Closes-Bug: #1796824
Create a method for bulk assignment of IP addresses within the ipam
driver, to support bulk creation of ports.
This also changes the logic for how the window of available IP addresses
to assign from is calculated within the neutrondb IPAM driver. The
Python random module is used to produce a statistically sampled set of
IP addresses out of the set of available IPs; this will facilitate
collission avoidance. When requesting multiple IP addresses the
selection window sized is increased significantly to ensure a larger
number of available IPs, but caps are placed on the amount to make sure
we do not transgress system limits when building pools of IPv6
addresses.
Change-Id: Iad8088eaa261b07153fa358ae34b9a2442bc2a3e
Implements: blueprint speed-up-neutron-bulk-creation
The neutron.db.api.context_manager already references neutron-lib's
context manager; so consumers of it are already using neutron-lib. This
patch switches neutron's references to the context_manager over to
use neutron-lib's directly rather than that in neutron.db.api.
NeutronLibImpact
Change-Id: I97120faeec73690592ed21a5ec3c6202f61e1429
Michael Bayer while analysing neutron process function call trace,
suggested to run queries against specific columns rather than full
ORM entities as it can help reduce load both at the DB level and
in the Python level since they are much faster to fetch as
non-ORM entities. In this patch we are trying that on simpler
queries to improve neutron performance.
Co-Authored-By: Joe Talerico <jtaleric@redhat.com>
Change-Id: I6a41e9487a4427f876442bbeeae61974e892225e
Try to give it a large random chance during generate IP
address.
The DB retry mechanism change moved to neutron-lib:
I5ad139bdfb3ae125658b36d05f85f139a1b47bee
Closes-Bug: #1777968
Change-Id: I2acc9c720b39271bde2a89da4a66958f7aba5b7d
Fix W503 (line break before binary operator) pep8 warnings
and no longer ignore new failures.
Trivialfix
Change-Id: I7539f3b7187f2ad40681781f74b6e05a01bac474
There was one code path where the existing device_owner
value, which is used for subnet selection when service_type's
are in use was not passed to _imap_get_subnets(). This could
trigger one of two exceptions - HostNotCompatibleWithFixedIps()
or IpAddressGenerationFailureNoMatchingSubnet() depending
on the environment. Pass it along if known.
Also update the IpAddressGenerationFailureNoMatchingSubnet
exception to print the network_id and service_type values since
it could aid in debugging the problem quicker.
Change-Id: Ic13901b92cac05e8ddf1092b66aa5bcc5623fa8a
Closes-bug: #1637366
Instead of using old _get_subnet() method new method
_get_subnet_object() will now be used in neutrondb_ipam
driver and because of that it will use Subnet OVO object.
Change-Id: Ie9876064da989842576eeb30804b6a991d4d2937
Partially-Implements: blueprint adopt-oslo-versioned-objects-for-db
Since Pike log messages should not be translated.
This patch removes calls to i18n _LC, _LI, _LE, _LW from
logging logic throughout the code. Translators definition
from neutron._i18n is removed as well.
This patch also removes log translation verification from
ignore directive in tox.ini.
Change-Id: If9aa76fcf121c0e61a7c08088006c5873faee56e
We only support one driver right now so there isn't a point
in logging it for every single IP allocation call.
TrivialFix
Change-Id: Iebd753e9ca433227c9622227ba76a246b4fea2b3
Usage reader and writer for db operations.
In Neutron code we have common situation like:
with context.session.begin():
context.session.add(obj)
self._make_obj_dict(obj)
With new enginefacade we change context.session.begin() for
db.context_manager.writer(reader).using(context).
When object leaves this with-block, its reference to session is
cleared because session is discarded. To use this object later to
load some data from its dependencies, we have to provide different
session to work in. To solve this obj either can be moved under
with-block or we have to do context.session.add(obj) one more time
to be able to load relations.
This change also switches to usage of new enginefacade for some db
operations with ports, in order to pass unit and functional tests.
Partially-Implements blueprint: enginefacade-switch
Change-Id: Ia15c63f94d2c67791da3b65546e59f6929c8c685
This class was defined a while ago, but never used
in the code for some reason. Let's use it so we don't
have two nearly-identical strings in different places.
Change-Id: Ib0e1ae51c8ef579de30ba26d30bda4bdeee5cbb8
This patch introduces and integrates Oslo-Versioned Objects for
IpamAllocationPool,IpamAllocation and IpamSubnet model classes.
Change-Id: Ib7d86dd10e92b5e2baeef0e69620c8688859fe7d
Partially-Implements: blueprint adopt-oslo-versioned-objects-for-db
Co-Authored-By: Anindita Das <anindita.das@intel.com>
Co-Authored-By: Mohit Malik <mohit.malik@intel.com>
Co-Authored-By: Victor Morales <victor.morales@intel.com>
Objects must use project_id and not tenant_id. The object framework
ensures that tenant_id is added as an extra field for backward
compatibility.
This patch reverts the workaround implemented in change
I4ec9340094bc51cd8aa6e5112bf8114aa26c2982 and implements a proper fix
by explicitly updating the objects.
Co-Authored-By: Artur Korzeniewski <artur.korzeniewski@intel.com>
Co-Authored-By: Darek Smigiel <smigiel.dariusz@gmail.com>
Closes-Bug: #1630748
Change-Id: Iab90bcab41655b2e210aea0e7581eb00b94ce5e5
Neutron Manager is loaded at the very startup of the neutron
server process and with it plugins are loaded and stored for
lookup purposes as their references are widely used across the
entire neutron codebase.
Rather than holding these references directly in NeutronManager
this patch refactors the code so that these references are held
by a plugin directory.
This allows subprojects and other parts of the Neutron codebase
to use the directory in lieu of the manager. The result is a
leaner, cleaner, and more decoupled code.
Usage pattern [1,2] can be translated to [3,4] respectively.
[1] manager.NeutronManager.get_service_plugins()[FOO]
[2] manager.NeutronManager.get_plugin()
[3] directory.get_plugin(FOO)
[4] directory.get_plugin()
The more entangled part is in the neutron unit tests, where the
use of the manager can be simplified as mocking is typically
replaced by a call to the directory add_plugin() method. This is
safe as each test case gets its own copy of the plugin directory.
That said, unit tests that look more like API tests and that rely on
the entire plugin machinery, need some tweaking to avoid stumbling
into plugin loading failures.
Due to the massive use of the manager, deprecation warnings are
considered impractical as they cause logs to bloat out of proportion.
Follow-up patches that show how to adopt the directory in neutron
subprojects are tagged with topic:plugin-directory.
NeutronLibImpact
Partially-implements: blueprint neutron-lib
Change-Id: I7331e914234c5f0b7abe836604fdd7e4067551cf
This code is not being used outside neutron tree so it's
safe to change the api to use context instead of session.
This change is required for implementing Ipam OVO and is
consistent with rest of the DB API code in neutron.
Change-Id: I600ce4384632bf013846e8aa1618acb126f7c598
Partially-Implements: blueprint adopt-oslo-versioned-objects-for-db
If a subnet is removed after lookup but before IP allocation
insert, we can end up getting a DBReferenceError in the IPAM
system. Since this can occur on any flush we force a flush
with a session context manager to get the DBReference error
in a desired spot (in multi-writer galera this is a deadlock
much later, which we retry). We then convert it to SubnetNotFound,
which is an expected exception.
Closes-Bug: #1623402
Change-Id: I65632b174f8692cd465c721e285526827842a740
Sometimes object users need access to corresponding models that are used
to persist object data. While it's not encouraged, and object consumers
should try to rely solely on object API and fields, we should fulfill
this special need, at least for now.
One of use cases to access the corresponding database model are
functions registered by plugins to extend core resources. Those
functions are passed into register_dict_extend_funcs and expect the
model as one of its arguments.
Later, when more objects are adopted in base plugin code, and we are
ready to switch extensions to objects, we can pass to those functions
some wrappers that would trigger deprecation warnings on attempts to
access attributes that are not available on objects; and then after a
while finally switch to passing objects directly instead of those
wrappers. Of course, that would not happen overnight, and the path would
take several cycles.
To avoid the stored reference to the model to influence other code
fetching from the session, we detach (expunge) the model from the active
database session on every fetch. We also refresh the model before
detaching it when the corresponding object had synthetic fields changed,
because that's usually an indication that some relationships may be
stale on the model.
Since we now consistently detach the model from the active session on
each fetch, we cannot reuse it. So every time we hit update, we now need
to refetch the model from the session, otherwise we will hit an error
trying to refresh and/or detach an already detached model. Hence the
change in NeutronDbObject.update to always trigger update_object
irrespective to whether any persistent fields were changed. This makes
test_update_no_changes test case incorrect, hence its removal.
Due to the way RBAC metaclass works, it may trigger cls.get_object in
the middle of object creation (to validate newly created RBAC entry
against the object). It results in duplicate expunge calls for the same
object model (one during object creation, another when fetching the same
object to validate it for RBAC). To avoid that, switched RBAC code from
objects API to direct objects.db_api.get_object calls that will avoid
triggering the whole model expunge/refresh machinery.
Now that we have models stored on objects, the patch switched back
plugin code to passing models in places where we previously, by mistake,
were passing objects into extensions.
Specifically, the switch for allowed address pairs occurred with
I3c937267ce789ed510373616713b3fa9517c18ac. For subnetpools, it happened
in I1415c7a29af86d377ed31cce40888631a34d4811. Neither of those was
released in Mitaka, so it did not break anyone using major releases.
Also, we have not heard from any trunk chaser that would be affected by
the mistake.
There are not other objects used in database code where we would pass
them into extensions, so we should be good.
Closes-Bug: #1621837
Change-Id: I130609194f15b89df89e5606fb8193849edd14d8
Partially-Implements: blueprint adopt-oslo-versioned-objects-for-db
This is the Neutron side of the unaddressed ports blueprint. In order
to allow unaddressed ports, Nova wants the port to explicitly say it
is okay that it doesn't have any IP addresses.
In Neutron, an unaddressed port is one that was created by explicitly
passing [] in fixed_ips to create the port. A new DB field is added
to the port to distinguish the unaddressed port case from the deferred
IP allocation case where routed networks is involved.
Change-Id: Ia61af4c14e955697a7d3fcc0bf4826a6d9475c98
Implements: blueprint vm-without-l3-address
APIImpact: port now has ip_allocation attribute, set on port create
Oslo.utils provides same method get_ipv6_addr_by_EUI64, let's use
it and deprecate method get_ipv6_addr_by_EUI64 in neutron.
Closes-Bug: #1597213
Change-Id: If1eda1283b4405e13ccb60dab530ce4cc1b22948
This changes the way that IPAM decides which subnets to use when
assigning IPs to newly created ports. If the port has a defined
device_owner, this is used to filter available subnets to choose
from only those with a matching service_type or no service_type
at all.
If the given network has no service subnets, then the existing
behaviour is used.
A new IPAM exception is introduced to handle the following scenarios:
1. A port is created with a device_owner and only non-matching service
subnets exist.
2. A port is created without a device owner, and no subnets exist
without a service_type.
With this patch, service subnets are now usable.
Implements: blueprint service-subnets
APIImpact: subnet-create and subnet-update with service_types
DocImpact: IPs assigned to new ports will now come from a service subnet
matching the port device_owner, if one exists.
Closes-Bug: 1544768
Change-Id: If3dd94a46bdee24c13d1f17c4f2e69af0cb8af63
With the in-tree pluggable IPAM driver, IPAM rollback tries to use the
DB session after it has been rolled back due to an exception. This
driver doesn't need roll back, so fix this by adding a method to the
driver signalling that rollback shouldn't be attempted.
Change-Id: Ic254789e58a8a51cd1aa943cb71de12410f4c0a7
Closes-Bug: #1603162
Related-Bug: #1516156
Rafael Weingärtner