OpenStack: Port additionalSecurityGroups are neither reconciled nor applied to existing ports

[rdebnath-x-ion]

/kind bug

1. What kops version are you running?

master (HEAD). The behaviour is present in released versions that contain the OpenStack Port task as well — the affected code is not new.

2. What Kubernetes version are you running?

N/A — this is a kOps reconciliation bug in the OpenStack provider and is independent of the Kubernetes version.

3. What cloud provider are you using?

OpenStack

4. What commands did you run? What is the simplest way to reproduce this issue?

kops edit ig <ig>          # add or change spec.additionalSecurityGroups
kops update cluster --yes  # expected to attach/detach the group on existing ports

5. What happened after the commands executed?

Nothing changed on the existing ports. kops update cluster reports no port change, and the security groups attached to already-running instances' ports are left untouched. The new additionalSecurityGroups value only takes effect on instances that happen to be recreated later (e.g. via a rolling-update), because groups are applied at port-create time.

6. What did you expect to happen?

kops update cluster --yes should reconcile the port's security groups in place — attaching newly-added additionalSecurityGroups and removing ones no longer desired — without requiring the instance to be recreated.

Background

The OpenStack Port task (upup/pkg/fi/cloudup/openstacktasks/port.go) tracks two distinct sets of security groups:

• SecurityGroups — groups kOps creates and manages itself.
• AdditionalSecurityGroups — extra groups, referenced by name, that the user attaches but kOps does not own.

An OpenStack port exposes a single flat list of security-group IDs, so the task has to split that list into "managed" vs "additional" when reading, and merge the two back into one list when writing. There are two independent bugs at that split/merge boundary.

Bug 1 — the actual state does not reflect reality (drift is invisible)

In newPortTaskFromCloud (the function backing Port.Find), the reconstructed "actual" state simply copies the desired configuration:

actual.AdditionalSecurityGroups = find.AdditionalSecurityGroups

It never checks whether those groups are actually present on the port. As a result, "actual" always equals "expected" for this field, so kOps can never detect that an additional group is missing (or was removed out-of-band) and never schedules a correction.

Bug 2 — changes are never applied to an existing port

Port.RenderOpenstack handles creation, and for existing ports it updates Tags and AllowedAddressPairs — but it has no code path that pushes a security-group change to an existing port. So even when a change is desired, it is silently dropped; the only way a group ends up on a port is at port creation (portCreateOptsFromPortTask), i.e. when the instance is (re)created.

How to reproduce it (precisely)

1. Create a cluster with an instance group that has no (or one) additionalSecurityGroups.
2. Add a security group to spec.additionalSecurityGroups (the group must already exist in OpenStack), then kops update cluster --yes.
3. Inspect a running instance's port: openstack port show <port-id> -c security_group_ids.
4. Observe the new group is not attached. It only appears on instances created after the change (e.g. after a kops rolling-update cluster).

The mirror case: manually detach an additionalSecurityGroups member from a port out-of-band, then kops update cluster --yes — kOps reports no drift and does not re-attach it (Bug 1).

Proposed fix

Two parts, matching the two bugs:

1. In newPortTaskFromCloud, resolve each configured additional group name to its ID and record it in the actual state only if that ID is actually present on the port, so a missing group correctly surfaces as drift.
2. In RenderOpenstack, when the actual vs desired security groups differ, build the merged set of managed group IDs + resolved additional group IDs and call UpdatePort with it. Because OpenStack replaces the entire security-group set on update, both kinds must be sent together.

The change detection compares the actual and expected tasks directly rather than inspecting changes, because BuildChanges copies the expected value into changes for slice fields, making "the user emptied the list" indistinguishable there from "no change".

I have a change ready with unit tests covering both the read (drift detection) and write (in-place UpdatePort) paths, and will open a PR referencing this issue.