Skip to content

Commit 6600f3a

Browse files
committed
feat: GCP-503 Implement OrphanDeleter for GCP
Removes finalizers from orphaned GCPMachines when WIF credentials become invalid to prevent cluster teardown getting stuck. The implementation is similar to the existing AWS implementation.
1 parent bb1af6d commit 6600f3a

2 files changed

Lines changed: 84 additions & 0 deletions

File tree

hypershift-operator/controllers/hostedcluster/internal/platform/gcp/gcp.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,12 @@ import (
3333
"k8s.io/apimachinery/pkg/api/meta"
3434
"k8s.io/apimachinery/pkg/api/resource"
3535
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
36+
utilerrors "k8s.io/apimachinery/pkg/util/errors"
3637
"k8s.io/utils/ptr"
3738

3839
capigcp "sigs.k8s.io/cluster-api-provider-gcp/api/v1beta1"
3940
capiv1 "sigs.k8s.io/cluster-api/api/core/v1beta1"
41+
ctrl "sigs.k8s.io/controller-runtime"
4042
"sigs.k8s.io/controller-runtime/pkg/client"
4143

4244
"github.com/blang/semver"
@@ -532,6 +534,34 @@ func (p GCP) validateWorkloadIdentityConfiguration(hcluster *hyperv1.HostedClust
532534
return nil
533535
}
534536

537+
func (GCP) DeleteOrphanedMachines(ctx context.Context, c client.Client, hc *hyperv1.HostedCluster, controlPlaneNamespace string) error {
538+
if ValidCredentials(hc) {
539+
return nil
540+
}
541+
gcpMachineList := capigcp.GCPMachineList{}
542+
if err := c.List(ctx, &gcpMachineList, client.InNamespace(controlPlaneNamespace)); err != nil {
543+
return fmt.Errorf("failed to list GCPMachines in %s: %w", controlPlaneNamespace, err)
544+
}
545+
logger := ctrl.LoggerFrom(ctx)
546+
var errs []error
547+
for i := range gcpMachineList.Items {
548+
gcpMachine := &gcpMachineList.Items[i]
549+
if gcpMachine.DeletionTimestamp.IsZero() ||
550+
len(gcpMachine.Finalizers) == 0 {
551+
continue
552+
}
553+
554+
gcpMachine.Finalizers = []string{}
555+
if err := c.Update(ctx, gcpMachine); err != nil {
556+
errs = append(errs, fmt.Errorf("failed to delete machine %s/%s: %w", gcpMachine.Namespace, gcpMachine.Name, err))
557+
continue
558+
}
559+
logger.Info("removed finalizers of gcpmachine because of invalid AWS identity provider", "machine", client.ObjectKeyFromObject(gcpMachine))
560+
561+
}
562+
return utilerrors.NewAggregate(errs)
563+
}
564+
535565
// setCondition updates or creates a condition on the HostedCluster.
536566
// This follows the standard HyperShift pattern for condition management.
537567
func setCondition(hcluster *hyperv1.HostedCluster, conditionType hyperv1.ConditionType, status metav1.ConditionStatus, reason, message string) {

hypershift-operator/controllers/hostedcluster/internal/platform/gcp/gcp_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,3 +416,57 @@ func TestReconcileGCPClusterPreservesServerDefaultedFields(t *testing.T) {
416416
g.Expect(gcpCluster.Spec.Network.Subnets[0].Name).To(Equal("test-subnet"))
417417
g.Expect(gcpCluster.Spec.Network.Subnets[0].Region).To(Equal("us-central1"))
418418
}
419+
420+
func TestDeleteOrphanedMachines(t *testing.T) {
421+
g := NewWithT(t)
422+
423+
platform := New("test-utilities-image", "test-capg-image", &semver.Version{Major: 4, Minor: 17, Patch: 0})
424+
hc := validHostedCluster()
425+
426+
// Create a scheme with both HyperShift and CAPG types
427+
scheme := runtime.NewScheme()
428+
g.Expect(clientgoscheme.AddToScheme(scheme)).To(Succeed())
429+
g.Expect(hyperv1.AddToScheme(scheme)).To(Succeed())
430+
g.Expect(capigcp.AddToScheme(scheme)).To(Succeed())
431+
fakeClient := fake.NewClientBuilder().
432+
WithObjects(
433+
&capigcp.GCPMachine{
434+
ObjectMeta: metav1.ObjectMeta{
435+
Name: "test-xl-1",
436+
Namespace: "test-control-plane-namespace",
437+
DeletionTimestamp: ptr.To(metav1.Now()),
438+
Finalizers: []string{"test", "testz"},
439+
},
440+
},
441+
// GCPMachines without deletionTimestamp should not get their finalizers removed.
442+
&capigcp.GCPMachine{
443+
ObjectMeta: metav1.ObjectMeta{
444+
Name: "test-xl-2",
445+
Namespace: "test-control-plane-namespace",
446+
Finalizers: []string{"test", "testz"},
447+
},
448+
},
449+
).
450+
WithScheme(scheme).
451+
Build()
452+
453+
err := platform.DeleteOrphanedMachines(
454+
t.Context(),
455+
fakeClient,
456+
hc,
457+
"test-control-plane-namespace",
458+
)
459+
g.Expect(err).To(BeNil())
460+
461+
gcpMachineList := &capigcp.GCPMachineList{}
462+
err = fakeClient.List(t.Context(), gcpMachineList)
463+
g.Expect(err).To(BeNil())
464+
465+
for _, gcpMachine := range gcpMachineList.Items {
466+
if !gcpMachine.DeletionTimestamp.IsZero() {
467+
g.Expect(gcpMachine.Finalizers).To(BeEmpty())
468+
} else {
469+
g.Expect(gcpMachine.Finalizers).ToNot(BeEmpty())
470+
}
471+
}
472+
}

0 commit comments

Comments
 (0)