-
Notifications
You must be signed in to change notification settings - Fork 261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
⚠️ SubnetFilter to SubnetParam #1971
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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:
Approvers can indicate their approval by writing |
✅ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -125,11 +144,25 @@ type SubnetFilter struct { | |||
CIDR string `json:"cidr,omitempty"` | |||
IPv6AddressMode string `json:"ipv6AddressMode,omitempty"` | |||
IPv6RAMode string `json:"ipv6RAMode,omitempty"` | |||
ID string `json:"id,omitempty"` | |||
|
|||
FilterByNeutronTags `json:",inline"` | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could add comments and some validations here:
- Name and description has max length of 255 in Neutron.
- ProjectID is
uuid.uuid4().hex
which passes K8s UUID validation. - IPVersion is an enum of 4 or 6.
- GatewayIP is IPv4 or IPv6 address.
- CIDR is CIDR.
- IPv6*Mode can be enums, but maybe here it's better to leave them unvalidated, to not require API update on new Neutron features.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that and I deliberately didn't do it because:
- It can easily be a follow-on
- The code-churn would be different to the code churn of this PR
- This is big enough already!
/test pull-cluster-api-provider-openstack-e2e-full-test |
0e15a90
to
826eeda
Compare
/test pull-cluster-api-provider-openstack-e2e-full-test |
Looks like a network-related flake /test pull-cluster-api-provider-openstack-e2e-full-test |
/test pull-cluster-api-provider-openstack-e2e-full-test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/hold cancel |
See #1975
TODO:
/hold