Skip to content

feat: PUC-917: portgroup name validation#1666

Open
haseebsyed12 wants to merge 1 commit intomainfrom
portgroup_name_validation
Open

feat: PUC-917: portgroup name validation#1666
haseebsyed12 wants to merge 1 commit intomainfrom
portgroup_name_validation

Conversation

@haseebsyed12
Copy link
Contributor

@haseebsyed12 haseebsyed12 commented Feb 6, 2026

Portgroup Name Validation

Adds middleware to enforce portgroup naming convention required by undersync. Names must match {node_name}-port-channel{number} where number is between 100-998.

Validation Examples

Rejected - number out of range (20 < 100):

openstack baremetal port group create \
  --node 207ee67e-c8b1-41c1-bcf0-9c4c99db6fde \
  --name "blah-port-channel20" \
  --mode "active-backup" \
  --support-standalone-ports
# Portgroup name 'blah-port-channel20' has invalid port-channel number 20. Must be between 100 and 998 (inclusive). (HTTP 400)

Rejected - missing number:

openstack baremetal port group create \
  --node 207ee67e-c8b1-41c1-bcf0-9c4c99db6fde \
  --name "blah-port-channel" \
  --mode "active-backup" \
  --support-standalone-ports
# Portgroup name 'blah-port-channel' must match format '{node_name}-port-channel{number}' (HTTP 400)

Rejected - wrong suffix format:

openstack baremetal port group create \
  --node 207ee67e-c8b1-41c1-bcf0-9c4c99db6fde \
  --name "blah-port-abc234" \
  --mode "active-backup" \
  --support-standalone-ports
# Portgroup name 'blah-port-abc234' must match format '{node_name}-port-channel{number}' (HTTP 400)

Rejected - non-numeric suffix:

openstack baremetal port group create \
  --node 207ee67e-c8b1-41c1-bcf0-9c4c99db6fde \
  --name "blah-port-channelabc234" \
  --mode "active-backup" \
  --support-standalone-ports
# Portgroup name 'blah-port-channelabc234' must match format '{node_name}-port-channel{number}' (HTTP 400)

Rejected - number out of range (2234 > 998):

openstack baremetal port group create \
  --node 207ee67e-c8b1-41c1-bcf0-9c4c99db6fde \
  --name "blah-port-channel2234" \
  --mode "active-backup" \
  --support-standalone-ports
# Portgroup name 'blah-port-channel2234' has invalid port-channel number 2234. Must be between 100 and 998 (inclusive). (HTTP 400)

Accepted - valid name:

openstack baremetal port group create \
  --node 207ee67e-c8b1-41c1-bcf0-9c4c99db6fde \
  --name "blah-port-channel234" \
  --mode "active-backup

@haseebsyed12 haseebsyed12 force-pushed the portgroup_name_validation branch 5 times, most recently from e7af2b7 to e1f3b20 Compare February 6, 2026 17:55
@haseebsyed12 haseebsyed12 changed the title feat: patch portgroup name validation feat: PUC-917: portgroup name validation Feb 6, 2026
@haseebsyed12 haseebsyed12 force-pushed the portgroup_name_validation branch from e1f3b20 to dd8a80a Compare February 6, 2026 18:22
@haseebsyed12
Copy link
Contributor Author

haseebsyed12 commented Feb 6, 2026

to test this validation change uc_repo_ref to branch test_portgroup_name_validation in argocd

@haseebsyed12 haseebsyed12 requested review from a team February 6, 2026 18:27
@haseebsyed12 haseebsyed12 marked this pull request as ready for review February 6, 2026 18:27
@haseebsyed12 haseebsyed12 force-pushed the portgroup_name_validation branch from dd8a80a to f71516a Compare February 6, 2026 18:45
Strategy: Minimal patch to Ironic's app.py that imports middleware.
The patch adds 6 lines before `return app` in setup_app() - if upstream
changes break this, the build fails immediately rather than silently.

All validation logic (`{node_name}-port-channel{number} where number is between 100-998`) lives in ironic-understack package,
making it easy to update without touching the patch.
@haseebsyed12 haseebsyed12 force-pushed the portgroup_name_validation branch from f71516a to 279c334 Compare February 6, 2026 18:54
Copy link
Contributor

@cardoe cardoe left a comment

Choose a reason for hiding this comment

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

Since this is a direct Ironic patch, please read the information on how to backport this patch into our stable branch in the README.md in the container directory. Please make me a Jira to update the documentation to make this clearer and I will work on that when I return.

Copy link
Contributor

@cardoe cardoe left a comment

Choose a reason for hiding this comment

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

Oh never mind. I see what you did by enabling more middleware's. That's actually an interesting idea. This might be good to propose an interface upstream on how to do this without needing to directly patch ironic. Make a Jira for this anyway and we can discuss it when I return. You're also welcome to create an ironic bug at https://bugs.launchpad.net and ask for a way to load additional middleware's. Link this PR and that'll start the discussion upstream and we can pick it up further when I return. But doing that bug report would be the first step and very helpful.

Copy link
Contributor

@cardoe cardoe left a comment

Choose a reason for hiding this comment

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

I'm +1 on this. But with the improvements I suggested as a follow on. You can add an entry point into pyproject.toml for this module as well.

Comment on lines +20 to +23
+ from ironic_understack.portgroup_name_middleware import (
+ PortgroupNameValidationMiddleware,
+ )
+ app = PortgroupNameValidationMiddleware(app)
Copy link
Contributor

Choose a reason for hiding this comment

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

So we can look into installing a hook into a path like ironic.api.middleware like other interfaces like hooks. Then we use steveadore to load them by name. This would make this dynamic and extensible.

@cardoe
Copy link
Contributor

cardoe commented Feb 6, 2026

If you open that bug report please link it here as a comment.

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.

3 participants