migrate mcm-provider from openstack to stackit by aniruddha2000 · Pull Request #272 · stackitcloud/gardener-extension-provider-stackit

aniruddha2000 · 2026-08-26T09:25:34Z

How to categorize this PR?

/kind enhancement

What this PR does / why we need it:

This PR replaces mcm provider from openstack to stackit. Replaces openstack provider ID with stackit provider ID and add stackit.cloud/migrated-machine: true annotation to machines and puts annotation stackit.cloud/machine-controller-manager-migrated: true to workers once the migration is done.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Breaking changes:

ske-prow · 2026-08-26T09:25:41Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign aniruddha2000 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

aniruddha2000 · 2026-08-31T13:22:09Z

/retest-required

aniruddha2000 · 2026-08-31T13:22:25Z

/retest

aniruddha2000 · 2026-08-31T13:23:04Z

/retest

aniruddha2000 · 2026-08-31T13:25:25Z

/retest

aniruddha2000 · 2026-08-31T20:49:16Z

/retest

aniruddha2000 · 2026-08-31T20:50:00Z

/test pull-gardener-extension-provider-stackit-dev-artifacts

aniruddha2000 · 2026-08-31T20:50:41Z

/test all

aniruddha2000 · 2026-09-07T15:04:55Z

/test pull-gardener-extension-provider-stackit-integration-tests

# Conflicts: # pkg/controller/worker/machines.go

stackit-ske-bot

SKE Code Review

Architectural Feedback

Inverted Migration Transaction Flow & Redundant Patch Overhead: In pkg/controller/worker/machines.go#L456-L499, each machine undergoes three separate Kubernetes API Patch calls (shouldMigrateMachineAnnotation + migratedMachineAnnotation, then Spec.ProviderID, then removing shouldMigrateMachineAnnotation). This transaction complexity and ephemeral annotation were introduced because Spec.ProviderID is updated in Kubernetes before confirming that the STACKIT IaaS UpdateServer call succeeded. Reversing the sequence—executing UpdateServer (an idempotent operation) on STACKIT IaaS first, and then executing a single atomic Kubernetes Patch updating both Spec.ProviderID and migratedMachineAnnotation—eliminates the ephemeral shouldMigrateMachineAnnotation, reduces Kubernetes API patch traffic by 66% during node migrations, and avoids leaving machine resources in a state where Spec.ProviderID points to STACKIT while the remote server labels were never updated.
Unconditional IaaS Client Initialization in WorkerDelegate: In pkg/controller/worker/actuator.go#L76-L80, stackitClient.IaaS(ctx, d.seedClient, worker.Spec.SecretRef) is called unconditionally on every Worker reconciliation. Because w.iaaSClient is exclusively used by migrateMachines (which only executes when feature.MigrateStackitMachineControllerManager(cluster) is enabled and the worker has not yet migrated), Shoot clusters that do not use MCM migration or OpenStack clusters whose secret lacks STACKIT service account credentials (saKeyJSON) will fail secret credential parsing in stackit.GetCredentialsFromSecretRef, breaking Worker reconciliation. The IaaS client should only be instantiated when feature.MigrateStackitMachineControllerManager(cluster) && worker.Annotations[workerMigratedAnnotation] != "true", or lazily on demand inside migrateMachines.

Findings & Feedback

All findings and concrete recommendations have been provided as inline code suggestions above.

Verdict

Comment

breuerfelix · 2026-09-21T11:02:48Z

+		// It is okay to skip machine without a provider ID, as there is a fallback to get
+		// the server by name in case there is no providerID during deletion by the MCM.
+		// Normally this is done with a label containing the machine name and a label selector.
+		// In case of a migrated machine with the stackit.cloud/migrated-machine annotation the deletion needs


this is wrong, since we have no provider id, we cannot update the labels.
in case the provider id is missing, we will fetch all servers and filter by name

breuerfelix · 2026-09-21T11:04:44Z

+		}
+		// This annotation is deleted when the server is updated, otherwise its incomplete migrated machine.
+		m.Annotations[shouldMigrateMachineAnnotation] = "true"
+		// The MCM needs to get and delete the NICs of the machine, as they were created separately which needs dedicated deletion


Suggested change

// The MCM needs to get and delete the NICs of the machine, as they were created separately which needs dedicated deletion

// The MCM needs to get and delete the NICs of the machine, as they were created separately which needs dedicated deletion. The MCM used this annotation as a marker for the said NIC cleanup.

ske-prow Bot added the kind/enhancement Enhancement, improvement, extension label Aug 26, 2026

ske-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 26, 2026

aniruddha2000 force-pushed the migrate-mcm branch from 67e72b1 to dcf82cb Compare August 26, 2026 09:26

ske-prow Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 28, 2026

aniruddha2000 force-pushed the migrate-mcm branch from b696375 to 25fa93d Compare August 31, 2026 13:14

ske-prow Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 31, 2026

aniruddha2000 force-pushed the migrate-mcm branch from 81f51f4 to af6bcea Compare September 7, 2026 11:52

nschad reviewed Sep 9, 2026

View reviewed changes

nschad requested changes Sep 10, 2026

View reviewed changes

Comment thread pkg/controller/worker/machines.go Outdated

Comment thread pkg/controller/worker/machines.go Outdated

Comment thread pkg/controller/worker/machines.go Outdated

Comment thread pkg/controller/worker/machines.go Outdated

Comment thread pkg/controller/worker/machines.go Outdated

aniruddha2000 added 12 commits September 10, 2026 15:47

migrate mcm provider from openstack to stackit

8b86252

# Conflicts: # pkg/controller/worker/machines.go

add feature gates

01a1b39

add worker migrated annotation

1837d0f

add small check to not trigger migration when done

c47646a

minor nits

504056b

add diagnosis

462effd

trigger build

952006b

run ci

6a753b1

add nil stackit client to the tests

a67922d

remove diagnosis

8f4dd16

add poc test for the machine migration

5241e32

fix mocks in the tests

b32b095

aniruddha2000 added 9 commits September 10, 2026 15:47

add improvements to the test

a73bc4b

fix the test and its running

385baa8

add iaas failure tolerance testing

51b27e6

make fmt

320d8bf

make check fix<

ec219a7

make check fix

0733734

fix test

95619d8

fix test

2b1a268

resolve review

a712745

aniruddha2000 force-pushed the migrate-mcm branch from b636923 to a712745 Compare September 10, 2026 13:47

aniruddha2000 added 2 commits September 10, 2026 15:48

fix test

c44e976

use named capture groups in the regex

f66ecdc

nschad reviewed Sep 11, 2026

View reviewed changes

Comment thread pkg/controller/worker/machines.go

move regex compile to init func

84dd68b

aniruddha2000 requested a review from stackit-ske-bot September 16, 2026 10:47

stackit-ske-bot reviewed Sep 16, 2026

View reviewed changes

Comment thread pkg/controller/worker/machines.go Outdated

Comment thread pkg/controller/worker/machines.go

Comment thread pkg/controller/worker/machines.go

Comment thread pkg/controller/worker/machines.go Outdated

resolve AI suggestions

49091b3

breuerfelix reviewed Sep 17, 2026

View reviewed changes

Comment thread pkg/controller/worker/machines.go

resolve AI suggestions

1624513

breuerfelix reviewed Sep 17, 2026

View reviewed changes

Comment thread pkg/controller/worker/machines.go Outdated

revert provider id is empty review

84af27b

breuerfelix reviewed Sep 18, 2026

View reviewed changes

aniruddha2000 added 2 commits September 18, 2026 15:09

update labels and make nits

88d9ba2

fix test

183cd32

breuerfelix reviewed Sep 21, 2026

View reviewed changes

Comment thread pkg/controller/worker/machines.go

	// The MCM needs to get and delete the NICs of the machine, as they were created separately which needs dedicated deletion
	// The MCM needs to get and delete the NICs of the machine, as they were created separately which needs dedicated deletion. The MCM used this annotation as a marker for the said NIC cleanup.

Conversation

aniruddha2000 commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ske-prow Bot commented Aug 26, 2026

Uh oh!

aniruddha2000 commented Aug 31, 2026

Uh oh!

aniruddha2000 commented Aug 31, 2026

Uh oh!

aniruddha2000 commented Aug 31, 2026

Uh oh!

aniruddha2000 commented Aug 31, 2026

Uh oh!

aniruddha2000 commented Aug 31, 2026

Uh oh!

aniruddha2000 commented Aug 31, 2026

Uh oh!

aniruddha2000 commented Aug 31, 2026

Uh oh!

aniruddha2000 commented Sep 7, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stackit-ske-bot left a comment

Choose a reason for hiding this comment

SKE Code Review

Architectural Feedback

Findings & Feedback

Verdict

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

breuerfelix Sep 21, 2026

Choose a reason for hiding this comment

Uh oh!

breuerfelix Sep 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aniruddha2000 commented Aug 26, 2026 •

edited

Loading