-
Notifications
You must be signed in to change notification settings - Fork 26
Port b/neutron-{ext,tenant}-net-ksv3 to openstacksdk #344
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
base: main
Are you sure you want to change the base?
Conversation
The old python clients are deprecated and the openstacksdk code is a bit easier to read. Functionality should not be affected. Signed-off-by: Zachary Raines <[email protected]>
c45da5e to
0e6853d
Compare
|
Tested with a fresh jammy-caracal stsstack bundles deployment. |
brianphaley
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some nits but otherwise Ok. Changing to use the SDK was on my list for next year, as is some cleanup of calls using the openstackclient, but this is a good start.
So are the non -ksv3 files never used? Or post-deploy-config ?
BTW, there has been a similar multi-year push upstream as we want to retire neutronclient eventually, https://review.opendev.org/q/topic:%22bug/1999774%22
openstack/bin/neutron-ext-net-ksv3
Outdated
| 'network_id': network['id'], | ||
| 'enable_dhcp': False, | ||
| 'ip_version': 4, | ||
| 'tenant_id': project_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the project_id key work here? We've been moving away from tenant_id upstream for a while when possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
project_id isn't a named kwarg but I can test if it works since it seems it's passed along to the Subnet message. Maybe worth a PR in openstacksdk.
| name=subnet_name, | ||
| cidr=cidr, | ||
| ip_version=4, | ||
| tenant_id=project_id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/project_id if possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.openstack.org/openstacksdk/latest/user/connection.html#openstack.connection.Connection.create_subnet From my reading project_id isn't a supported keyword here, but I haven't actually looked at the code of the method.
| project_id = auth.get_project_id(sess) | ||
| project_id = conn.current_project_id | ||
| if not project_id: | ||
| logging.error("Unable to locate project id for %s.", opts.tenant) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll send a follow-on adding opts.project (-p) once this merges
Does openstacksdk work with keystone v2? If so we can probably unify the scripts. |
While the signature of create_subnet doesn't explicitly accept project_id, it correctly passes it along to the API nonetheless. Signed-off-by: Zachary Raines <[email protected]>
The old python clients are deprecated and the openstacksdk code is a bit easier to read. Functionality should not be affected.