From cb3402831dae5a99ddd043c4b02077b56c9f20eb Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Wed, 26 Aug 2026 11:24:04 +0200 Subject: [PATCH 01/17] migrate mcm provider from openstack to stackit # Conflicts: # pkg/controller/worker/machines.go --- pkg/controller/worker/actuator.go | 10 ++- pkg/controller/worker/machines.go | 93 +++++++++++++++++++++++++++- pkg/stackit/client/iaas.go | 5 ++ pkg/stackit/client/mock/iaas_mock.go | 15 +++++ 4 files changed, 121 insertions(+), 2 deletions(-) diff --git a/pkg/controller/worker/actuator.go b/pkg/controller/worker/actuator.go index be22a2f5..bcded0d4 100644 --- a/pkg/controller/worker/actuator.go +++ b/pkg/controller/worker/actuator.go @@ -14,6 +14,8 @@ import ( gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" gardener "github.com/gardener/gardener/pkg/client/kubernetes" + openstackclient "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/openstack/client" + "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/client-go/kubernetes" @@ -24,7 +26,7 @@ import ( "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/apis/stackit/helper" stackitv1alpha1 "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/apis/stackit/v1alpha1" - openstackclient "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/openstack/client" + stackitclient "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit/client" ) type delegateFactory struct { @@ -71,6 +73,8 @@ func (d *delegateFactory) WorkerDelegate(ctx context.Context, worker *extensions return nil, err } + stackitClient := stackitclient.New(stackit.DetermineRegion(cluster), cluster) + return NewWorkerDelegate( d.seedClient, d.scheme, @@ -81,6 +85,7 @@ func (d *delegateFactory) WorkerDelegate(ctx context.Context, worker *extensions worker, cluster, d.customLabelDomain, + stackitClient, ) } @@ -102,6 +107,7 @@ type workerDelegate struct { machineImages []stackitv1alpha1.MachineImage openstackClient openstackclient.Factory + stackitClient stackitclient.Factory } // NewWorkerDelegate creates a new context for a worker reconciliation. @@ -115,6 +121,7 @@ func NewWorkerDelegate( worker *extensionsv1alpha1.Worker, cluster *extensionscontroller.Cluster, customLabelDomain string, + stackitClient stackitclient.Factory, ) (genericactuator.WorkerDelegate, error) { config, err := helper.CloudProfileConfigFromCluster(cluster) if err != nil { @@ -133,5 +140,6 @@ func NewWorkerDelegate( cluster: cluster, worker: worker, customLabelDomain: customLabelDomain, + stackitClient: stackitClient, }, nil } diff --git a/pkg/controller/worker/machines.go b/pkg/controller/worker/machines.go index c81ba5fb..cc0cc47f 100644 --- a/pkg/controller/worker/machines.go +++ b/pkg/controller/worker/machines.go @@ -11,6 +11,7 @@ import ( "path/filepath" "regexp" "sort" + "strconv" "strings" extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller" @@ -34,8 +35,12 @@ import ( "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/openstack" "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit" stackitutils "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/utils" + iaas2 "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" ) +const shouldMigrateMachineAnnotation = "stackit.cloud/machine-should-be-migrated" +const migratedMachineAnnotation = "stackit.cloud/migrated-machine" + // MachineClassKind yields the name of the machine class kind used by OpenStack provider. func (w *workerDelegate) MachineClassKind() string { return "MachineClass" @@ -63,7 +68,19 @@ func (w *workerDelegate) DeployMachineClasses(ctx context.Context) error { if feature.UseStackitMachineControllerManager(w.cluster) { chartPath = "machineclass-stackit" } - return w.seedChartApplier.ApplyFromEmbeddedFS(ctx, charts.InternalChart, filepath.Join(charts.InternalChartsPath, chartPath), w.worker.Namespace, "machineclass", kubernetes.Values(map[string]any{"machineClasses": w.machineClasses})) + err := w.seedChartApplier.ApplyFromEmbeddedFS(ctx, charts.InternalChart, filepath.Join(charts.InternalChartsPath, chartPath), w.worker.Namespace, "machineclass", kubernetes.Values(map[string]any{"machineClasses": w.machineClasses})) + if err != nil { + return err + } + + if feature.MigrateStackitMachineControllerManager(w.cluster) { + err = w.migrateMachines(ctx) + if err != nil { + return err + } + } + + return nil } // GenerateMachineDeployments generates the configuration for the desired machine deployments. @@ -395,3 +412,77 @@ func EnsureUniformMachineImages(images []stackitv1alpha1.MachineImage, definitio } return uniformMachineImages } + +func (w *workerDelegate) migrateMachines(ctx context.Context) error { + var allMachines machinev1alpha1.MachineList + var migrateMachines []machinev1alpha1.Machine + + err := w.seedClient.List(ctx, &allMachines, &client.ListOptions{Namespace: w.worker.Namespace}) + if err != nil { + return err + } + + for i := range allMachines.Items { + // ignore error as default is false + migrateAnnotation, _ := strconv.ParseBool(allMachines.Items[i].Annotations[shouldMigrateMachineAnnotation]) + if !strings.HasPrefix(allMachines.Items[i].Spec.ProviderID, "stackit://") || migrateAnnotation { + migrateMachines = append(migrateMachines, allMachines.Items[i]) + } + } + + if len(migrateMachines) == 0 { + // no old openstack machine + return nil + } + + iaas, err := w.stackitClient.IaaS(ctx, w.seedClient, w.worker.Spec.SecretRef) + if err != nil { + return err + } + + for _, m := range migrateMachines { + patchAnnotations := client.MergeFrom(m.DeepCopy()) + m.Annotations[shouldMigrateMachineAnnotation] = "true" + m.Annotations[migratedMachineAnnotation] = "true" + err = w.seedClient.Patch(ctx, &m, patchAnnotations) + if err != nil { + return err + } + + if m.Spec.ProviderID != "" { + providerIDParts := strings.Split(m.Spec.ProviderID, "/") + if len(providerIDParts) == 0 { + return fmt.Errorf("migrateMachines: malformed machine provider ID: %s", m.Spec.ProviderID) + } + serverID := providerIDParts[len(providerIDParts)-1] + + patch := client.MergeFrom(m.DeepCopy()) + m.Spec.ProviderID = fmt.Sprintf("stackit://%s/%s", iaas.ProjectID(), serverID) + err = w.seedClient.Patch(ctx, &m, patch) + if err != nil { + return err + } + + _, err = iaas.UpdateServer(ctx, serverID, iaas2.UpdateServerPayload{ + Labels: map[string]any{ + // // TODO refine labels + "mcm.gardener.cloud/machine": m.Name, + "mcm.gardener.cloud/machineclass": m.Spec.Class.Name, + "mcm.gardener.cloud/role": "node", + }, + }) + if err != nil { + return err + } + } + + patchRemoveMigrationAnnotation := client.MergeFrom(m.DeepCopy()) + delete(m.Annotations, shouldMigrateMachineAnnotation) + err = w.seedClient.Patch(ctx, &m, patchRemoveMigrationAnnotation) + if err != nil { + return err + } + } + + return nil +} diff --git a/pkg/stackit/client/iaas.go b/pkg/stackit/client/iaas.go index 628f1a96..f7605012 100644 --- a/pkg/stackit/client/iaas.go +++ b/pkg/stackit/client/iaas.go @@ -36,6 +36,7 @@ type IaaSClient interface { CreateServer(ctx context.Context, payload iaas.CreateServerPayload) (*iaas.Server, error) DeleteServer(ctx context.Context, serverId string) error + UpdateServer(ctx context.Context, serverId string, payload iaas.UpdateServerPayload) (*iaas.Server, error) GetServerByName(ctx context.Context, name string) ([]iaas.Server, error) CreatePublicIp(ctx context.Context, payload iaas.CreatePublicIPPayload) (*iaas.PublicIp, error) @@ -288,6 +289,10 @@ func (c iaasClient) DeleteServer(ctx context.Context, serverId string) error { return c.Client.DeleteServer(ctx, c.projectID, c.region, serverId).Execute() } +func (c iaasClient) UpdateServer(ctx context.Context, serverId string, payload iaas.UpdateServerPayload) (*iaas.Server, error) { + return c.Client.UpdateServer(ctx, c.projectID, c.region, serverId).UpdateServerPayload(payload).Execute() +} + // GetServerByName finds the first server with the given name. func (c iaasClient) GetServerByName(ctx context.Context, name string) ([]iaas.Server, error) { servers, err := c.Client.ListServers(ctx, c.projectID, c.region).Execute() diff --git a/pkg/stackit/client/mock/iaas_mock.go b/pkg/stackit/client/mock/iaas_mock.go index 3527b584..c62431d7 100644 --- a/pkg/stackit/client/mock/iaas_mock.go +++ b/pkg/stackit/client/mock/iaas_mock.go @@ -379,3 +379,18 @@ func (mr *MockIaaSClientMockRecorder) UpdateSecurityGroupRules(ctx, group, desir mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateSecurityGroupRules", reflect.TypeOf((*MockIaaSClient)(nil).UpdateSecurityGroupRules), ctx, group, desiredRules, allowDelete) } + +// UpdateServer mocks base method. +func (m *MockIaaSClient) UpdateServer(ctx context.Context, serverId string, payload v2api.UpdateServerPayload) (*v2api.Server, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateServer", ctx, serverId, payload) + ret0, _ := ret[0].(*v2api.Server) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateServer indicates an expected call of UpdateServer. +func (mr *MockIaaSClientMockRecorder) UpdateServer(ctx, serverId, payload any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateServer", reflect.TypeOf((*MockIaaSClient)(nil).UpdateServer), ctx, serverId, payload) +} From e5acd549def9064f276b2e165def31bec634ceea Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Thu, 27 Aug 2026 14:23:57 +0200 Subject: [PATCH 02/17] add feature gates --- pkg/feature/feature.go | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/pkg/feature/feature.go b/pkg/feature/feature.go index e5c4a0a7..681b94f3 100644 --- a/pkg/feature/feature.go +++ b/pkg/feature/feature.go @@ -22,8 +22,12 @@ const ( UseSTACKITAPIInfrastructureController featuregate.Feature = "UseSTACKITAPIInfrastructureController" // UseSTACKITMachineControllerManager Uses the STACKIT machine controller Manager to manage nodes. UseSTACKITMachineControllerManager featuregate.Feature = "UseSTACKITMachineControllerManager" + // MigrateSTACKITMachineControllerManager Migrates the existing openstack machines to stackit. Only works if feature gate UseSTACKITMachineControllerManager is enabled. + MigrateSTACKITMachineControllerManager featuregate.Feature = "MigrateSTACKITMachineControllerManager" // ShootUseSTACKITMachineControllerManager Uses the STACKIT machine controller Manager to manage nodes for a specific Shoot. ShootUseSTACKITMachineControllerManager = "shoot.gardener.cloud/use-stackit-machine-controller-manager" + // ShootMigrateSTACKITMachineControllerManager Migrates the existing openstack machines to stackit. Only works if feature gate UseSTACKITMachineControllerManager is enabled. + ShootMigrateSTACKITMachineControllerManager = "shoot.gardener.cloud/migrate-stackit-machine-controller-manager" // ShootUseSTACKITAPIInfrastructureController Uses the STACKIT API to create the shoot resources instead of OpenStack for a specific Shoot. ShootUseSTACKITAPIInfrastructureController = "shoot.gardener.cloud/use-stackit-api-infrastructure-controller" // EnableSTACKITWorkloadIdentity activates the deployment of the stackit-pod-identity-webhook to enable workload identity injection into pods. @@ -44,11 +48,12 @@ var ( Gate featuregate.FeatureGate = MutableGate allGates = map[featuregate.Feature]featuregate.FeatureSpec{ - EnsureSTACKITLBDeletion: {Default: true, PreRelease: featuregate.Alpha}, - EnsureSTACKITALBDeletion: {Default: false, PreRelease: featuregate.Alpha}, - UseSTACKITAPIInfrastructureController: {Default: true, PreRelease: featuregate.Alpha}, - UseSTACKITMachineControllerManager: {Default: true, PreRelease: featuregate.Alpha}, - EnableSTACKITWorkloadIdentity: {Default: false, PreRelease: featuregate.Alpha}, + EnsureSTACKITLBDeletion: {Default: true, PreRelease: featuregate.Alpha}, + EnsureSTACKITALBDeletion: {Default: false, PreRelease: featuregate.Alpha}, + UseSTACKITAPIInfrastructureController: {Default: true, PreRelease: featuregate.Alpha}, + UseSTACKITMachineControllerManager: {Default: true, PreRelease: featuregate.Alpha}, + EnableSTACKITWorkloadIdentity: {Default: false, PreRelease: featuregate.Alpha}, + MigrateSTACKITMachineControllerManager: {Default: false, PreRelease: featuregate.Alpha}, } ) @@ -81,3 +86,20 @@ func UseStackitAPIInfrastructureController(cluster *extensionscontroller.Cluster } return Gate.Enabled(UseSTACKITAPIInfrastructureController) } + +func MigrateStackitMachineControllerManager(cluster *extensionscontroller.Cluster) bool { + if !UseStackitMachineControllerManager(cluster) { + return false + } + + if cluster != nil && cluster.Shoot != nil { + annotation, ok := cluster.Shoot.Annotations[ShootMigrateSTACKITMachineControllerManager] + if ok { + enabledByAnnotation, err := strconv.ParseBool(annotation) + if err == nil { + return enabledByAnnotation + } + } + } + return Gate.Enabled(MigrateSTACKITMachineControllerManager) +} From 8f45fcdc6ad8787a31afb93c68dbd90b1bf6fcf2 Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Thu, 27 Aug 2026 17:04:54 +0200 Subject: [PATCH 03/17] add worker migrated annotation --- pkg/controller/worker/machines.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/pkg/controller/worker/machines.go b/pkg/controller/worker/machines.go index cc0cc47f..70148655 100644 --- a/pkg/controller/worker/machines.go +++ b/pkg/controller/worker/machines.go @@ -40,6 +40,7 @@ import ( const shouldMigrateMachineAnnotation = "stackit.cloud/machine-should-be-migrated" const migratedMachineAnnotation = "stackit.cloud/migrated-machine" +const workerMigratedAnnotation = "stackit.cloud/machine-controller-manager-migrated" // MachineClassKind yields the name of the machine class kind used by OpenStack provider. func (w *workerDelegate) MachineClassKind() string { @@ -484,5 +485,17 @@ func (w *workerDelegate) migrateMachines(ctx context.Context) error { } } - return nil + return w.markWorkerAsMigrated(ctx) +} + +func (w *workerDelegate) markWorkerAsMigrated(ctx context.Context) error { + patchWorker := client.MergeFrom(w.worker.DeepCopy()) + + if w.worker.Annotations == nil { + w.worker.Annotations = make(map[string]string) + } + + w.worker.Annotations[workerMigratedAnnotation] = "true" + + return w.seedClient.Patch(ctx, w.worker, patchWorker) } From a9c4a2e99dc6c394310ccdfdd0dbea8315823f15 Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Thu, 27 Aug 2026 17:07:14 +0200 Subject: [PATCH 04/17] add small check to not trigger migration when done --- pkg/controller/worker/machines.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/worker/machines.go b/pkg/controller/worker/machines.go index 70148655..e9c89b1c 100644 --- a/pkg/controller/worker/machines.go +++ b/pkg/controller/worker/machines.go @@ -74,7 +74,7 @@ func (w *workerDelegate) DeployMachineClasses(ctx context.Context) error { return err } - if feature.MigrateStackitMachineControllerManager(w.cluster) { + if feature.MigrateStackitMachineControllerManager(w.cluster) && w.worker.Annotations[workerMigratedAnnotation] != "true" { err = w.migrateMachines(ctx) if err != nil { return err From c465664f39697df86ab752e302c194be96c848f6 Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Thu, 27 Aug 2026 17:14:58 +0200 Subject: [PATCH 05/17] minor nits --- pkg/controller/worker/machines.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/controller/worker/machines.go b/pkg/controller/worker/machines.go index e9c89b1c..f0989f92 100644 --- a/pkg/controller/worker/machines.go +++ b/pkg/controller/worker/machines.go @@ -38,9 +38,11 @@ import ( iaas2 "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" ) -const shouldMigrateMachineAnnotation = "stackit.cloud/machine-should-be-migrated" -const migratedMachineAnnotation = "stackit.cloud/migrated-machine" -const workerMigratedAnnotation = "stackit.cloud/machine-controller-manager-migrated" +const ( + shouldMigrateMachineAnnotation = "stackit.cloud/machine-should-be-migrated" + migratedMachineAnnotation = "stackit.cloud/migrated-machine" + workerMigratedAnnotation = "stackit.cloud/machine-controller-manager-migrated" +) // MachineClassKind yields the name of the machine class kind used by OpenStack provider. func (w *workerDelegate) MachineClassKind() string { @@ -433,7 +435,7 @@ func (w *workerDelegate) migrateMachines(ctx context.Context) error { if len(migrateMachines) == 0 { // no old openstack machine - return nil + return w.markWorkerAsMigrated(ctx) } iaas, err := w.stackitClient.IaaS(ctx, w.seedClient, w.worker.Spec.SecretRef) From 65ddc5e1b919286b25cab0e8cb01dffc9611e7e6 Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Mon, 31 Aug 2026 15:11:20 +0200 Subject: [PATCH 06/17] add diagnosis --- pkg/controller/worker/machines.go | 80 ++++++++++++++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/pkg/controller/worker/machines.go b/pkg/controller/worker/machines.go index f0989f92..52f189e1 100644 --- a/pkg/controller/worker/machines.go +++ b/pkg/controller/worker/machines.go @@ -7,6 +7,7 @@ package worker import ( "context" "fmt" + "log" "math" "path/filepath" "regexp" @@ -490,7 +491,28 @@ func (w *workerDelegate) migrateMachines(ctx context.Context) error { return w.markWorkerAsMigrated(ctx) } +//func (w *workerDelegate) markWorkerAsMigrated(ctx context.Context) error { +// patchWorker := client.MergeFrom(w.worker.DeepCopy()) +// +// if w.worker.Annotations == nil { +// w.worker.Annotations = make(map[string]string) +// } +// +// w.worker.Annotations[workerMigratedAnnotation] = "true" +// +// return w.seedClient.Patch(ctx, w.worker, patchWorker) +//} + +// this is a diagnosis code and should not be used in production code. func (w *workerDelegate) markWorkerAsMigrated(ctx context.Context) error { + log.Printf( + "markWorkerAsMigrated: worker=%s namespace=%s resourceVersion=%s annotationsBefore=%v", + w.worker.Name, + w.worker.Namespace, + w.worker.ResourceVersion, + w.worker.Annotations, + ) + patchWorker := client.MergeFrom(w.worker.DeepCopy()) if w.worker.Annotations == nil { @@ -499,5 +521,61 @@ func (w *workerDelegate) markWorkerAsMigrated(ctx context.Context) error { w.worker.Annotations[workerMigratedAnnotation] = "true" - return w.seedClient.Patch(ctx, w.worker, patchWorker) + log.Printf( + "markWorkerAsMigrated: patching worker=%s annotation=%s value=%s", + w.worker.Name, + workerMigratedAnnotation, + w.worker.Annotations[workerMigratedAnnotation], + ) + + if err := w.seedClient.Patch(ctx, w.worker, patchWorker); err != nil { + log.Printf( + "markWorkerAsMigrated: PATCH FAILED worker=%s error=%v", + w.worker.Name, + err, + ) + return fmt.Errorf("patch worker migration annotation: %w", err) + } + + log.Printf( + "markWorkerAsMigrated: PATCH SUCCEEDED worker=%s resourceVersion=%s annotations=%v", + w.worker.Name, + w.worker.ResourceVersion, + w.worker.Annotations, + ) + + // Read the Worker again from the API server. + var worker extensionsv1alpha1.Worker + if err := w.seedClient.Get( + ctx, + client.ObjectKeyFromObject(w.worker), + &worker, + ); err != nil { + log.Printf( + "markWorkerAsMigrated: GET AFTER PATCH FAILED worker=%s error=%v", + w.worker.Name, + err, + ) + return fmt.Errorf("get worker after migration patch: %w", err) + } + + actualValue := worker.Annotations[workerMigratedAnnotation] + + log.Printf( + "markWorkerAsMigrated: API SERVER VALUE worker=%s annotation=%s value=%q allAnnotations=%v", + worker.Name, + workerMigratedAnnotation, + actualValue, + worker.Annotations, + ) + + if actualValue != "true" { + return fmt.Errorf( + "worker migration annotation was not persisted: worker=%s value=%q", + worker.Name, + actualValue, + ) + } + + return nil } From 0802b8cc9357b2b5c61b896d921817fb200a9205 Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Mon, 31 Aug 2026 15:26:19 +0200 Subject: [PATCH 07/17] trigger build From 26174dfe50e9b6d8c2e663ae1f1b552c37886248 Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Mon, 31 Aug 2026 22:51:08 +0200 Subject: [PATCH 08/17] run ci From abf5e0c51d0a9b3608b3113e16c6ed9d16e663ff Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Wed, 2 Sep 2026 09:18:39 +0200 Subject: [PATCH 09/17] add nil stackit client to the tests --- pkg/controller/worker/machines_test.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/pkg/controller/worker/machines_test.go b/pkg/controller/worker/machines_test.go index c8101e98..f8523993 100644 --- a/pkg/controller/worker/machines_test.go +++ b/pkg/controller/worker/machines_test.go @@ -70,7 +70,7 @@ var _ = Describe("Machines", func() { Context("workerDelegate", func() { BeforeEach(func() { - workerDelegate, _ = NewWorkerDelegate(nil, scheme, nil, "", nil, nil, "") + workerDelegate, _ = NewWorkerDelegate(nil, scheme, nil, "", nil, nil, "", nil) }) Describe("#TestLabelNormalization", func() { @@ -571,7 +571,7 @@ var _ = Describe("Machines", func() { WithStatusSubresource(&extensionsv1alpha1.Worker{}). Build() - workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, clusterWithoutImages, customLabelDomain) + workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, clusterWithoutImages, customLabelDomain, nil) }) Describe("machine images", func() { @@ -841,7 +841,7 @@ var _ = Describe("Machines", func() { }) It("should return the expected machine deployments for profile image types", func() { - workerDelegate, _ := NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain) + workerDelegate, _ := NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain, nil) // Test workerDelegate.DeployMachineClasses() @@ -880,7 +880,7 @@ var _ = Describe("Machines", func() { It("should return the expected machine deployments for profile image types with id", func() { // setup(region, "", machineImageID, archARM) - workerDelegate, _ := NewWorkerDelegate(c, scheme, chartApplier, "", workerWithRegion, clusterWithRegion, customLabelDomain) + workerDelegate, _ := NewWorkerDelegate(c, scheme, chartApplier, "", workerWithRegion, clusterWithRegion, customLabelDomain, nil) clusterWithRegion.Shoot.Spec.Hibernation = &gardencorev1beta1.Hibernation{Enabled: new(true)} // Test workerDelegate.DeployMachineClasses() @@ -923,7 +923,7 @@ var _ = Describe("Machines", func() { w.Spec.Pools[0].ProviderConfig = &runtime.RawExtension{ Raw: encode(workerConfig), } - workerDelegate, _ := NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain) + workerDelegate, _ := NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain, nil) result, err := workerDelegate.GenerateMachineDeployments(ctx) Expect(err).NotTo(HaveOccurred()) @@ -973,7 +973,7 @@ var _ = Describe("Machines", func() { It("should fail because the infrastructure status cannot be decoded", func() { w.Spec.InfrastructureProviderStatus = &runtime.RawExtension{} - workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain) + workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain, nil) result, err := workerDelegate.GenerateMachineDeployments(ctx) Expect(err).To(HaveOccurred()) @@ -985,7 +985,7 @@ var _ = Describe("Machines", func() { Raw: encode(&stackitv1alpha1.InfrastructureStatus{}), } - workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain) + workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain, nil) result, err := workerDelegate.GenerateMachineDeployments(ctx) Expect(err).To(HaveOccurred()) @@ -995,7 +995,7 @@ var _ = Describe("Machines", func() { It("should fail because the machine image for this cloud profile cannot be found", func() { clusterWithoutImages.CloudProfile.Name = "another-cloud-profile" - workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, clusterWithoutImages, customLabelDomain) + workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, clusterWithoutImages, customLabelDomain, nil) result, err := workerDelegate.GenerateMachineDeployments(ctx) Expect(err).To(HaveOccurred()) @@ -1016,7 +1016,7 @@ var _ = Describe("Machines", func() { NodeConditions: testNodeConditions, } - workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain) + workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain, nil) result, err := workerDelegate.GenerateMachineDeployments(ctx) resultSettings := result[0].MachineConfiguration @@ -1039,7 +1039,7 @@ var _ = Describe("Machines", func() { ScaleDownUtilizationThreshold: new("0.5"), } w.Spec.Pools[1].ClusterAutoscaler = nil - workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain) + workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain, nil) result, err := workerDelegate.GenerateMachineDeployments(ctx) Expect(err).NotTo(HaveOccurred()) @@ -1107,7 +1107,7 @@ var _ = Describe("Machines", func() { w.Spec.Pools[0].MachineControllerManagerSettings = &gardencorev1beta1.MachineControllerManagerSettings{ AutoPreserveFailedMachineMax: new(int32(4)), } - workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain) + workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain, nil) result, err := workerDelegate.GenerateMachineDeployments(ctx) Expect(err).NotTo(HaveOccurred()) @@ -1118,7 +1118,7 @@ var _ = Describe("Machines", func() { It("should set autoPreserveFailedMachineMax to 0 per zone when machineControllerManagerSettings is nil", func() { w.Spec.Pools[0].MachineControllerManagerSettings = nil - workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain) + workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain, nil) result, err := workerDelegate.GenerateMachineDeployments(ctx) Expect(err).NotTo(HaveOccurred()) @@ -1131,7 +1131,7 @@ var _ = Describe("Machines", func() { w.Spec.Pools[0].MachineControllerManagerSettings = &gardencorev1beta1.MachineControllerManagerSettings{ AutoPreserveFailedMachineMax: nil, } - workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain) + workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain, nil) result, err := workerDelegate.GenerateMachineDeployments(ctx) Expect(err).NotTo(HaveOccurred()) From 3a6506a13ff86b14c7eaad3e9e894b7e3b708b16 Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Wed, 2 Sep 2026 09:31:56 +0200 Subject: [PATCH 10/17] remove diagnosis --- pkg/controller/worker/machines.go | 80 +------------------------------ 1 file changed, 1 insertion(+), 79 deletions(-) diff --git a/pkg/controller/worker/machines.go b/pkg/controller/worker/machines.go index 52f189e1..f0989f92 100644 --- a/pkg/controller/worker/machines.go +++ b/pkg/controller/worker/machines.go @@ -7,7 +7,6 @@ package worker import ( "context" "fmt" - "log" "math" "path/filepath" "regexp" @@ -491,28 +490,7 @@ func (w *workerDelegate) migrateMachines(ctx context.Context) error { return w.markWorkerAsMigrated(ctx) } -//func (w *workerDelegate) markWorkerAsMigrated(ctx context.Context) error { -// patchWorker := client.MergeFrom(w.worker.DeepCopy()) -// -// if w.worker.Annotations == nil { -// w.worker.Annotations = make(map[string]string) -// } -// -// w.worker.Annotations[workerMigratedAnnotation] = "true" -// -// return w.seedClient.Patch(ctx, w.worker, patchWorker) -//} - -// this is a diagnosis code and should not be used in production code. func (w *workerDelegate) markWorkerAsMigrated(ctx context.Context) error { - log.Printf( - "markWorkerAsMigrated: worker=%s namespace=%s resourceVersion=%s annotationsBefore=%v", - w.worker.Name, - w.worker.Namespace, - w.worker.ResourceVersion, - w.worker.Annotations, - ) - patchWorker := client.MergeFrom(w.worker.DeepCopy()) if w.worker.Annotations == nil { @@ -521,61 +499,5 @@ func (w *workerDelegate) markWorkerAsMigrated(ctx context.Context) error { w.worker.Annotations[workerMigratedAnnotation] = "true" - log.Printf( - "markWorkerAsMigrated: patching worker=%s annotation=%s value=%s", - w.worker.Name, - workerMigratedAnnotation, - w.worker.Annotations[workerMigratedAnnotation], - ) - - if err := w.seedClient.Patch(ctx, w.worker, patchWorker); err != nil { - log.Printf( - "markWorkerAsMigrated: PATCH FAILED worker=%s error=%v", - w.worker.Name, - err, - ) - return fmt.Errorf("patch worker migration annotation: %w", err) - } - - log.Printf( - "markWorkerAsMigrated: PATCH SUCCEEDED worker=%s resourceVersion=%s annotations=%v", - w.worker.Name, - w.worker.ResourceVersion, - w.worker.Annotations, - ) - - // Read the Worker again from the API server. - var worker extensionsv1alpha1.Worker - if err := w.seedClient.Get( - ctx, - client.ObjectKeyFromObject(w.worker), - &worker, - ); err != nil { - log.Printf( - "markWorkerAsMigrated: GET AFTER PATCH FAILED worker=%s error=%v", - w.worker.Name, - err, - ) - return fmt.Errorf("get worker after migration patch: %w", err) - } - - actualValue := worker.Annotations[workerMigratedAnnotation] - - log.Printf( - "markWorkerAsMigrated: API SERVER VALUE worker=%s annotation=%s value=%q allAnnotations=%v", - worker.Name, - workerMigratedAnnotation, - actualValue, - worker.Annotations, - ) - - if actualValue != "true" { - return fmt.Errorf( - "worker migration annotation was not persisted: worker=%s value=%q", - worker.Name, - actualValue, - ) - } - - return nil + return w.seedClient.Patch(ctx, w.worker, patchWorker) } From 4f796e40b8f0e4c60c25f63cd5aaef51188fb3a7 Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Wed, 2 Sep 2026 13:23:51 +0200 Subject: [PATCH 11/17] add poc test for the machine migration --- pkg/controller/worker/machines_test.go | 95 ++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/pkg/controller/worker/machines_test.go b/pkg/controller/worker/machines_test.go index f8523993..12366d68 100644 --- a/pkg/controller/worker/machines_test.go +++ b/pkg/controller/worker/machines_test.go @@ -26,6 +26,7 @@ import ( machinev1alpha1 "github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + iaas2 "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" "go.uber.org/mock/gomock" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -40,6 +41,7 @@ import ( . "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/controller/worker" "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/feature" "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/openstack" + mockstackitclient "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit/client/mock" ) var _ = Describe("Machines", func() { @@ -50,6 +52,8 @@ var _ = Describe("Machines", func() { c client.Client chartApplier *mockkubernetes.MockChartApplier + mockStackitClient *mockstackitclient.MockFactory + workerDelegate genericworkeractuator.WorkerDelegate scheme *runtime.Scheme ) @@ -907,6 +911,97 @@ var _ = Describe("Machines", func() { Expect(result).To(Equal(machineDeployments)) }) + Context("MCM migration", func() { + BeforeEach(func() { + DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.UseSTACKITMachineControllerManager, useStackitMCM)) + //DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.MigrateSTACKITMachineControllerManager, true)) + mockStackitClient = mockstackitclient.NewMockFactory(ctrl) + workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain, mockStackitClient) + }) + + It("should migrate the machine controller manager", func() { + By("creating a machine") + machine := &machinev1alpha1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machine-1", + Namespace: w.Namespace, + }, + Spec: machinev1alpha1.MachineSpec{ + Class: machinev1alpha1.ClassSpec{ + Name: "machineclass", + }, + ProviderID: "openstack:///RegionOne/server-123", + }, + } + + Expect(c.Create(ctx, machine)).To(Succeed()) + + By("creating a machine class") + machineClassPath := filepath.Join("internal", "machineclass") + if useStackitMCM { + machineClassPath = filepath.Join("internal", "machineclass-stackit") + } + + chartApplier. + EXPECT(). + ApplyFromEmbeddedFS( + ctx, + charts.InternalChart, + machineClassPath, + w.Namespace, + "machineclass", + kubernetes.Values(machineClasses), + ). + Return(nil) + + Expect(workerDelegate.DeployMachineClasses(ctx)).To(Succeed()) + + By("enabling the feature gate") + DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.UseSTACKITMachineControllerManager, true)) + DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.MigrateSTACKITMachineControllerManager, true)) + + mockstackitclient.NewMockIaaSClient(ctrl).EXPECT().UpdateServer(ctx, "server-123", iaas2.UpdateServerPayload{ + Labels: map[string]any{ + "mcm.gardener.cloud/machine": machine.Name, + "mcm.gardener.cloud/machineclass": machine.Spec.Class.Name, + "mcm.gardener.cloud/role": "node", + }, + }).Return(nil, nil) + + By("checking for the migrated machine") + migratedMachine := &machinev1alpha1.Machine{} + + Expect(c.Get(ctx, client.ObjectKey{ + Name: "machine-1", + Namespace: w.Namespace, + }, migratedMachine)).To(Succeed()) + + Expect(migratedMachine.Spec.ProviderID). + To(Equal("stackit://project-id-123/server-123")) + + Expect(migratedMachine.Annotations). + To(HaveKeyWithValue("stackit.cloud/migrated-machine", "true")) + + // TODO: how do i check this, as the annotation will get removed soon after the migration + //Expect(migratedMachine.Annotations). + // NotTo(HaveKey("stackit.cloud/machine-should-be-migrated")) + + By("checking for the worker") + migratedWorker := &extensionsv1alpha1.Worker{} + + Expect(c.Get(ctx, client.ObjectKey{ + Name: w.Name, + Namespace: w.Namespace, + }, migratedWorker)).To(Succeed()) + + Expect(migratedWorker.Annotations). + To(HaveKeyWithValue( + "stackit.cloud/machine-controller-manager-migrated", + "true", + )) + }) + }) + Context("Machine Labels", func() { It("should consider rolling machine labels for the worker pool hash", func() { // setup(region, machineImage, "") From 240ad694d27a620467ac61566620683d6dc697b2 Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Wed, 2 Sep 2026 15:17:21 +0200 Subject: [PATCH 12/17] fix mocks in the tests --- pkg/controller/worker/machines.go | 1 + pkg/controller/worker/machines_test.go | 35 ++++++++++++++++++++------ 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/pkg/controller/worker/machines.go b/pkg/controller/worker/machines.go index f0989f92..e2187c75 100644 --- a/pkg/controller/worker/machines.go +++ b/pkg/controller/worker/machines.go @@ -77,6 +77,7 @@ func (w *workerDelegate) DeployMachineClasses(ctx context.Context) error { } if feature.MigrateStackitMachineControllerManager(w.cluster) && w.worker.Annotations[workerMigratedAnnotation] != "true" { + fmt.Println("Migrating Stackit Machine Controller Manager to Gardener Machine Controller Manager...") err = w.migrateMachines(ctx) if err != nil { return err diff --git a/pkg/controller/worker/machines_test.go b/pkg/controller/worker/machines_test.go index 12366d68..4857e941 100644 --- a/pkg/controller/worker/machines_test.go +++ b/pkg/controller/worker/machines_test.go @@ -560,6 +560,7 @@ var _ = Describe("Machines", func() { fakeScheme := runtime.NewScheme() Expect(corev1.AddToScheme(fakeScheme)).To(Succeed()) Expect(extensionsv1alpha1.AddToScheme(fakeScheme)).To(Succeed()) + Expect(machinev1alpha1.AddToScheme(fakeScheme)).To(Succeed()) c = fakeclient.NewClientBuilder(). WithScheme(fakeScheme). WithObjects( @@ -960,13 +961,33 @@ var _ = Describe("Machines", func() { DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.UseSTACKITMachineControllerManager, true)) DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.MigrateSTACKITMachineControllerManager, true)) - mockstackitclient.NewMockIaaSClient(ctrl).EXPECT().UpdateServer(ctx, "server-123", iaas2.UpdateServerPayload{ - Labels: map[string]any{ - "mcm.gardener.cloud/machine": machine.Name, - "mcm.gardener.cloud/machineclass": machine.Spec.Class.Name, - "mcm.gardener.cloud/role": "node", - }, - }).Return(nil, nil) + //mockstackitclient.NewMockIaaSClient(ctrl).EXPECT().UpdateServer(ctx, "server-123", iaas2.UpdateServerPayload{ + // Labels: map[string]any{ + // "mcm.gardener.cloud/machine": machine.Name, + // "mcm.gardener.cloud/machineclass": machine.Spec.Class.Name, + // "mcm.gardener.cloud/role": "node", + // }, + //}).Return(nil, nil) + + mockIaaSClient := mockstackitclient.NewMockIaaSClient(ctrl) + + mockStackitClient.EXPECT(). + IaaS(ctx, c, w.Spec.SecretRef). + Return(mockIaaSClient, nil) + + mockIaaSClient.EXPECT(). + ProjectID(). + Return("project-id-123") + + mockIaaSClient.EXPECT(). + UpdateServer(ctx, "server-123", iaas2.UpdateServerPayload{ + Labels: map[string]any{ + "mcm.gardener.cloud/machine": machine.Name, + "mcm.gardener.cloud/machineclass": machine.Spec.Class.Name, + "mcm.gardener.cloud/role": "node", + }, + }). + Return(nil, nil) By("checking for the migrated machine") migratedMachine := &machinev1alpha1.Machine{} From 2471058b474e17031ca1eb1949fb80f0c369906d Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Thu, 3 Sep 2026 11:50:40 +0200 Subject: [PATCH 13/17] add improvements to the test --- pkg/controller/worker/machines.go | 3 +- pkg/controller/worker/machines_test.go | 287 +++++++++++++++---------- 2 files changed, 178 insertions(+), 112 deletions(-) diff --git a/pkg/controller/worker/machines.go b/pkg/controller/worker/machines.go index e2187c75..2dc2dc2e 100644 --- a/pkg/controller/worker/machines.go +++ b/pkg/controller/worker/machines.go @@ -76,8 +76,9 @@ func (w *workerDelegate) DeployMachineClasses(ctx context.Context) error { return err } + fmt.Println("Deploying OpenStack machine classes...") if feature.MigrateStackitMachineControllerManager(w.cluster) && w.worker.Annotations[workerMigratedAnnotation] != "true" { - fmt.Println("Migrating Stackit Machine Controller Manager to Gardener Machine Controller Manager...") + fmt.Println("Migrating Stackit Machine Controller Manager to Gardener Machine Controller Manager...") // TODO: remove later err = w.migrateMachines(ctx) if err != nil { return err diff --git a/pkg/controller/worker/machines_test.go b/pkg/controller/worker/machines_test.go index 4857e941..82ebdf9b 100644 --- a/pkg/controller/worker/machines_test.go +++ b/pkg/controller/worker/machines_test.go @@ -912,117 +912,6 @@ var _ = Describe("Machines", func() { Expect(result).To(Equal(machineDeployments)) }) - Context("MCM migration", func() { - BeforeEach(func() { - DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.UseSTACKITMachineControllerManager, useStackitMCM)) - //DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.MigrateSTACKITMachineControllerManager, true)) - mockStackitClient = mockstackitclient.NewMockFactory(ctrl) - workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain, mockStackitClient) - }) - - It("should migrate the machine controller manager", func() { - By("creating a machine") - machine := &machinev1alpha1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "machine-1", - Namespace: w.Namespace, - }, - Spec: machinev1alpha1.MachineSpec{ - Class: machinev1alpha1.ClassSpec{ - Name: "machineclass", - }, - ProviderID: "openstack:///RegionOne/server-123", - }, - } - - Expect(c.Create(ctx, machine)).To(Succeed()) - - By("creating a machine class") - machineClassPath := filepath.Join("internal", "machineclass") - if useStackitMCM { - machineClassPath = filepath.Join("internal", "machineclass-stackit") - } - - chartApplier. - EXPECT(). - ApplyFromEmbeddedFS( - ctx, - charts.InternalChart, - machineClassPath, - w.Namespace, - "machineclass", - kubernetes.Values(machineClasses), - ). - Return(nil) - - Expect(workerDelegate.DeployMachineClasses(ctx)).To(Succeed()) - - By("enabling the feature gate") - DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.UseSTACKITMachineControllerManager, true)) - DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.MigrateSTACKITMachineControllerManager, true)) - - //mockstackitclient.NewMockIaaSClient(ctrl).EXPECT().UpdateServer(ctx, "server-123", iaas2.UpdateServerPayload{ - // Labels: map[string]any{ - // "mcm.gardener.cloud/machine": machine.Name, - // "mcm.gardener.cloud/machineclass": machine.Spec.Class.Name, - // "mcm.gardener.cloud/role": "node", - // }, - //}).Return(nil, nil) - - mockIaaSClient := mockstackitclient.NewMockIaaSClient(ctrl) - - mockStackitClient.EXPECT(). - IaaS(ctx, c, w.Spec.SecretRef). - Return(mockIaaSClient, nil) - - mockIaaSClient.EXPECT(). - ProjectID(). - Return("project-id-123") - - mockIaaSClient.EXPECT(). - UpdateServer(ctx, "server-123", iaas2.UpdateServerPayload{ - Labels: map[string]any{ - "mcm.gardener.cloud/machine": machine.Name, - "mcm.gardener.cloud/machineclass": machine.Spec.Class.Name, - "mcm.gardener.cloud/role": "node", - }, - }). - Return(nil, nil) - - By("checking for the migrated machine") - migratedMachine := &machinev1alpha1.Machine{} - - Expect(c.Get(ctx, client.ObjectKey{ - Name: "machine-1", - Namespace: w.Namespace, - }, migratedMachine)).To(Succeed()) - - Expect(migratedMachine.Spec.ProviderID). - To(Equal("stackit://project-id-123/server-123")) - - Expect(migratedMachine.Annotations). - To(HaveKeyWithValue("stackit.cloud/migrated-machine", "true")) - - // TODO: how do i check this, as the annotation will get removed soon after the migration - //Expect(migratedMachine.Annotations). - // NotTo(HaveKey("stackit.cloud/machine-should-be-migrated")) - - By("checking for the worker") - migratedWorker := &extensionsv1alpha1.Worker{} - - Expect(c.Get(ctx, client.ObjectKey{ - Name: w.Name, - Namespace: w.Namespace, - }, migratedWorker)).To(Succeed()) - - Expect(migratedWorker.Annotations). - To(HaveKeyWithValue( - "stackit.cloud/machine-controller-manager-migrated", - "true", - )) - }) - }) - Context("Machine Labels", func() { It("should consider rolling machine labels for the worker pool hash", func() { // setup(region, machineImage, "") @@ -1086,6 +975,182 @@ var _ = Describe("Machines", func() { }) }) + Context("MCM migration", func() { + var ( + defaultMachineClass map[string]any + machineClasses map[string]any + ) + + BeforeEach(func() { + defaultMachineClass = map[string]any{ + "region": region, + "keyName": keyName, + "networkID": networkID, + "podNetworkCIDRs": []string{podCIDR}, + "securityGroups": []string{securityGroupName}, + "tags": map[string]string{ + fmt.Sprintf("kubernetes.io-cluster-%s", technicalID): "1", + "kubernetes.io-role-node": "1", + }, + "secret": map[string]any{ + "cloudConfig": string(userData), + }, + "operatingSystem": map[string]any{ + "operatingSystemName": machineImageName, + "operatingSystemVersion": strings.ReplaceAll(machineImageVersion, "+", "_"), + }, + } + + if useStackitMCM { + securityGroupID := "sg-12345" + // STACKIT uses security group IDs and simplified tags + defaultMachineClass["securityGroups"] = []string{securityGroupID} + defaultMachineClass["tags"] = map[string]string{ + "kubernetes.io/cluster": technicalID, + } + // Note: subnetID is NOT included for STACKIT + } else { + // OpenStack uses security group names, full tags, and subnetID + defaultMachineClass["securityGroups"] = []string{securityGroupName} + defaultMachineClass["tags"] = map[string]string{ + fmt.Sprintf("kubernetes.io-cluster-%s", technicalID): "1", + "kubernetes.io-role-node": "1", + } + defaultMachineClass["subnetID"] = subnetID + } + + var ( + machineClassPool1Zone1 = addKeyValueToMap(defaultMachineClass, "availabilityZone", zone1) + machineClassPool1Zone2 = addKeyValueToMap(defaultMachineClass, "availabilityZone", zone2) + machineClassPool2Zone1 = addKeyValueToMap(defaultMachineClass, "availabilityZone", zone1) + machineClassPool2Zone2 = addKeyValueToMap(defaultMachineClass, "availabilityZone", zone2) + machineClassPool3Zone1 = addKeyValueToMap(defaultMachineClass, "availabilityZone", zone1) + machineClassPool3Zone2 = addKeyValueToMap(defaultMachineClass, "availabilityZone", zone2) + ) + + machineClasses = map[string]any{"machineClasses": []map[string]any{ + machineClassPool1Zone1, + machineClassPool1Zone2, + machineClassPool2Zone1, + machineClassPool2Zone2, + machineClassPool3Zone1, + machineClassPool3Zone2, + }} + + DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.UseSTACKITMachineControllerManager, useStackitMCM)) + + mockStackitClient = mockstackitclient.NewMockFactory(ctrl) + if cluster.Shoot.Annotations == nil { + cluster.Shoot.Annotations = map[string]string{} + } + + cluster.Shoot.Annotations[feature.ShootMigrateSTACKITMachineControllerManager] = "true" + workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain, mockStackitClient) + }) + + It("should migrate the machine controller manager", func() { + By("creating a machine") + machine := &machinev1alpha1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machine-1", + Namespace: w.Namespace, + }, + Spec: machinev1alpha1.MachineSpec{ + Class: machinev1alpha1.ClassSpec{ + Name: "machineclass", + }, + ProviderID: "openstack:///RegionOne/server-123", + }, + } + + Expect(c.Create(ctx, machine)).To(Succeed()) + + By("creating a machine class") + machineClassPath := filepath.Join("internal", "machineclass") + if useStackitMCM { + machineClassPath = filepath.Join("internal", "machineclass-stackit") + } + + chartApplier. + EXPECT(). + ApplyFromEmbeddedFS( + ctx, + charts.InternalChart, + machineClassPath, + w.Namespace, + "machineclass", + kubernetes.Values(machineClasses), + ). + Return(nil) + + Expect(workerDelegate.DeployMachineClasses(ctx)).To(Succeed()) + + By("enabling the feature gate") + DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.UseSTACKITMachineControllerManager, true)) + DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.MigrateSTACKITMachineControllerManager, true)) + + //mockstackitclient.NewMockIaaSClient(ctrl).EXPECT().UpdateServer(ctx, "server-123", iaas2.UpdateServerPayload{ + // Labels: map[string]any{ + // "mcm.gardener.cloud/machine": machine.Name, + // "mcm.gardener.cloud/machineclass": machine.Spec.Class.Name, + // "mcm.gardener.cloud/role": "node", + // }, + //}).Return(nil, nil) + + mockIaaSClient := mockstackitclient.NewMockIaaSClient(ctrl) + + mockStackitClient.EXPECT(). + IaaS(ctx, c, w.Spec.SecretRef). + Return(mockIaaSClient, nil) + + mockIaaSClient.EXPECT(). + ProjectID(). + Return("project-id-123") + + mockIaaSClient.EXPECT(). + UpdateServer(ctx, "server-123", iaas2.UpdateServerPayload{ + Labels: map[string]any{ + "mcm.gardener.cloud/machine": machine.Name, + "mcm.gardener.cloud/machineclass": machine.Spec.Class.Name, + "mcm.gardener.cloud/role": "node", + }, + }). + Return(nil, nil) + + By("checking for the migrated machine") + migratedMachine := &machinev1alpha1.Machine{} + + Expect(c.Get(ctx, client.ObjectKey{ + Name: "machine-1", + Namespace: w.Namespace, + }, migratedMachine)).To(Succeed()) + + Expect(migratedMachine.Spec.ProviderID). + To(Equal("stackit://project-id-123/server-123")) + + Expect(migratedMachine.Annotations). + To(HaveKeyWithValue("stackit.cloud/migrated-machine", "true")) + + // TODO: how do i check this, as the annotation will get removed soon after the migration + //Expect(migratedMachine.Annotations). + // NotTo(HaveKey("stackit.cloud/machine-should-be-migrated")) + + By("checking for the worker") + migratedWorker := &extensionsv1alpha1.Worker{} + + Expect(c.Get(ctx, client.ObjectKey{ + Name: w.Name, + Namespace: w.Namespace, + }, migratedWorker)).To(Succeed()) + + Expect(migratedWorker.Annotations). + To(HaveKeyWithValue( + "stackit.cloud/machine-controller-manager-migrated", + "true", + )) + }) + }) + It("should fail because the infrastructure status cannot be decoded", func() { w.Spec.InfrastructureProviderStatus = &runtime.RawExtension{} From 16422cf3c93ee270e966ce7fb1e0cdca7e42d5c3 Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Thu, 3 Sep 2026 16:33:07 +0200 Subject: [PATCH 14/17] fix the test and its running --- pkg/controller/worker/machines.go | 5 +- pkg/controller/worker/machines_test.go | 100 +++---------------------- 2 files changed, 14 insertions(+), 91 deletions(-) diff --git a/pkg/controller/worker/machines.go b/pkg/controller/worker/machines.go index 2dc2dc2e..455f0a78 100644 --- a/pkg/controller/worker/machines.go +++ b/pkg/controller/worker/machines.go @@ -76,9 +76,7 @@ func (w *workerDelegate) DeployMachineClasses(ctx context.Context) error { return err } - fmt.Println("Deploying OpenStack machine classes...") if feature.MigrateStackitMachineControllerManager(w.cluster) && w.worker.Annotations[workerMigratedAnnotation] != "true" { - fmt.Println("Migrating Stackit Machine Controller Manager to Gardener Machine Controller Manager...") // TODO: remove later err = w.migrateMachines(ctx) if err != nil { return err @@ -447,6 +445,9 @@ func (w *workerDelegate) migrateMachines(ctx context.Context) error { for _, m := range migrateMachines { patchAnnotations := client.MergeFrom(m.DeepCopy()) + if m.Annotations == nil { + m.Annotations = make(map[string]string) + } m.Annotations[shouldMigrateMachineAnnotation] = "true" m.Annotations[migratedMachineAnnotation] = "true" err = w.seedClient.Patch(ctx, &m, patchAnnotations) diff --git a/pkg/controller/worker/machines_test.go b/pkg/controller/worker/machines_test.go index 82ebdf9b..2506c833 100644 --- a/pkg/controller/worker/machines_test.go +++ b/pkg/controller/worker/machines_test.go @@ -976,68 +976,10 @@ var _ = Describe("Machines", func() { }) Context("MCM migration", func() { - var ( - defaultMachineClass map[string]any - machineClasses map[string]any - ) + var machine *machinev1alpha1.Machine BeforeEach(func() { - defaultMachineClass = map[string]any{ - "region": region, - "keyName": keyName, - "networkID": networkID, - "podNetworkCIDRs": []string{podCIDR}, - "securityGroups": []string{securityGroupName}, - "tags": map[string]string{ - fmt.Sprintf("kubernetes.io-cluster-%s", technicalID): "1", - "kubernetes.io-role-node": "1", - }, - "secret": map[string]any{ - "cloudConfig": string(userData), - }, - "operatingSystem": map[string]any{ - "operatingSystemName": machineImageName, - "operatingSystemVersion": strings.ReplaceAll(machineImageVersion, "+", "_"), - }, - } - - if useStackitMCM { - securityGroupID := "sg-12345" - // STACKIT uses security group IDs and simplified tags - defaultMachineClass["securityGroups"] = []string{securityGroupID} - defaultMachineClass["tags"] = map[string]string{ - "kubernetes.io/cluster": technicalID, - } - // Note: subnetID is NOT included for STACKIT - } else { - // OpenStack uses security group names, full tags, and subnetID - defaultMachineClass["securityGroups"] = []string{securityGroupName} - defaultMachineClass["tags"] = map[string]string{ - fmt.Sprintf("kubernetes.io-cluster-%s", technicalID): "1", - "kubernetes.io-role-node": "1", - } - defaultMachineClass["subnetID"] = subnetID - } - - var ( - machineClassPool1Zone1 = addKeyValueToMap(defaultMachineClass, "availabilityZone", zone1) - machineClassPool1Zone2 = addKeyValueToMap(defaultMachineClass, "availabilityZone", zone2) - machineClassPool2Zone1 = addKeyValueToMap(defaultMachineClass, "availabilityZone", zone1) - machineClassPool2Zone2 = addKeyValueToMap(defaultMachineClass, "availabilityZone", zone2) - machineClassPool3Zone1 = addKeyValueToMap(defaultMachineClass, "availabilityZone", zone1) - machineClassPool3Zone2 = addKeyValueToMap(defaultMachineClass, "availabilityZone", zone2) - ) - - machineClasses = map[string]any{"machineClasses": []map[string]any{ - machineClassPool1Zone1, - machineClassPool1Zone2, - machineClassPool2Zone1, - machineClassPool2Zone2, - machineClassPool3Zone1, - machineClassPool3Zone2, - }} - - DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.UseSTACKITMachineControllerManager, useStackitMCM)) + DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.UseSTACKITMachineControllerManager, true)) mockStackitClient = mockstackitclient.NewMockFactory(ctrl) if cluster.Shoot.Annotations == nil { @@ -1046,11 +988,8 @@ var _ = Describe("Machines", func() { cluster.Shoot.Annotations[feature.ShootMigrateSTACKITMachineControllerManager] = "true" workerDelegate, _ = NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customLabelDomain, mockStackitClient) - }) - It("should migrate the machine controller manager", func() { - By("creating a machine") - machine := &machinev1alpha1.Machine{ + machine = &machinev1alpha1.Machine{ ObjectMeta: metav1.ObjectMeta{ Name: "machine-1", Namespace: w.Namespace, @@ -1065,38 +1004,20 @@ var _ = Describe("Machines", func() { Expect(c.Create(ctx, machine)).To(Succeed()) - By("creating a machine class") - machineClassPath := filepath.Join("internal", "machineclass") - if useStackitMCM { - machineClassPath = filepath.Join("internal", "machineclass-stackit") - } - chartApplier. EXPECT(). ApplyFromEmbeddedFS( ctx, charts.InternalChart, - machineClassPath, + filepath.Join("internal", "machineclass-stackit"), w.Namespace, "machineclass", - kubernetes.Values(machineClasses), + gomock.Any(), ). Return(nil) + }) - Expect(workerDelegate.DeployMachineClasses(ctx)).To(Succeed()) - - By("enabling the feature gate") - DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.UseSTACKITMachineControllerManager, true)) - DeferCleanup(testutils.WithFeatureGate(feature.MutableGate, feature.MigrateSTACKITMachineControllerManager, true)) - - //mockstackitclient.NewMockIaaSClient(ctrl).EXPECT().UpdateServer(ctx, "server-123", iaas2.UpdateServerPayload{ - // Labels: map[string]any{ - // "mcm.gardener.cloud/machine": machine.Name, - // "mcm.gardener.cloud/machineclass": machine.Spec.Class.Name, - // "mcm.gardener.cloud/role": "node", - // }, - //}).Return(nil, nil) - + It("should migrate the machine controller manager", func() { mockIaaSClient := mockstackitclient.NewMockIaaSClient(ctrl) mockStackitClient.EXPECT(). @@ -1117,6 +1038,8 @@ var _ = Describe("Machines", func() { }). Return(nil, nil) + Expect(workerDelegate.DeployMachineClasses(ctx)).To(Succeed()) + By("checking for the migrated machine") migratedMachine := &machinev1alpha1.Machine{} @@ -1131,9 +1054,8 @@ var _ = Describe("Machines", func() { Expect(migratedMachine.Annotations). To(HaveKeyWithValue("stackit.cloud/migrated-machine", "true")) - // TODO: how do i check this, as the annotation will get removed soon after the migration - //Expect(migratedMachine.Annotations). - // NotTo(HaveKey("stackit.cloud/machine-should-be-migrated")) + Expect(migratedMachine.Annotations). + NotTo(HaveKey("stackit.cloud/machine-should-be-migrated")) By("checking for the worker") migratedWorker := &extensionsv1alpha1.Worker{} From 425505cc78689579d4bdbbb52681530877fbd943 Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Fri, 4 Sep 2026 11:20:10 +0200 Subject: [PATCH 15/17] add iaas failure tolerance testing --- pkg/controller/worker/machines_test.go | 77 ++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/pkg/controller/worker/machines_test.go b/pkg/controller/worker/machines_test.go index 2506c833..86167d3a 100644 --- a/pkg/controller/worker/machines_test.go +++ b/pkg/controller/worker/machines_test.go @@ -1071,6 +1071,83 @@ var _ = Describe("Machines", func() { "true", )) }) + + It("should retry a machine after an IaaS API failure without touching migrated machines", func() { + migratedMachine := &machinev1alpha1.Machine{} + Expect(c.Get(ctx, client.ObjectKey{Name: machine.Name, Namespace: machine.Namespace}, migratedMachine)).To(Succeed()) + migratedMachine.Spec.ProviderID = "stackit://project-id-123/server-123" + migratedMachine.Annotations = map[string]string{"stackit.cloud/migrated-machine": "true"} + Expect(c.Update(ctx, migratedMachine)).To(Succeed()) + expectedMigratedMachine := migratedMachine.DeepCopy() + + pendingMachine := &machinev1alpha1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machine-2", + Namespace: w.Namespace, + Annotations: map[string]string{"stackit.cloud/machine-should-be-migrated": "true", "stackit.cloud/migrated-machine": "true"}, + }, + Spec: machinev1alpha1.MachineSpec{ + Class: machinev1alpha1.ClassSpec{Name: "machineclass"}, + ProviderID: "stackit://project-id-123/server-456", + }, + } + Expect(c.Create(ctx, pendingMachine)).To(Succeed()) + + failingIaaSClient := mockstackitclient.NewMockIaaSClient(ctrl) + mockStackitClient.EXPECT(). + IaaS(ctx, c, w.Spec.SecretRef). + Return(failingIaaSClient, nil) + failingIaaSClient.EXPECT().ProjectID().Return("project-id-123") + failingIaaSClient.EXPECT(). + UpdateServer(ctx, "server-456", iaas2.UpdateServerPayload{ + Labels: map[string]any{ + "mcm.gardener.cloud/machine": pendingMachine.Name, + "mcm.gardener.cloud/machineclass": pendingMachine.Spec.Class.Name, + "mcm.gardener.cloud/role": "node", + }, + }). + Return(nil, fmt.Errorf("temporary IaaS API failure")) + + Expect(workerDelegate.DeployMachineClasses(ctx)).To(MatchError(ContainSubstring("temporary IaaS API failure"))) + + failedMachine := &machinev1alpha1.Machine{} + Expect(c.Get(ctx, client.ObjectKey{Name: pendingMachine.Name, Namespace: pendingMachine.Namespace}, failedMachine)).To(Succeed()) + Expect(failedMachine.Annotations).To(HaveKeyWithValue("stackit.cloud/machine-should-be-migrated", "true")) + + unchangedMigratedMachine := &machinev1alpha1.Machine{} + Expect(c.Get(ctx, client.ObjectKey{Name: machine.Name, Namespace: machine.Namespace}, unchangedMigratedMachine)).To(Succeed()) + Expect(unchangedMigratedMachine).To(Equal(expectedMigratedMachine)) + + retryIaaSClient := mockstackitclient.NewMockIaaSClient(ctrl) + mockStackitClient.EXPECT(). + IaaS(ctx, c, w.Spec.SecretRef). + Return(retryIaaSClient, nil) + retryIaaSClient.EXPECT().ProjectID().Return("project-id-123") + retryIaaSClient.EXPECT(). + UpdateServer(ctx, "server-456", iaas2.UpdateServerPayload{ + Labels: map[string]any{ + "mcm.gardener.cloud/machine": pendingMachine.Name, + "mcm.gardener.cloud/machineclass": pendingMachine.Spec.Class.Name, + "mcm.gardener.cloud/role": "node", + }, + }). + Return(nil, nil) + chartApplier.EXPECT(). + ApplyFromEmbeddedFS(ctx, charts.InternalChart, filepath.Join("internal", "machineclass-stackit"), w.Namespace, "machineclass", gomock.Any()). + Return(nil) + + Expect(workerDelegate.DeployMachineClasses(ctx)).To(Succeed()) + + retriedMachine := &machinev1alpha1.Machine{} + Expect(c.Get(ctx, client.ObjectKey{Name: pendingMachine.Name, Namespace: pendingMachine.Namespace}, retriedMachine)).To(Succeed()) + Expect(retriedMachine.Annotations).NotTo(HaveKey("stackit.cloud/machine-should-be-migrated")) + Expect(c.Get(ctx, client.ObjectKey{Name: machine.Name, Namespace: machine.Namespace}, unchangedMigratedMachine)).To(Succeed()) + Expect(unchangedMigratedMachine).To(Equal(expectedMigratedMachine)) + + migratedWorker := &extensionsv1alpha1.Worker{} + Expect(c.Get(ctx, client.ObjectKey{Name: w.Name, Namespace: w.Namespace}, migratedWorker)).To(Succeed()) + Expect(migratedWorker.Annotations).To(HaveKeyWithValue("stackit.cloud/machine-controller-manager-migrated", "true")) + }) }) It("should fail because the infrastructure status cannot be decoded", func() { From ffb7b66fd27a8d236fddce5a355555ce4709609f Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Mon, 7 Sep 2026 13:20:59 +0200 Subject: [PATCH 16/17] make fmt --- pkg/controller/worker/actuator.go | 4 ++-- pkg/controller/worker/machines.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/controller/worker/actuator.go b/pkg/controller/worker/actuator.go index bcded0d4..7076da2a 100644 --- a/pkg/controller/worker/actuator.go +++ b/pkg/controller/worker/actuator.go @@ -14,8 +14,6 @@ import ( gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" gardener "github.com/gardener/gardener/pkg/client/kubernetes" - openstackclient "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/openstack/client" - "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/client-go/kubernetes" @@ -26,6 +24,8 @@ import ( "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/apis/stackit/helper" stackitv1alpha1 "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/apis/stackit/v1alpha1" + openstackclient "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/openstack/client" + "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit" stackitclient "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit/client" ) diff --git a/pkg/controller/worker/machines.go b/pkg/controller/worker/machines.go index 455f0a78..bacc71e0 100644 --- a/pkg/controller/worker/machines.go +++ b/pkg/controller/worker/machines.go @@ -25,6 +25,7 @@ import ( "github.com/gardener/gardener/pkg/client/kubernetes" gardenutils "github.com/gardener/gardener/pkg/utils" machinev1alpha1 "github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1" + iaas2 "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -35,7 +36,6 @@ import ( "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/openstack" "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/stackit" stackitutils "github.com/stackitcloud/gardener-extension-provider-stackit/v2/pkg/utils" - iaas2 "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" ) const ( From af6bcea038613bcbcd2c6d5531c3eeac790a17f6 Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Mon, 7 Sep 2026 13:51:03 +0200 Subject: [PATCH 17/17] make check fix< --- pkg/controller/worker/machines_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/worker/machines_test.go b/pkg/controller/worker/machines_test.go index 86167d3a..fda9ebc7 100644 --- a/pkg/controller/worker/machines_test.go +++ b/pkg/controller/worker/machines_test.go @@ -1248,7 +1248,7 @@ var _ = Describe("Machines", func() { }) DescribeTable("customLabelDomain in machineclass helm chart", func(customDomain string) { - workerDelegate, _ := NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customDomain) + workerDelegate, _ := NewWorkerDelegate(c, scheme, chartApplier, "", w, cluster, customDomain, nil) machineClassPath := filepath.Join("internal", "machineclass") if useStackitMCM {