port.valueSpecs is a no-op

[mdbooth]

/kind bug

OpenStackMachine.Spec.Ports.ValueSpecs ultimate results in this gophercloud create call: https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/010408dfca1cfe775234f54359bb2ffe524bc5c2/pkg/cloud/services/networking/port.go#L135-L147

However, ValueSpecs does not correspond to any field in Neutron and consequently setting it has no effect. This is also an issue in gophercloud.

The intended purpose of ValueSpecs is an api 'escape hatch' which allows arbitrary request fields to be set during port creation. If this use case still exists we could fix the implementation in gophercloud and keep the CAPO api. However, as we know nobody is currently relying on it we could also remove it and add it back again should it come up again.

cc @lentzi90

[lentzi90]

Thanks for clearing this up and creating the issue! I will clarify internally if/how we use this and comeback with some suggested action in roughly a week

[EmilienM]

@kubernetes-sigs/cluster-api-provider-openstack-maintainers please add v1beta1 to milestone.

[lentzi90]

Alright I have gotten an answer. We do need this. I would prefer to keep the API here and fix the implementation in gophercloud

[dulek]

Alright I have gotten an answer. We do need this. I would prefer to keep the API here and fix the implementation in gophercloud

How do you depend on this when it basically has no effect on resources created in OpenStack?

[lentzi90]

The current implementation is broken, yes. But we do need the functionality. How we did not notice that it was broken I cannot really tell though... this is unclear to me still

[dulek]

The current implementation is broken, yes. But we do need the functionality. How we did not notice that it was broken I cannot really tell though... this is unclear to me still

Alright, I understand. Is there any other way we could implement your use case other than adding a free-for-all parameter to a resource? Honestly I don't like it for the reason of its ambiguity. For example what happens if user puts name into ValueSpecs? Do we expect users to be able to override Name of a port? How would CAPO behave here? Having such a parameter is problematic from the API standpoint IMO.

[lentzi90]

I would expect that we can error out if any of the ValueSpecs clash with existing values. I will also try to clarify the exact use-case so that we could perhaps verify it here upstream. If the point is lack of fields in the current API I hope we can add them instead

[dulek]

I would expect that we can error out if any of the ValueSpecs clash with existing values.

Sure, but it's a grey, ambiguous territory, where what user expects might stop to match what we expect. There's also a question on how to implement that on gophercloud side, e.g. if user specifies foo in valueSpecs and gophercloud starts to support foo explicitly, do we invalidate all the older apps using valueSpecs for that? Same could apply to CAPO.

I will also try to clarify the exact use-case so that we could perhaps verify it here upstream. If the point is lack of fields in the current API I hope we can add them instead

I believe that would be the preferred way to proceed and I'll be supportive of the new fields if there's a use case.

[EmilienM]

This is part of the v1beta1 list of things we want to take care, so please let us know. If the answer is " lack of fields in the current API", let's just add them in Gophercloud+CAPO and move on with life.

[mdbooth]

This is part of the v1beta1 list of things we want to take care, so please let us know. If the answer is " lack of fields in the current API", let's just add them in Gophercloud+CAPO and move on with life.

But soon! I don't want to hold v1beta1 up for this.

@lentzi90 As it remains unclear for now, would you be ok if we removed it before v1beta1 anyway? In the worst case where we discover it is needed in its current form we can safely add it back without a version bump. However, in the best case where we add specific fields to the API instead we'll have dropped a deprecated field which doesn't do anything. For now, the default position is that it stays.

[lentzi90]

I want to fix it right away, if that's alright! I'll do a PR for gophercloud ASAP.

From downstream I basically hear that we would need things to work the same as Heat. There are plugins (not necessarily open source) for Neutron that can be used with Heat because of the value_specs, and now people are expecting CAPO to be able to do the same. So we are after a certain parity in what CAPO and Heat can do.

[EmilienM]

Fix in gophercloud: https://github.com/gophercloud/gophercloud/pull/2886

[mdbooth]

As we're not removing it, this is no longer a v1beta1 blocker.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale