networking-calico: eliminate "SQL execution without transaction" warnings

[nelljerram]

DevStack runs neutron-server with [DEFAULT] debug = True, which enables an ORM-execute listener in neutron.objects.base that warns on every session.execute(...) where session.in_transaction() is False:

ORM session: SQL execution without transaction in progress, traceback:
...

It's a SQLAlchemy-2.0-compliance tripwire: it flags statements that are running in implicit-transaction mode (auto-begun and auto-discarded per statement) instead of the explicit reader/writer pattern. The warning is debug-only and doesn't affect correctness today, but it's real 2.0-readiness signal and very noisy in the journal during scale runs.

After PR #12930 removed our outer CONTEXT_WRITER.using wrapper from the mech driver's postcommit body, every raw context.session.query(...) in endpoints.WorkloadEndpointSyncer.get_extra_port_information ran with no enginefacade-recognised transaction active, and so tripped the listener. The PR-#12930 lesson stands: we can't restore a single broad wrapper, because the body also calls @retry_if_session_inactive- decorated upstream methods (self.db.get_subnet via add_port_gateways, self.db.get_security_groups via add_port_sg_names) whose retry recovery for the _ensure_default_security_group IntegrityError race is exactly what gets disabled by an outer transaction.

Restructure get_extra_port_information into three phases by transaction shape:

• Phase A -- raw context.session.query(...) reads (fixed IPs, floating IPs, network, QoS rules) plus self.db._get_port_security_group_bindings (private upstream helper, no @retry_if_session_inactive). These get a single CONTEXT_READER.using(context) block so session.in_transaction() is True at execute time. Safe to wrap because none of these calls goes through a retry-decorated upstream method that would have its retry disabled.

  All five reads are now inlined into the with block. The QoS rule queries use list(...) to force materialisation inside the reader; the other four are list-comprehensions that iterate eagerly before the with exits.

• Phase B -- add_port_gateways and add_port_sg_names call into @retry_if_session_inactive-decorated upstream methods and stay outside Phase A's reader for the same reason PR networking-calico: stop wrapping postcommit hooks in CONTEXT_WRITER #12930 stripped the old wrapper: keep the inner retry decorator enabled. Each upstream call manages its own reader transaction internally, so the warning doesn't fire here either.

• Phase C -- add_port_interface_name (pure compute) and add_port_project_data (Keystone-backed in-process cache) touch no DB; position relative to A/B is correctness-irrelevant.

The QoS handling is also factored: add_port_qos becomes a pure static method build_qos_controls(bw_rules, pr_rules) that takes already-materialised rule lists, and the rule fetch lives in the caller -- inside Phase A for the postcommit path, from self._bulk for the resync path. This removes the previous bulk-vs-postcommit branch from inside the method body and makes the consumer testable in isolation.

The four tiny one-call helpers (get_security_groups_for_port, get_fixed_ips_for_port, get_floating_ips_for_port, get_network_properties_for_port) are removed entirely; their bodies are inlined.

Misleading docstrings on add_port_gateways, add_port_sg_names, and the former add_port_qos -- left over from the pre-#12930 era when an outer _txn_from_context / CONTEXT_WRITER.using wrapped the entire postcommit body -- claimed "This method assumes it's being called from within a database transaction and does not take out another one". That assumption was true then; it isn't now. Updated to describe the actual Phase A / Phase B placement.

Tests: 185 pass (make tox-caracal). No mech-driver semantics change for either the postcommit path or the resync (bulk) path.

Release note:

TBD

[Copilot]

Pull request overview

This PR reduces SQLAlchemy “SQL execution without transaction” debug warnings in the OpenStack Calico ML2 driver by ensuring raw SQLAlchemy session.query(...) reads run under an explicit CONTEXT_READER transaction, while keeping calls that rely on Neutron’s @retry_if_session_inactive behavior outside any outer transaction.

Changes:

• Wrap raw per-port DB reads in get_extra_port_information() with db_api.CONTEXT_READER.using(context) to ensure session.in_transaction() is true at execute time.
• Refactor QoS handling: inline QoS rule fetch in callers and convert add_port_qos into a pure build_qos_controls(bw_rules, pr_rules) helper.
• Inline and remove several small helper methods that previously performed raw per-port queries.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.