Port: add propagationUplinkStatus field by winiciusallan · Pull Request #641 · k-orc/openstack-resource-controller

winiciusallan · 2026-01-12T14:42:17Z

This PR introduces the propagationUplinkStatus

https://docs.openstack.org/api-ref/network/v2/index.html#uplink-status-propagation

Notes:

I've decided to keep the default value for that field as false, even if the doc says that the default value is true. The reason is that in environments where this extension is not enabled, we can't update that field and it always returns false, so if we enforce it as true, it may raise an error. So if the user wants to enable it, they must explicitly define it.

edit:

If the extension is enabled in the environment, the default value for this field is the Openstack default value, which is true. Otherwise, it'll be nil

Partial #616

winiciusallan · 2026-01-12T22:51:34Z

weird... the uplink_status_propagation extension was not added to the devstack deployment. I tested locally and it only works by setting Q_ML2_PLUGIN_EXT_DRIVERS directly.

Has anyone ever seen something like this?

winiciusallan · 2026-01-13T17:28:42Z

Ok, here we go. I needed to add the neutron plugin on CI so that devstack can run this line and enable uplink-status-propagation accordingly. However, it looks like that Dalmatian release doesn't enable the ability to update propagateUplinkStatus by default on Devstack like did in newer releases, so the CI ended up failing. I believe that adding a line explicitly enabling uplink_status_propagation_updatable extension should work.

About the job failing on Flamingo I really don't get the why. The E2E job has failed in domain-import-error tests.

cc. @mandre I'll wait for you feedback before making any additional change.

mandre · 2026-01-15T15:12:26Z

+	// propagateUplinkStatus represents the uplink status propagation of
+	// the port.
+	// +optional
+	PropagateUplinkStatus *bool `json:"propagateUplinkStatus,omitempty"`


We've discussed this privately already, after you brought the issue to my attention: gophercloud implements the PropagateUplinkStatus in the response Port structure as a bool while it should really be a *bool with omitempty, and because of this we can't reliably know the status for PropagateUplinkStatus. I suggest that until the issue if fixed in gophercloud, we make that field immutable.

It's been a while and I don't remember all of the discussion. From reading the PR comments again, I believe we're past the gophercloud limitation (thanks to custom unmarshalling) but we still need to make the field immutable because dalmatien doesn't support updating the field. If that's effectively the case, we should add that as a comment so we know when we can change that behavior.

Yes, you're absolutely right. I'll add this comment.

mandre · 2026-01-15T15:35:21Z

About the job failing on Flamingo I really don't get the why. The E2E job has failed in domain-import-error tests.

I re-ran this job and the error is gone. I was most likely a flake (bound to happen as we add more tests).

winiciusallan · 2026-01-16T18:06:44Z

@mandre let me know when you think it is ready to go, I can squash the commits to make the PR cleaner. :)

mandre · 2026-03-09T14:57:29Z

+	// propagateUplinkStatus represents the uplink status propagation of
+	// the port.
+	// +optional
+	PropagateUplinkStatus *bool `json:"propagateUplinkStatus,omitempty"`


It's been a while and I don't remember all of the discussion. From reading the PR comments again, I believe we're past the gophercloud limitation (thanks to custom unmarshalling) but we still need to make the field immutable because dalmatien doesn't support updating the field. If that's effectively the case, we should add that as a comment so we know when we can change that behavior.

mandre · 2026-03-09T15:28:11Z

 	return floatingips.Update(ctx, c.serviceClient, id, opts).Extract()
 }

 func (c networkClient) ListPort(ctx context.Context, opts ports.ListOptsBuilder) iter.Seq2[*PortExt, error] {


We shouldn't have to update ListPort I believe.

IIRC, there was a failing test, and the solution was to change the ListPort function, but I'll double-check this to confirm what exactly the error was.

After discussion, we found a potential bug in Gophercloud, and a PR has been created. More information is there.

gophercloud/gophercloud#3726

So, do we also need the same thing for GetPort (and potentially CreatePort and UpdatePort)? It looks to me like we should wait for a new gophercloud that includes your fix.

As long as I can see, we don't need. For Get, Create, and Update Port, all of them pass a struct to the ExtractInto function, making the function pass through this path. At the end of execution, the unmarshaling of non-struct field is handled by the err = json.Unmarshal(b, &to).

I also don't like the way it is, so it is ok for me to wait for a new release and get it properly fixed. Wdyt?

That would be my preference as well.

Ok, now we have the fix, I've reverted the logic on ListPort and tests should (i hope ) pass now.

winiciusallan · 2026-04-14T00:35:05Z

@mandre I'm working again to have this PR merged. A lot of work has been done since the last commit, so I'll rebase and start addressing the comments.

winiciusallan · 2026-04-23T17:45:29Z

@mandre let me know when you think it is good. I'd like to squash the commits. keeping the history clear 🤌

mandre · 2026-05-04T15:01:52Z

@winiciusallan Just looked at your changes again. They look good to me now. Would you like to do the rebase yourself, to fix the conflicts and squash what needs squashing, or would you like me to take care of it?

winiciusallan · 2026-05-04T15:10:07Z

@mandre I will do this, thanks for asking!

mandre · 2026-05-11T15:23:11Z

 	return floatingips.Update(ctx, c.serviceClient, id, opts).Extract()
 }

 func (c networkClient) ListPort(ctx context.Context, opts ports.ListOptsBuilder) iter.Seq2[*PortExt, error] {


So, do we also need the same thing for GetPort (and potentially CreatePort and UpdatePort)? It looks to me like we should wait for a new gophercloud that includes your fix.

winiciusallan · 2026-06-30T15:58:15Z

@mandre I have added the new changes to a new commit to make the review easier. Let me know if you want me to squash or feel free to do it yourself when it is ready to go.

mandre · 2026-06-30T19:49:45Z

@mandre I have added the new changes to a new commit to make the review easier. Let me know if you want me to squash or feel free to do it yourself when it is ready to go.

Perfect, thanks! Ready to squash.

winiciusallan mentioned this pull request Jan 12, 2026

Port controller parity with CAPO #616

Open

7 tasks

github-actions Bot added the semver:major Breaking change label Jan 12, 2026

winiciusallan force-pushed the port-propagate-uplink-status branch from 8cbef64 to 4d21547 Compare January 13, 2026 16:40

mandre reviewed Jan 15, 2026

View reviewed changes

github-actions Bot added semver:minor Backwards-compatible change and removed semver:major Breaking change labels Jan 16, 2026

mandre reviewed Jan 19, 2026

View reviewed changes

Comment thread internal/osclients/networking.go Outdated

mandre reviewed Mar 9, 2026

View reviewed changes

winiciusallan force-pushed the port-propagate-uplink-status branch 2 times, most recently from 0f1e900 to 60786e3 Compare April 23, 2026 17:03

winiciusallan force-pushed the port-propagate-uplink-status branch from 60786e3 to 1749c64 Compare May 4, 2026 16:05

mandre reviewed May 11, 2026

View reviewed changes

winiciusallan force-pushed the port-propagate-uplink-status branch from 1749c64 to f6585f9 Compare June 30, 2026 15:55

port: add propagationUplinkStatus field

a58fd06

winiciusallan force-pushed the port-propagate-uplink-status branch from f6585f9 to a58fd06 Compare June 30, 2026 19:57

Uh oh!

Conversation

winiciusallan commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

winiciusallan commented Jan 12, 2026

Uh oh!

winiciusallan commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mandre commented Jan 15, 2026

Uh oh!

winiciusallan commented Jan 16, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

winiciusallan May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

winiciusallan commented Apr 14, 2026

Uh oh!

winiciusallan commented Apr 23, 2026

Uh oh!

mandre commented May 4, 2026

Uh oh!

winiciusallan commented May 4, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

winiciusallan commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mandre commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

winiciusallan commented Jan 12, 2026 •

edited

Loading

winiciusallan commented Jan 13, 2026 •

edited

Loading

winiciusallan May 11, 2026 •

edited

Loading

winiciusallan commented Jun 30, 2026 •

edited

Loading