Skip to content

Commit 456b330

Browse files
jfpucheuclaude
andcommitted
feat: make allowedAddressPairs on OpenStackMachine ports mutable
Neutron ports support updating allowedAddressPairs without recreating the server. This change allows operators to update allowedAddressPairs on existing machines by patching the OpenStackMachineTemplate, without triggering a rolling replacement of the nodes. Changes: - OpenStackMachineTemplate webhook: exclude allowedAddressPairs from the immutability check so the field can be updated in-place. - OpenStackMachine webhook: same exclusion to allow the controller to annotate OSMs without being rejected. - OpenStackMachine controller: deep-copy ports before mutation to avoid corrupting the original spec through shared slice backing arrays, which would trigger the spec-immutability webhook. - Networking service: add UpdateAllowedAddressPairs() to call ports.Update on the Neutron API. - OpenStackMachineTemplate controller: add reconcileAllowedAddressPairs which walks MachineSets → Machines → OpenStackMachines → OpenStackServers and calls UpdateAllowedAddressPairs() for each provisioned port. Idempotency is tracked via an annotation on each OpenStackMachine so only a metadata-only patch is needed, avoiding the spec-immutability webhook entirely. - RBAC: add machinesets/machines/openstackmachines get/list/watch/patch permissions to the OSMT controller. - Unit tests for reconcileAllowedAddressPairs. - E2E test: creates a cluster with 1 CP and 1 worker (via a dedicated MachineDeployment with an explicit port), then patches the OSMT three times to accumulate allowedAddressPairs and verifies via the Neutron API after each patch that the port is updated without server restart. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 7d3ae9e commit 456b330

File tree

8 files changed

+602
-10
lines changed

8 files changed

+602
-10
lines changed

config/rbac/role.yaml

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

controllers/openstackmachine_controller.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -556,9 +556,17 @@ func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.Ope
556556

557557
// If not ports are provided we create one.
558558
// Ports must have a network so if none is provided we use the default network.
559-
serverPorts := openStackMachineSpec.Ports
559+
// Deep-copy the ports so in-place mutations below (Network injection, SecurityGroups
560+
// merge) do not modify the original OpenStackMachineSpec referenced by the patchHelper
561+
// baseline, which would cause a spurious spec-immutability webhook rejection.
562+
var serverPorts []infrav1.PortOpts
560563
if len(openStackMachineSpec.Ports) == 0 {
561564
serverPorts = make([]infrav1.PortOpts, 1)
565+
} else {
566+
serverPorts = make([]infrav1.PortOpts, len(openStackMachineSpec.Ports))
567+
for i, p := range openStackMachineSpec.Ports {
568+
serverPorts[i] = *p.DeepCopy()
569+
}
562570
}
563571

564572
var clusterSubnets []infrav1.FixedIP

controllers/openstackmachinetemplate_controller.go

Lines changed: 163 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package controllers
1717

1818
import (
1919
"context"
20+
"encoding/json"
2021
"errors"
2122

2223
corev1 "k8s.io/api/core/v1"
@@ -25,6 +26,7 @@ import (
2526
kerrors "k8s.io/apimachinery/pkg/util/errors"
2627
"k8s.io/client-go/tools/record"
2728
clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1"
29+
clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
2830
"sigs.k8s.io/cluster-api/util"
2931
"sigs.k8s.io/cluster-api/util/annotations"
3032
v1beta1conditions "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/conditions"
@@ -35,15 +37,25 @@ import (
3537
"sigs.k8s.io/controller-runtime/pkg/controller"
3638

3739
infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
40+
infrav1alpha1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha1"
3841
"sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/compute"
42+
"sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking"
3943
"sigs.k8s.io/cluster-api-provider-openstack/pkg/scope"
4044
controllers "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/controllers"
4145
)
4246

43-
const imagePropertyForOS = "os_type"
47+
const (
48+
imagePropertyForOS = "os_type"
49+
50+
// annotationAllowedAddressPairs tracks the last-applied allowedAddressPairs per
51+
// OpenStackMachine (stored as JSON). Written as a metadata annotation so we never
52+
// touch the immutable OSM spec, which would trigger the spec-immutability webhook.
53+
annotationAllowedAddressPairs = "infrastructure.cluster.x-k8s.io/osmt-allowed-address-pairs"
54+
)
4455

4556
// Set here so we can easily mock it in tests.
4657
var newComputeService = compute.NewService
58+
var newNetworkingService = networking.NewService
4759

4860
// OpenStackMachineTemplateReconciler reconciles a OpenStackMachineTemplate object.
4961
// it only updates the .status field to allow auto-scaling.
@@ -57,6 +69,9 @@ type OpenStackMachineTemplateReconciler struct {
5769

5870
// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=openstackmachinetemplates,verbs=get;list;watch
5971
// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=openstackmachinetemplates/status,verbs=get;update;patch
72+
// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=openstackmachines,verbs=get;list;watch;patch
73+
// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinesets,verbs=get;list;watch
74+
// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machines,verbs=get;list;watch
6075

6176
func (r *OpenStackMachineTemplateReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, reterr error) {
6277
log := ctrl.LoggerFrom(ctx)
@@ -132,14 +147,14 @@ func (r *OpenStackMachineTemplateReconciler) Reconcile(ctx context.Context, req
132147
scope := scope.NewWithLogger(clientScope, log)
133148

134149
// Handle non-deleted OpenStackMachineTemplates
135-
if err := r.reconcileNormal(ctx, scope, openStackMachineTemplate); err != nil {
150+
if err := r.reconcileNormal(ctx, scope, cluster.Name, openStackMachineTemplate); err != nil {
136151
return ctrl.Result{}, err
137152
}
138153
log.V(4).Info("Successfully reconciled OpenStackMachineTemplate")
139154
return ctrl.Result{}, nil
140155
}
141156

142-
func (r *OpenStackMachineTemplateReconciler) reconcileNormal(ctx context.Context, scope *scope.WithLogger, openStackMachineTemplate *infrav1.OpenStackMachineTemplate) (reterr error) {
157+
func (r *OpenStackMachineTemplateReconciler) reconcileNormal(ctx context.Context, scope *scope.WithLogger, clusterName string, openStackMachineTemplate *infrav1.OpenStackMachineTemplate) (reterr error) {
143158
log := scope.Logger()
144159

145160
computeService, err := newComputeService(scope)
@@ -195,6 +210,9 @@ func (r *OpenStackMachineTemplateReconciler) reconcileNormal(ctx context.Context
195210
if err != nil {
196211
return err
197212
}
213+
if imageID == nil {
214+
return nil
215+
}
198216

199217
image, err := computeService.GetImageDetails(*imageID)
200218
if err != nil {
@@ -211,9 +229,151 @@ func (r *OpenStackMachineTemplateReconciler) reconcileNormal(ctx context.Context
211229
}
212230
}
213231

232+
if err := r.reconcileAllowedAddressPairs(ctx, scope, clusterName, openStackMachineTemplate); err != nil {
233+
return err
234+
}
235+
236+
return nil
237+
}
238+
239+
// reconcileAllowedAddressPairs updates the allowedAddressPairs on existing Neutron ports
240+
// to match what is defined in the OpenStackMachineTemplate.
241+
// Idempotency is tracked via an annotation on each OpenStackMachine so that only a
242+
// metadata-only patch is needed — this avoids touching the immutable OSM spec.
243+
func (r *OpenStackMachineTemplateReconciler) reconcileAllowedAddressPairs(ctx context.Context, scope *scope.WithLogger, clusterName string, openStackMachineTemplate *infrav1.OpenStackMachineTemplate) error {
244+
log := scope.Logger()
245+
246+
if len(openStackMachineTemplate.Spec.Template.Spec.Ports) == 0 || clusterName == "" {
247+
return nil
248+
}
249+
250+
// Build the desired state as JSON for idempotency comparison.
251+
type portPairs = []infrav1.AddressPair
252+
templatePorts := openStackMachineTemplate.Spec.Template.Spec.Ports
253+
desired := make([]portPairs, len(templatePorts))
254+
for i, p := range templatePorts {
255+
desired[i] = p.AllowedAddressPairs
256+
}
257+
desiredJSON, err := json.Marshal(desired)
258+
if err != nil {
259+
return err
260+
}
261+
desiredStr := string(desiredJSON)
262+
263+
// List MachineSets in the namespace for this cluster.
264+
machineSetList := &clusterv1.MachineSetList{}
265+
if err := r.Client.List(ctx, machineSetList,
266+
client.InNamespace(openStackMachineTemplate.Namespace),
267+
client.MatchingLabels{clusterv1.ClusterNameLabel: clusterName},
268+
); err != nil {
269+
return err
270+
}
271+
272+
// List Machines in the namespace for this cluster.
273+
machineList := &clusterv1.MachineList{}
274+
if err := r.Client.List(ctx, machineList,
275+
client.InNamespace(openStackMachineTemplate.Namespace),
276+
client.MatchingLabels{clusterv1.ClusterNameLabel: clusterName},
277+
); err != nil {
278+
return err
279+
}
280+
281+
// Networking service is initialised lazily on first actual port update.
282+
var networkingService *networking.Service
283+
284+
for i := range machineSetList.Items {
285+
ms := &machineSetList.Items[i]
286+
if ms.Spec.Template.Spec.InfrastructureRef.Name != openStackMachineTemplate.Name {
287+
continue
288+
}
289+
290+
for j := range machineList.Items {
291+
machine := &machineList.Items[j]
292+
if !isOwnedByMachineSet(machine, ms) {
293+
continue
294+
}
295+
infraName := machine.Spec.InfrastructureRef.Name
296+
if infraName == "" {
297+
continue
298+
}
299+
300+
osm := &infrav1.OpenStackMachine{}
301+
if err := r.Client.Get(ctx, client.ObjectKey{
302+
Namespace: openStackMachineTemplate.Namespace,
303+
Name: infraName,
304+
}, osm); err != nil {
305+
if apierrors.IsNotFound(err) {
306+
continue
307+
}
308+
return err
309+
}
310+
311+
// Skip if annotation already reflects the desired state.
312+
if osm.Annotations[annotationAllowedAddressPairs] == desiredStr {
313+
continue
314+
}
315+
316+
// Port IDs are stored in the OpenStackServer status (same name as the OSM).
317+
openStackServer := &infrav1alpha1.OpenStackServer{}
318+
if err := r.Client.Get(ctx, client.ObjectKey{
319+
Namespace: openStackMachineTemplate.Namespace,
320+
Name: infraName,
321+
}, openStackServer); err != nil {
322+
if apierrors.IsNotFound(err) {
323+
continue
324+
}
325+
return err
326+
}
327+
328+
if openStackServer.Status.Resources == nil || len(openStackServer.Status.Resources.Ports) == 0 {
329+
continue
330+
}
331+
332+
if networkingService == nil {
333+
networkingService, err = newNetworkingService(scope)
334+
if err != nil {
335+
return err
336+
}
337+
}
338+
339+
for portIdx, portStatus := range openStackServer.Status.Resources.Ports {
340+
if portIdx >= len(templatePorts) {
341+
break
342+
}
343+
pairs := templatePorts[portIdx].AllowedAddressPairs
344+
log.Info("Updating allowedAddressPairs on port", "portID", portStatus.ID,
345+
"machine", osm.Name, "portIndex", portIdx)
346+
if err := networkingService.UpdateAllowedAddressPairs(portStatus.ID, pairs); err != nil {
347+
log.Error(err, "Failed to update allowedAddressPairs", "portID", portStatus.ID)
348+
return err
349+
}
350+
}
351+
352+
// Record the applied state in an annotation (metadata-only patch).
353+
osmCopy := osm.DeepCopy()
354+
if osm.Annotations == nil {
355+
osm.Annotations = map[string]string{}
356+
}
357+
osm.Annotations[annotationAllowedAddressPairs] = desiredStr
358+
if err := r.Client.Patch(ctx, osm, client.MergeFrom(osmCopy)); err != nil {
359+
return err
360+
}
361+
}
362+
}
363+
214364
return nil
215365
}
216366

367+
// isOwnedByMachineSet returns true if the Machine has an owner reference pointing to ms.
368+
func isOwnedByMachineSet(machine *clusterv1.Machine, ms *clusterv1.MachineSet) bool {
369+
for _, ref := range machine.OwnerReferences {
370+
if ref.Kind == "MachineSet" && ref.Name == ms.Name {
371+
return true
372+
}
373+
}
374+
return false
375+
}
376+
217377
func (r *OpenStackMachineTemplateReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
218378
log := ctrl.LoggerFrom(ctx)
219379

0 commit comments

Comments
 (0)