fix: deletion defects and resource orphans - #17
Open
Herbaert wants to merge 15 commits into
Open
Conversation
…lers (cherry picked from commit bbb2711)
tuunit
requested changes
Aug 20, 2026
tuunit
requested changes
Aug 20, 2026
Comment on lines
+118
to
+126
| case machine == nil: | ||
| log.Info("StackitMachine has no owning Machine yet, waiting") | ||
| return ctrl.Result{}, nil | ||
| case cluster == nil: | ||
| log.Info("Machine has no owning Cluster yet, waiting") | ||
| return ctrl.Result{}, nil | ||
| case stackitCluster == nil: | ||
| log.Info("StackitCluster not found, waiting") | ||
| return ctrl.Result{}, nil |
Collaborator
There was a problem hiding this comment.
how or when can these cases effectively happen? as far as i know provider machines are owned by machines which in turn are owned by a cluster and they are created in that order
Cluster -> MachineDeployment -> Machine -> ProviderMachine
tuunit
requested changes
Aug 20, 2026
tuunit
requested changes
Aug 20, 2026
tuunit
reviewed
Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes six defects in the cluster and machine reconcilers where deletion could
hang and leave STACKIT resources orphaned, and where changes that should trigger
a reconcile did not. Also stops the manager from holding Secret contents in
memory.
Changes
both reconcilers moved behind the
DeletionTimestampbranch. The clusterreconciler tolerates a deleted owning
Clusterand takes its name from theownerReference; the machine reconciler finalizes with aCleanupSkippedwarning event when the
Machine,ClusterorStackitClusteris gone, sincewithout them there are neither credentials nor tags left to reach the server.
Only a genuinely absent owner is tolerated — a transient API error still fails
the reconcile rather than dropping a finalizer over a running server.
StackitClusterkeeps its finalizer and requeues whileMachines for thecluster still exist, so those machines can still resolve credentials and delete
their servers. Cluster API orders this correctly on its own, but a namespace
teardown or a direct delete bypasses that ordering.
through the deferred patch at the end of
Reconcile. A crash in betweenpreviously left a running server or load balancer behind an object carrying no
finalizer to clean it up.
StackitMachinedeletion resolves the server by tags whenstatus.instanceIDis empty, instead of treating empty status as proof that noVM exists.
StackitCluster→StackitMachinewatch resolves the owningClusterbefore matching
Machine.spec.clusterName, and label-selects server-side. Itpreviously matched against the
StackitClustername, which breaks whenever thetwo differ — as they always do for ClusterClass-generated infrastructure refs.
StackitClusterwhosecredentialsSecretRefpoints at the changed Secret,searching across namespaces because that ref's namespace is optional.
Previously, correcting an invalid Secret never re-reconciled the cluster,
because the failure path deliberately returns without a requeue.
Transformthat dropsDataand managed fields, withDisableForon theclient so the reads that need the real bytes go to the API server. No label
selector, unlike Cluster API's own equivalent: Cluster API only watches Secrets
it stamps itself, whereas the credentials and bastion cloud-init Secrets here
are user-created and unlabelled, so a selector would silently disable those
watches.
debug/references dropped from test comments; the README compatibility tablecorrected from the v1beta1 to the v1beta2 contract, which is what
metadata.yaml, the CRD labels and every import already declare.Notes
CacheandClientoptions, soevery Secret read is a live API call — one per cluster reconcile, two per
machine reconcile. CAPA and CAPG do not harden the Secret cache at all and
therefore pay no read cost; Cluster API core hardens it and compensates with a
second, caching client for hot reads. This provider takes the hardened side
without that compensation, which stays bounded and event-driven at the fleet
sizes in scope. Watching Secrets as
PartialObjectMetadataagainst a dedicatedmetadata cache would go one step further, at the cost of a second cache in
main.gofor a small remaining gain. The new credentials watch adds reconcileswhen that Secret changes.
verified to fail when its fix is reverted. Unit tests cover the new cache
options.
go build ./...,go test ./...andgolangci-lint run ./...pass;make manifestsproduces no diff. The e2e suite was not run.