Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Register DEFAULT group on agent start and [Caracal] Update github workflow & Fix unit tests #76

Merged
merged 3 commits into from
Nov 22, 2024

Conversation

sven-rosenzweig
Copy link
Contributor

@sven-rosenzweig sven-rosenzweig commented Nov 12, 2024

Register DEFAULT group on agent start

From upstream commit 4d3a2747 fixing bug (#1968606), registration of the
group DEFAULT does not happen automatically anymore.

If not loaded, agent fails with 'oslo_config.cfg.NoSuchOptError:
no such option api_extensions_path in group [DEFAULT]'

[Caracal] Update github workflow & Fix unit tests
Update python version to 3.10 and set the base branch in the github
workflowto Caracal(2024.1-m3).

Fix Unit Tests:
With later versions of pecan, WebTest is not a dependency of it anymore,
so in order to continue using this, an additional import in
test-requirements is required.

Remove autoconfig loading, which breaks stestr execution.
Cli options can be loaded only once (without
clearing them), which causes our tests to fail as required default
configuration cli options can not be registered, as an default configuration
has already been loaded.

As part of the secure RBAC community goal, the "enforce_new_defaults" and
"enforce_scope" setting is True by default. Avoiding PolicyNotAuthorized erros
during test execution certain actions simply need to be executed as admin.

From upstream commit 4d3a2747 fixing bug (#1968606), registration of the
group DEFAULT does not happen automatically anymore.

If not loaded, agent fails with 'oslo_config.cfg.NoSuchOptError:
no such option api_extensions_path in group [DEFAULT]'
@sven-rosenzweig sven-rosenzweig force-pushed the load_default_config branch 9 times, most recently from c3ac460 to 7b3376f Compare November 15, 2024 10:05
@sven-rosenzweig sven-rosenzweig changed the title Register DEFAULT group on agent start Register DEFAULT group on agent start and [Caracal] Update github workflow & Fix unit tests Nov 15, 2024
@sven-rosenzweig sven-rosenzweig force-pushed the load_default_config branch 2 times, most recently from 16aabd5 to c98ddee Compare November 15, 2024 10:42
Comment on lines -26 to +27
with ctx.session.begin(subtransactions=True):
with db_api.CONTEXT_WRITER.using(ctx):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I see you're replacing the ctx-session.begin with the proper Neutron-way of using db_api - is this only a problem in the tests or do we need now with caracal to do that outside the tests as well?

quoth:~/repos/networking-aci/networking_aci$ git curr
stable/2024.1-m3
quoth:~/repos/networking-aci/networking_aci$ git grep session.begin
plugins/ml2/drivers/mech_aci/allocations_manager.py:            with session.begin(subtransactions=True):
plugins/ml2/drivers/mech_aci/allocations_manager.py:        with db_api.exc_to_retry(sa.exc.IntegrityError), session.begin(subtransactions=True):
plugins/ml2/drivers/mech_aci/allocations_manager.py:        with session.begin(subtransactions=True):
plugins/ml2/drivers/mech_aci/allocations_manager.py:        with session.begin(subtransactions=True):
plugins/ml2/drivers/mech_aci/allocations_manager.py:        with db_api.exc_to_retry(sa.exc.IntegrityError), session.begin(subtransactions=True):
plugins/ml2/drivers/mech_aci/allocations_manager.py:        with session.begin(subtransactions=True):
plugins/ml2/drivers/mech_aci/common.py:        with context.session.begin(subtransactions=True):
plugins/ml2/drivers/mech_aci/common.py:        with context.session.begin(subtransactions=True):
tests/unit/plugins/ml2/drivers/mech_aci/test_driver.py:        with ctx.session.begin(subtransactions=True):
tests/unit/plugins/ml2/drivers/mech_aci/test_driver.py:                with ctx.session.begin():

Update python version to 3.10 and set the base branch in the github
workflowto Caracal(2024.1-m3).

Fix Unit Tests:

With later versions of pecan, WebTest is not a dependency of it anymore,
so in order to continue using this, an additional import in
test-requirements is required.

Remove autoconfig loading, which breaks stestr test execution.
Cli options can be loaded only once (without
clearing them), which causes our tests to fail as required default
configuration cli options can not be registered, as an default configuration
has already been loaded.

As part of the secure RBAC community goal, the "enforce_new_defaults" and
"enforce_scope" setting is True by default. Avoiding PolicyNotAuthorized erros
during test execution certain actions simply need to be executed as admin.
@sven-rosenzweig sven-rosenzweig force-pushed the load_default_config branch 3 times, most recently from 66f6e82 to 64e5755 Compare November 22, 2024 10:47
Comment on lines 106 to 113
@db_api.CONTEXT_READER
def set_hostgroup_mode(self, context, hostgroup_name, hostgroup_mode):
with context.session.begin(subtransactions=True):
with db_api.CONTEXT_WRITER.using(context):
query = context.session.query(HostgroupModeModel).filter(HostgroupModeModel.hostgroup == hostgroup_name)
hg = query.first()
if not hg:
return False
hg.mode = hostgroup_mode
return True
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has to be a writer. I wonder why we don't do an explicit add of the updated object. Does that still work with the new db_api?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with you on that. A simple test shows that this method works as intended.

As per blueprint [1], the existing use of oslo_db session
handling (e.g., context.session.begin()) introduces potential issues.
Notably, unit tests failed during the Caracal release, though
no definitive deployment impact has been identified yet.

To future-proof the code and align with recommended practices,
we are migrating to the enginefacade pattern now.
This involves replacing:
with context.session.begin():
   context.session.add(obj)

with 'db_api.CONTEXT_WRITER.using(context)'

[1] https://blueprints.launchpad.net/neutron/+spec/enginefacade-switch
[2] Oslo db spec: http://specs.openstack.org/openstack/oslo-specs/specs/kilo/make-enginefacade-a-facade.html
@sven-rosenzweig sven-rosenzweig merged commit 74260f2 into stable/2024.1-m3 Nov 22, 2024
1 check passed
@sven-rosenzweig sven-rosenzweig deleted the load_default_config branch November 22, 2024 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants