search

kubernetes-sigs / cluster-api-provider-openstack

Cluster API implementation for OpenStack
https://cluster-api-openstack.sigs.k8s.io/
Apache License 2.0
283 stars 253 forks source link

⚠️ API cleanup of PortOpts #1914

Closed mdbooth closed 6 months ago

mdbooth commented 6 months ago

Internally, all optional fields become pointers. This change has no direct effect on the CRD, but means that unset values and zero values now have different meanings.

SecurityGroupFilters is renamed to SecurityGroups for consistency with other filter fields used throughout the API. Note that this change thoroughly confuses conversion-gen. Consequently we explicitly disable the conversion of these fields in v1alpha6 and v1alpha5 and do the conversion entirely manually.

/hold

netlify[bot] commented 6 months ago

Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!

Name Link
Latest commit 4c01e655f34771ec92455ee35166aa7eedba84fe
Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-cluster-api-openstack/deploys/65e0769d185d4c0008c7648e
Deploy Preview https://deploy-preview-1914--kubernetes-sigs-cluster-api-openstack.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

mdbooth commented 6 months ago

@lentzi90 @EmilienM This is the first API cleanup change I've extracted. The API change itself is fairly simple. Some of the conversion is a bit hairy, which is mostly specific to this PR.

The new optional package will become a feature though. This allows us to customise the conversion behaviour for specific fields whilst otherwise retaining the defaults in apimachinery.

There will be more like this, but mostly simpler.

EmilienM commented 6 months ago

I really like it! /lgtm

EmilienM commented 6 months ago

/lgtm

k8s-ci-robot commented 6 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lentzi90, mdbooth

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/main/OWNERS)~~ [lentzi90,mdbooth] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
EmilienM commented 6 months ago

/hold cancel

EmilienM commented 6 months ago

/lgtm restoring lgtm, I just rebased on main and ran make generate.

mdbooth commented 6 months ago

/lgtm restoring lgtm, I just rebased on main and ran make generate.

I'm pretty sure this will have fixed it. Not sure why the test run against your most recent push is wedged.

mdbooth commented 6 months ago

/test pull-cluster-api-provider-openstack-test