Skip to content

networking: extract port uplink status propagation into extension - #3609

Open
kayrus wants to merge 1 commit into
gophercloud:mainfrom
kayrus:uplinkstatuspropagation
Open

networking: extract port uplink status propagation into extension#3609
kayrus wants to merge 1 commit into
gophercloud:mainfrom
kayrus:uplinkstatuspropagation

Conversation

@kayrus

@kayrus kayrus commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

Fixes #3605

@github-actions github-actions Bot added edit:networking This PR updates networking code semver:major Breaking change labels Jan 22, 2026
@kayrus
kayrus force-pushed the uplinkstatuspropagation branch from 850dfb6 to f9a6b41 Compare January 22, 2026 14:39
@coveralls

coveralls commented Jan 22, 2026 •
edited
Loading

Copy link
Copy Markdown

Coverage Status

coverage: 63.983% (-0.04%) from 64.024% — kayrus:uplinkstatuspropagation into gophercloud:main

@kayrus

kayrus commented Jan 22, 2026

Copy link
Copy Markdown
Contributor Author

@mandre @stephenfin @EmilienM please review

mandre
mandre reviewed Jan 28, 2026
View reviewed changes

@mandre mandre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The code itself looks good.

A couple of small things:

  • we should probably add a doc.go file for good measure.
  • I wished we had acceptance test covering PropagateUplinkStatus. It would have clearly highlighted the breaking change.

How easy would it be to add those?

@winiciusallan

winiciusallan commented May 11, 2026

Copy link
Copy Markdown
Contributor

hi @kayrus! Are you intending to continue working on this? If so, I'll be happy to review and help to merge it. If not, it'll be a pleasure to address the comments myself :).

@kayrus
kayrus force-pushed the uplinkstatuspropagation branch from f9a6b41 to 00962f6 Compare May 12, 2026 07:52
@github-actions github-actions Bot added the edit:config This PR updates config code label May 12, 2026
@kayrus
kayrus force-pushed the uplinkstatuspropagation branch 2 times, most recently from ff71a9f to a314e23 Compare May 12, 2026 08:55
@kayrus

kayrus commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

according to the internal/acceptance/openstack/networking/v2.TestExtensionsList test results, the uplink-status-propagation extension is not appearing in the list of available neutron extensions, despite being enabled in the devstack config :/

@winiciusallan

winiciusallan commented May 12, 2026

Copy link
Copy Markdown
Contributor

@kayrus you need to enable the neutron plugin in the CI so devstack can run the function to activate the extension

https://github.com/openstack/neutron/blob/master/devstack/lib/uplink_status_propagation

@kayrus

kayrus commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

@winiciusallan it is enabled:

gophercloud/.github/workflows/functional-networking.yaml

Line 78 in 148e8c3

enabled_services: "neutron-dns,neutron-qos,neutron-segments,neutron-trunk,neutron-uplink-status-propagation,neutron-network-segment-range,neutron-port-forwarding,openstack-cli-server"

@winiciusallan

winiciusallan commented May 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

@kayrus indeed. but I meant the devstack neutron plugin, like we did here

https://github.com/k-orc/openstack-resource-controller/blob/main/.github/workflows/e2e.yaml#L43

@github-actions github-actions Bot added edit:actions This PR updates GitHub Actions code edit:testinfra This PR updates testing infrastructure code labels May 12, 2026
@kayrus

kayrus commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

but I meant the devstack neutron plugin, like we did here

https://github.com/k-orc/openstack-resource-controller/blob/main/.github/workflows/e2e.yaml#L43

the uplink-status-propagation extension is not a separate plugin requiring github repo enablement

@kayrus
kayrus force-pushed the uplinkstatuspropagation branch from 3689c33 to 1138c40 Compare May 12, 2026 09:37
@winiciusallan

winiciusallan commented May 12, 2026

Copy link
Copy Markdown
Contributor

but I meant the devstack neutron plugin, like we did here
https://github.com/k-orc/openstack-resource-controller/blob/main/.github/workflows/e2e.yaml#L43

the uplink-status-propagation extension is not a separate plugin requiring github repo enablement

Yeah, you're right (the tests speak for themselves). The way that I suggested is how was working for me. Thanks for the insight.

Regarding the failing test, it looks like propagate_uplink_status is not a valid filter field, wondering why the other tests are green.

@kayrus
kayrus requested a review from mandre May 21, 2026 09:59
@kayrus
kayrus force-pushed the uplinkstatuspropagation branch 3 times, most recently from 3978ff2 to 8aee8ab Compare May 26, 2026 17:32
@kayrus
kayrus force-pushed the uplinkstatuspropagation branch from 8aee8ab to 9b9d6da Compare May 26, 2026 18:01
winiciusallan
winiciusallan approved these changes May 26, 2026
View reviewed changes

@winiciusallan winiciusallan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a nit comment. Despite that, all changes look pretty good to me. Thanks @kayrus!

Comment thread ...e/openstack/networking/v2/extensions/uplinkstatuspropagation/uplinkstatuspropagation_test.go
th.AssertNoErr(t, err)

// Skip these tests if we don't have the required extension
networking.RequireNeutronExtension(t, client, "uplink-status-propagation")

@winiciusallan winiciusallan May 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: fwiw, we can't also test if we don't have the uplink-status-propagation-updatable extension

@kayrus kayrus May 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

devstack activates both uplink-status-propagation and uplink-status-propagation-updatable. for the test suite this is acceptable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@winiciusallan winiciusallan winiciusallan approved these changes
@mandre mandre Awaiting requested review from mandre

Assignees

No one assigned

Labels

edit:actions This PR updates GitHub Actions code edit:config This PR updates config code edit:networking This PR updates networking code edit:testinfra This PR updates testing infrastructure code semver:major Breaking change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

networking.v2.Port: Missing pointer on PropagateUplinkStatus

4 participants

@kayrus @coveralls @winiciusallan @mandre