Skip to content

Add SubnetIDs field to support multiple subnets for machine instances #347

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

Open
axel7born wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add SubnetIDs field to support multiple subnets for machine instances #347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
651c0a3
Add SubnetIDs field to support multiple subnets for machine instances
axel7born Dec 18, 2025
1d896b4
make SubnetIDs optional
axel7born Jan 8, 2026
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
30 changes: 28 additions & 2 deletions hack/api-reference/api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,20 @@ string
</td>
<td>
<em>(Optional)</em>
<p>SubnetID is the ID of the subnet the instance should belong to. If SubnetID is not specified</p>
<p>SubnetID is the ID of the subnet the instance should belong to.
Deprecated - use <code>SubnetIDs</code> instead.</p>
</td>
</tr>
<tr>
<td>
<code>subnetIDs</code></br>
<em>
[]string
</em>
</td>
<td>
<em>(Optional)</em>
<p>SubnetIDs is a list of IDs of the subnets the instance should belong to.</p>
</td>
</tr>
<tr>
Expand Down Expand Up @@ -363,7 +376,20 @@ string
</td>
<td>
<em>(Optional)</em>
<p>SubnetID is the ID of the subnet the instance should belong to. If SubnetID is not specified</p>
<p>SubnetID is the ID of the subnet the instance should belong to.
Deprecated - use <code>SubnetIDs</code> instead.</p>
</td>
</tr>
<tr>
<td>
<code>subnetIDs</code></br>
<em>
[]string
</em>
</td>
<td>
<em>(Optional)</em>
<p>SubnetIDs is a list of IDs of the subnets the instance should belong to.</p>
</td>
</tr>
<tr>
Expand Down
5 changes: 4 additions & 1 deletion pkg/apis/openstack/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,11 @@ type MachineProviderConfigSpec struct {
Tags map[string]string
// NetworkID is the ID of the network the instance should belong to.
NetworkID string
// SubnetID is the ID of the subnet the instance should belong to. If SubnetID is not specified
// SubnetID is the ID of the subnet the instance should belong to.
// Deprecated - use `SubnetIDs` instead.
SubnetID *string
// SubnetIDs is a list of IDs of the subnets the instance should belong to.
SubnetIDs []string
// PodNetworkCidr is the CIDR range for the pods assigned to this instance.
// Deprecated - use `PodNetworkCIDRs` instead.
PodNetworkCidr string
Expand Down
6 changes: 5 additions & 1 deletion pkg/apis/openstack/v1alpha1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,13 @@ type MachineProviderConfigSpec struct {
Tags map[string]string `json:"tags,omitempty"`
// NetworkID is the ID of the network the instance should belong to.
NetworkID string `json:"networkID"`
// SubnetID is the ID of the subnet the instance should belong to. If SubnetID is not specified
// SubnetID is the ID of the subnet the instance should belong to.
// Deprecated - use `SubnetIDs` instead.
// +optional
SubnetID *string `json:"subnetID,omitempty"`
// SubnetIDs is a list of IDs of the subnets the instance should belong to.
// +optional
SubnetIDs []string `json:"subnetIDs,omitempty"`
Copy link
Contributor

@hebelsan hebelsan Jan 7, 2026

Choose a reason for hiding this comment

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

SubnetIDs is optional, right?
It would probably also make sense to mention that SubnetID and SubnetIDs are currently merged.

Also, the doc comment on SubnetID looks odd / unfinished.
Should we deprecate SubnetID and add a TODO noting its removal in, say, three releases?

Copy link
Author

@axel7born axel7born Jan 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

SubnetIDs is optional, right? It would probably also make sense to mention that SubnetID and SubnetIDs are currently merged.

Yes, SubnetIDs is optional.

Also, the doc comment on SubnetID looks odd / unfinished. Should we deprecate SubnetID and add a TODO noting its removal in, say, three releases?

I deleted the the unfinished sentence. In provider-openstack SubnetID is always set. I have no idea what should happen if it's omitted.
I'm not sure about a deprecation. For me it would be fine, if we have both for some time.

Copy link
Contributor

@hebelsan hebelsan Jan 8, 2026

Choose a reason for hiding this comment

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

I Just had a quick discussion with @kon-angelo about the deprecation.
If we add the comment (// Deprecated ...) for the SubnetID field now we can remove it in a future release and already switch to using the SubnetIDs field only in the provider extension.

// PodNetworkCidr is the CIDR range for the pods assigned to this instance.
// Deprecated: use PodNetworkCIDRs instead
// +optional
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/openstack/v1alpha1/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.

5 changes: 5 additions & 0 deletions pkg/apis/openstack/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

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

5 changes: 5 additions & 0 deletions pkg/apis/openstack/zz_generated.deepcopy.go
View file
Open in desktop

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

34 changes: 32 additions & 2 deletions pkg/driver/executor/executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ func (ex *Executor) getOrCreatePort(ctx context.Context, machineName string) (st
port, err := ex.Network.CreatePort(ctx, &ports.CreateOpts{
Name: machineName,
NetworkID: ex.Config.Spec.NetworkID,
FixedIPs: []ports.IP{{SubnetID: *ex.Config.Spec.SubnetID}},
FixedIPs: ex.buildFixedIPs(),
SecurityGroups: &securityGroupIDs,
})
if err != nil {
Expand All @@ -542,6 +542,32 @@ func (ex *Executor) getOrCreatePort(ctx context.Context, machineName string) (st
return port.ID, nil
}

// buildFixedIPs creates a list of FixedIPs from SubnetID and SubnetIDs, avoiding duplicates
func (ex *Executor) buildFixedIPs() []ports.IP {
// Use a set to track unique subnet IDs and avoid duplicates
subnetIDSet := sets.NewString()

// Add the single SubnetID if specified
if ex.Config.Spec.SubnetID != nil && *ex.Config.Spec.SubnetID != "" {
subnetIDSet.Insert(*ex.Config.Spec.SubnetID)
}

// Add all SubnetIDs if specified
for _, subnetID := range ex.Config.Spec.SubnetIDs {
if subnetID != "" {
subnetIDSet.Insert(subnetID)
}
}

// Convert to []ports.IP
var fixedIPs []ports.IP
for _, subnetID := range subnetIDSet.List() {
fixedIPs = append(fixedIPs, ports.IP{SubnetID: subnetID})
}

return fixedIPs
}

func (ex *Executor) deletePort(ctx context.Context, machineName string) error {
portList, err := ex.Network.ListPorts(ctx, ports.ListOpts{
Name: machineName,
Expand Down Expand Up @@ -698,5 +724,9 @@ func (ex *Executor) listServers(ctx context.Context) ([]servers.Server, error) {

// isUserManagedNetwork returns true if the port used by the machine will be created and managed by MCM.
func (ex *Executor) isUserManagedNetwork() bool {
return !isEmptyString(ptr.To(ex.Config.Spec.NetworkID)) && !isEmptyString(ex.Config.Spec.SubnetID)
hasNetworkID := !isEmptyString(ptr.To(ex.Config.Spec.NetworkID))
hasSubnetID := !isEmptyString(ex.Config.Spec.SubnetID)
hasSubnetIDs := len(ex.Config.Spec.SubnetIDs) > 0

return hasNetworkID && (hasSubnetID || hasSubnetIDs)
}
Loading