Skip to content

✨ Add feature to create ports with custom options #876

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.

Sign up for GitHub

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

Merged
k8s-ci-robot merged 1 commit into from
Jun 8, 2021
+818 −92
Merged

✨ Add feature to create ports with custom options #876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions api/v1alpha3/conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,39 @@ func Convert_v1alpha3_OpenStackClusterSpec_To_v1alpha4_OpenStackClusterSpec(in *
func Convert_v1alpha3_OpenStackMachineSpec_To_v1alpha4_OpenStackMachineSpec(in *OpenStackMachineSpec, out *v1alpha4.OpenStackMachineSpec, s conversion.Scope) error {
return autoConvert_v1alpha3_OpenStackMachineSpec_To_v1alpha4_OpenStackMachineSpec(in, out, s)
}

// Convert_v1alpha3_OpenStackClusterSpec_To_v1alpha4_OpenStackClusterSpec has to be added by us for the new portOpts
// parameter in v1alpha4. There is no intention to support this parameter in v1alpha3, so the field is just dropped.
func Convert_v1alpha4_Network_To_v1alpha3_Network(in *v1alpha4.Network, out *Network, s conversion.Scope) error {
Copy link
Copy Markdown

@jan-est jan-est May 31, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I think this should be like it says in the comment Convert_v1alpha3_OpenStackClusterSpec_To_v1alpha4_OpenStackClusterSpec, I think we don't need specific conversion function for Network since portOpts is part of the OpenStackMachineSpec. We need to be able to convert OpenStackMachineSpec both ways, from v1alpha4 to v1alpha3 and vice versa.

I am pretty sure when changing between api versions you just can't drop any fields that are not supported in another. You need to store and restore these field during conversion.

Copy link
Copy Markdown
Contributor Author

@macaptain macaptain Jun 3, 2021

Choose a reason for hiding this comment

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

I see that we'll drop fields if we go v1alpha4 -> v1alpha3 -> v1alpha4. This will also apply to other fields added/removed in v1alpha4 that aren't part of this change (e.g. .UserDataSecret, useOctavia). Can we raise and fix robust conversion between versions as a separate issue outside the scope of this change?

Happy to accept any suggestions on improving or fixing these conversion functions that I have written, because I have largely guessed at what they are supposed to be in order to get the code generation passing without warnings or errors.

return autoConvert_v1alpha4_Network_To_v1alpha3_Network(in, out, s)
}

// Convert_v1alpha3_OpenStackClusterSpec_To_v1alpha4_OpenStackClusterSpec has to be added by us for the new ports
// parameter in v1alpha4. There is no intention to support this parameter in v1alpha3, so the field is just dropped.
func Convert_v1alpha4_OpenStackMachineSpec_To_v1alpha3_OpenStackMachineSpec(in *v1alpha4.OpenStackMachineSpec, out *OpenStackMachineSpec, s conversion.Scope) error {
return autoConvert_v1alpha4_OpenStackMachineSpec_To_v1alpha3_OpenStackMachineSpec(in, out, s)
}

func Convert_Slice_v1alpha4_Network_To_Slice_v1alpha3_Network(in *[]v1alpha4.Network, out *[]Network, s conversion.Scope) error {
for i := range *in {
inNet := &(*in)[i]
outNet := new(Network)
if err := autoConvert_v1alpha4_Network_To_v1alpha3_Network(inNet, outNet, s); err != nil {
return err
}
*out = append(*out, *outNet)
}
return nil
}

func Convert_Slice_v1alpha3_Network_To_Slice_v1alpha4_Network(in *[]Network, out *[]v1alpha4.Network, s conversion.Scope) error {
for i := range *in {
inNet := &(*in)[i]
outNet := new(v1alpha4.Network)
if err := autoConvert_v1alpha3_Network_To_v1alpha4_Network(inNet, outNet, s); err != nil {
return err
}
*out = append(*out, *outNet)
}
return nil
}
122 changes: 94 additions & 28 deletions api/v1alpha3/zz_generated.conversion.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion api/v1alpha4/openstackmachine_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,13 @@ type OpenStackMachineSpec struct {
SSHKeyName string `json:"sshKeyName,omitempty"`

// A networks object. Required parameter when there are multiple networks defined for the tenant.
// When you do not specify the networks parameter, the server attaches to the only network created for the current tenant.
// When you do not specify both networks and ports parameters, the server attaches to the only network created for the current tenant.
Networks []NetworkParam `json:"networks,omitempty"`

// Ports to be attached to the server instance. They are created if a port with the given name does not already exist.
// When you do not specify both networks and ports parameters, the server attaches to the only network created for the current tenant.
Ports []PortOpts `json:"ports,omitempty"`

// UUID, IP address of a port from this subnet will be marked as AccessIPv4 on the created compute instance
Subnet string `json:"subnet,omitempty"`

Expand Down
41 changes: 37 additions & 4 deletions api/v1alpha4/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ type NetworkParam struct {
// The UUID of the network. Required if you omit the port attribute.
UUID string `json:"uuid,omitempty"`
// A fixed IPv4 address for the NIC.
FixedIP string `json:"fixedIp,omitempty"`
FixedIP string `json:"fixedIP,omitempty"`
// Filters for optional network query
Filter Filter `json:"filter,omitempty"`
// Subnet within a network to use
Expand Down Expand Up @@ -116,6 +116,38 @@ type SubnetFilter struct {
NotTagsAny string `json:"notTagsAny,omitempty"`
}

type PortOpts struct {
Copy link
Copy Markdown
Contributor

@jichenjc jichenjc Jun 7, 2021

Choose a reason for hiding this comment

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

as follow up, we may need consider add some samples in how to use this opts (not this PR)

Copy link
Copy Markdown
Contributor Author

@macaptain macaptain Jun 7, 2021

Choose a reason for hiding this comment

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

I can certainly add additional documentation in a follow up PR.

// ID of the OpenStack network on which to create the port. If unspecified, create the port on the default cluster network.
NetworkID string `json:"networkId,omitempty"`
// Used to make the name of the port unique. If unspecified, instead the 0-based index of the port in the list is used.
NameSuffix string `json:"nameSuffix,omitempty"`
Description string `json:"description,omitempty"`
AdminStateUp *bool `json:"adminStateUp,omitempty"`
MACAddress string `json:"macAddress,omitempty"`
// Specify pairs of subnet and/or IP address. These should be subnets of the network with the given NetworkID.
FixedIPs []FixedIP `json:"fixedIPs,omitempty"`
TenantID string `json:"tenantId,omitempty"`
ProjectID string `json:"projectId,omitempty"`
SecurityGroups *[]string `json:"securityGroups,omitempty"`
AllowedAddressPairs []AddressPair `json:"allowedAddressPairs,omitempty"`

// The ID of the host where the port is allocated
HostID string `json:"hostId,omitempty"`

// The virtual network interface card (vNIC) type that is bound to the neutron port.
VNICType string `json:"vnicType,omitempty"`
}

type FixedIP struct {
SubnetID string `json:"subnetId"`
Copy link
Copy Markdown
Contributor Author

@macaptain macaptain Jun 7, 2021

Choose a reason for hiding this comment

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

One thought: it would be possible here is to make the type SubnetParam instead of specifying the subnet only with SubnetID. Then (with additional code) it would be possible to specify this subnet with a filter, like the networks can be specified with a filter. Would that be desirable? It would be different from the OpenShift interface. I also think it would be possible to add a SubnetFilter as an additional optional field in the future if we wanted.

Copy link
Copy Markdown
Contributor

@iamemilio iamemilio Jun 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

That would be cool. That is probably a good idea for a lot of these fields that only take IDs as an input, but might be content for another iteration

IPAddress string `json:"ipAddress,omitempty"`
}

type AddressPair struct {
IPAddress string `json:"ipAddress,omitempty"`
MACAddress string `json:"macAddress,omitempty"`
}

type Instance struct {
ID string `json:"id,omitempty"`
Name string `json:"name,omitempty"`
Expand Down Expand Up @@ -145,16 +177,17 @@ type RootVolume struct {
Size int `json:"diskSize,omitempty"`
}

// Network represents basic information about the associated OpenStach Neutron Network.
// Network represents basic information about an OpenStack Neutron Network associated with an instance's port.
type Network struct {
Name string `json:"name"`
ID string `json:"id"`

//+optional
Tags []string `json:"tags,omitempty"`

Subnet *Subnet `json:"subnet,omitempty"`
Router *Router `json:"router,omitempty"`
Subnet *Subnet `json:"subnet,omitempty"`
PortOpts *PortOpts `json:"port,omitempty"`
Router *Router `json:"router,omitempty"`

// Be careful when using APIServerLoadBalancer, because this field is optional and therefore not
// set in all cases
Expand Down
Loading