Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌱 Update names of finalizers. #1455

Merged
merged 13 commits into from
Aug 22, 2024
7 changes: 5 additions & 2 deletions api/v1beta1/hcloudmachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@ import (
)

const (
// MachineFinalizer allows ReconcileHCloudMachine to clean up HCloud
// HCloudMachineFinalizer allows ReconcileHCloudMachine to clean up HCloud
// resources associated with HCloudMachine before removing it from the
// apiserver.
MachineFinalizer = "hcloudmachine.infrastructure.cluster.x-k8s.io"
HCloudMachineFinalizer = "infrastructure.cluster.x-k8s.io/hcloudmachine"
// DeprecatedHCloudMachineFinalizer contains the old string.
// The controller will automatically update to the new string.
DeprecatedHCloudMachineFinalizer = "hcloudmachine.infrastructure.cluster.x-k8s.io"
)

// HCloudMachineSpec defines the desired state of HCloudMachine.
Expand Down
4 changes: 0 additions & 4 deletions api/v1beta1/hcloudremediation_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ import (
)

const (
// HCloudRemediationFinalizer allows HCloudRemediationReconciler to clean up resources associated with HCloudRemediation before
// removing it from the apiserver.
HCloudRemediationFinalizer = "hcloudremediation.infrastructure.cluster.x-k8s.io"

// HCloudRebootAnnotation indicates that a bare metal host object should be rebooted.
HCloudRebootAnnotation = "reboot.hcloud.infrastructure.cluster.x-k8s.io"
)
Expand Down
18 changes: 11 additions & 7 deletions api/v1beta1/hetznerbaremetalhost_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ const (
// BareMetalHostFinalizer is the name of the finalizer added to
// hosts to block delete operations until the physical host can be
// deprovisioned.
BareMetalHostFinalizer string = "hetznerbaremetalhost.infrastructure.cluster.x-k8s.io"
BareMetalHostFinalizer = "infrastructure.cluster.x-k8s.io/hetznerbaremetalhost"
guettli marked this conversation as resolved.
Show resolved Hide resolved

// DeprecatedBareMetalHostFinalizer contains the old string.
// The controller will automatically update to the new string.
DeprecatedBareMetalHostFinalizer = "hetznerbaremetalhost.infrastructure.cluster.x-k8s.io"

// HostAnnotation is the key for an annotation that should go on a HetznerBareMetalMachine to
// reference what HetznerBareMetalHost it corresponds to.
Expand Down Expand Up @@ -132,17 +136,17 @@ const (

const (
// ErrorMessageMissingRootDeviceHints specifies the error message when no root device hints are specified.
ErrorMessageMissingRootDeviceHints string = "no root device hints specified"
ErrorMessageMissingRootDeviceHints = "no root device hints specified"
// ErrorMessageInvalidRootDeviceHints specifies the error message when invalid root device hints are specified.
ErrorMessageInvalidRootDeviceHints string = "invalid root device hints specified"
ErrorMessageInvalidRootDeviceHints = "invalid root device hints specified"
// ErrorMessageMissingHetznerSecret specifies the error message when no Hetzner secret is found.
ErrorMessageMissingHetznerSecret string = "could not find HetznerSecret"
ErrorMessageMissingHetznerSecret = "could not find HetznerSecret"
// ErrorMessageMissingRescueSSHSecret specifies the error message when no RescueSSH secret is found.
ErrorMessageMissingRescueSSHSecret string = "could not find RescueSSHSecret"
ErrorMessageMissingRescueSSHSecret = "could not find RescueSSHSecret"
// ErrorMessageMissingOSSSHSecret specifies the error message when no OSSSH secret is found.
ErrorMessageMissingOSSSHSecret string = "could not find OSSSHSecret"
ErrorMessageMissingOSSSHSecret = "could not find OSSSHSecret"
// ErrorMessageMissingOrInvalidSecretData specifies the error message when no data in secret is missing or invalid.
ErrorMessageMissingOrInvalidSecretData string = "invalid or not specified information in secret"
ErrorMessageMissingOrInvalidSecretData = "invalid or not specified information in secret"
)

// ProvisioningState defines the states of provisioning of the host.
Expand Down
6 changes: 5 additions & 1 deletion api/v1beta1/hetznerbaremetalmachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ import (
const (
// BareMetalMachineFinalizer allows Reconcilehetznerbaremetalmachine to clean up resources associated with hetznerbaremetalmachine before
// removing it from the apiserver.
BareMetalMachineFinalizer = "hetznerbaremetalmachine.infrastructure.cluster.x-k8s.io"
BareMetalMachineFinalizer = "infrastructure.cluster.x-k8s.io/hetznerbaremetalmachine"
guettli marked this conversation as resolved.
Show resolved Hide resolved

// DeprecatedBareMetalMachineFinalizer contains the old string.
// The controller will automatically update to the new string.
DeprecatedBareMetalMachineFinalizer = "hetznerbaremetalmachine.infrastructure.cluster.x-k8s.io"

// BareMetalHostNamePrefix is a prefix for all hostNames of bare metal servers.
BareMetalHostNamePrefix = "bm-"
Expand Down
4 changes: 0 additions & 4 deletions api/v1beta1/hetznerbaremetalremediation_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ import (
)

const (
// RemediationFinalizer allows HetznerBareMetalRemediationReconciler to clean up resources associated with HetznerBareMetalRemediation before
// removing it from the apiserver.
RemediationFinalizer = "hetznerbaremetalremediation.infrastructure.cluster.x-k8s.io"

// RebootAnnotation indicates that a bare metal host object should be rebooted.
RebootAnnotation = "reboot.hetznerbaremetalhost.infrastructure.cluster.x-k8s.io"

Expand Down
9 changes: 7 additions & 2 deletions api/v1beta1/hetznercluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,15 @@ import (
)

const (
// ClusterFinalizer allows ReconcileHetznerCluster to clean up HCloud
// HetznerClusterFinalizer allows ReconcileHetznerCluster to clean up HCloud
// resources associated with HetznerCluster before removing it from the
// apiserver.
ClusterFinalizer = "hetznercluster.infrastructure.cluster.x-k8s.io"
HetznerClusterFinalizer = "infrastructure.cluster.x-k8s.io/hetznercluster"

// DeprecatedHetznerClusterFinalizer contains the old string.
// The controller will automatically update to the new string.
DeprecatedHetznerClusterFinalizer = "hetznercluster.infrastructure.cluster.x-k8s.io"

// AllowEmptyControlPlaneAddressAnnotation allows HetznerCluster Webhook
// to skip some validation steps for externally managed control planes.
AllowEmptyControlPlaneAddressAnnotation = "capi.syself.com/allow-empty-control-plane-address"
Expand Down
6 changes: 4 additions & 2 deletions controllers/hcloudmachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ func (r *HCloudMachineReconciler) reconcileDelete(ctx context.Context, machineSc
return result, nil
}
// Machine is deleted so remove the finalizer.
controllerutil.RemoveFinalizer(machineScope.HCloudMachine, infrav1.MachineFinalizer)
controllerutil.RemoveFinalizer(machineScope.HCloudMachine, infrav1.HCloudMachineFinalizer)
controllerutil.RemoveFinalizer(machineScope.HCloudMachine, infrav1.DeprecatedHCloudMachineFinalizer)

return reconcile.Result{}, nil
}
Expand All @@ -196,7 +197,8 @@ func (r *HCloudMachineReconciler) reconcileNormal(ctx context.Context, machineSc
hcloudMachine := machineScope.HCloudMachine

// If the HCloudMachine doesn't have our finalizer, add it.
controllerutil.AddFinalizer(machineScope.HCloudMachine, infrav1.MachineFinalizer)
controllerutil.AddFinalizer(machineScope.HCloudMachine, infrav1.HCloudMachineFinalizer)
controllerutil.RemoveFinalizer(machineScope.HCloudMachine, infrav1.DeprecatedHCloudMachineFinalizer)

// Register the finalizer immediately to avoid orphaning HCloud resources on delete.
if err := machineScope.PatchObject(ctx); err != nil {
Expand Down
3 changes: 2 additions & 1 deletion controllers/hcloudmachinetemplate_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ func (r *HCloudMachineTemplateReconciler) Reconcile(ctx context.Context, req rec
}()

// removing finalizer that was set in previous versions but is not needed
controllerutil.RemoveFinalizer(machineTemplate, infrav1.MachineFinalizer)
// We can remove that code in 2025.
controllerutil.RemoveFinalizer(machineTemplate, infrav1.DeprecatedHCloudMachineFinalizer)

// Check whether owner is a ClusterClass. In that case there is nothing to do.
if hasOwnerClusterClass(machineTemplate.ObjectMeta) {
Expand Down
2 changes: 1 addition & 1 deletion controllers/hcloudremediation_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ var _ = Describe("HCloudRemediationReconciler", func() {
}, timeout).Should(BeTrue())
})

It("should delete machine if retry limit reached and reboot timed out", func() {
It("should delete machine if retry limit reached and reboot timed out (hcloud)", func() {
By("creating hcloudRemediation")
Expect(testEnv.Create(ctx, hcloudRemediation)).To(Succeed())

Expand Down
29 changes: 12 additions & 17 deletions controllers/hetznerbaremetalhost_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
Expand All @@ -46,7 +47,6 @@ import (
robotclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/robot"
sshclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/ssh"
"github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/host"
"github.com/syself/cluster-api-provider-hetzner/pkg/utils"
)

// HetznerBareMetalHostReconciler reconciles a HetznerBareMetalHost object.
Expand Down Expand Up @@ -87,12 +87,12 @@ func (r *HetznerBareMetalHostReconciler) Reconcile(ctx context.Context, req ctrl
}

// Add a finalizer to newly created objects.
if bmHost.DeletionTimestamp.IsZero() && !hostHasFinalizer(bmHost) {
bmHost.Finalizers = append(bmHost.Finalizers,
infrav1.BareMetalHostFinalizer)
if bmHost.DeletionTimestamp.IsZero() &&
(controllerutil.AddFinalizer(bmHost, infrav1.BareMetalHostFinalizer) ||
controllerutil.RemoveFinalizer(bmHost, infrav1.DeprecatedBareMetalHostFinalizer)) {
err := r.Update(ctx, bmHost)
if err != nil {
return reconcile.Result{}, fmt.Errorf("failed to add finalizer: %w", err)
return reconcile.Result{}, fmt.Errorf("failed to update finalizer: %w", err)
}
return ctrl.Result{Requeue: true}, nil
}
Expand Down Expand Up @@ -230,21 +230,20 @@ func (r *HetznerBareMetalHostReconciler) reconcileSelectedStates(ctx context.Con
if needsUpdate {
err := r.Update(ctx, bmHost)
if err != nil {
return reconcile.Result{}, fmt.Errorf("failed to add finalizer: %w", err)
return reconcile.Result{}, fmt.Errorf("UPdate() failed after setting ProvisioningState: %w", err)
guettli marked this conversation as resolved.
Show resolved Hide resolved
}
}

return ctrl.Result{RequeueAfter: 10 * time.Second}, nil

// Handle StateDeleting
case infrav1.StateDeleting:
if !utils.StringInList(bmHost.Finalizers, infrav1.BareMetalHostFinalizer) {
return reconcile.Result{Requeue: true}, nil
}

bmHost.Finalizers = utils.FilterStringFromList(bmHost.Finalizers, infrav1.BareMetalHostFinalizer)
if err := r.Update(ctx, bmHost); err != nil {
return reconcile.Result{}, fmt.Errorf("failed to remove finalizer: %w", err)
if controllerutil.RemoveFinalizer(bmHost, infrav1.BareMetalHostFinalizer) ||
controllerutil.RemoveFinalizer(bmHost, infrav1.DeprecatedBareMetalHostFinalizer) {
// at least one finalizer was removed.
if err := r.Update(ctx, bmHost); err != nil {
return reconcile.Result{}, fmt.Errorf("failed to remove finalizer: %w", err)
}
}
return reconcile.Result{Requeue: true}, nil
}
Expand Down Expand Up @@ -411,10 +410,6 @@ func hetznerSecretErrorResult(
return reconcile.Result{}, fmt.Errorf("hetznerSecretErrorResult: an unhandled failure occurred: %T %w", err, err)
}

func hostHasFinalizer(host *infrav1.HetznerBareMetalHost) bool {
return utils.StringInList(host.Finalizers, infrav1.BareMetalHostFinalizer)
}

// SetupWithManager sets up the controller with the Manager.
func (r *HetznerBareMetalHostReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
return ctrl.NewControllerManagedBy(mgr).
Expand Down
3 changes: 2 additions & 1 deletion controllers/hetznerbaremetalmachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,15 @@ func (r *HetznerBareMetalMachineReconciler) reconcileDelete(ctx context.Context,
}
// Machine is deleted so remove the finalizer.
controllerutil.RemoveFinalizer(machineScope.BareMetalMachine, infrav1.BareMetalMachineFinalizer)
controllerutil.RemoveFinalizer(machineScope.BareMetalMachine, infrav1.DeprecatedBareMetalMachineFinalizer)

return result, nil
}

func (r *HetznerBareMetalMachineReconciler) reconcileNormal(ctx context.Context, machineScope *scope.BareMetalMachineScope) (reconcile.Result, error) {
// If the HetznerBareMetalMachine doesn't have our finalizer, add it.
controllerutil.AddFinalizer(machineScope.BareMetalMachine, infrav1.BareMetalMachineFinalizer)

controllerutil.RemoveFinalizer(machineScope.BareMetalMachine, infrav1.DeprecatedBareMetalMachineFinalizer)
guettli marked this conversation as resolved.
Show resolved Hide resolved
// Register the finalizer immediately to avoid orphaning HetznerBareMetal resources on delete
if err := machineScope.PatchObject(ctx); err != nil {
return reconcile.Result{}, err
Expand Down
2 changes: 1 addition & 1 deletion controllers/hetznerbaremetalremediation_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ var _ = Describe("HetznerBareMetalRemediationReconciler", func() {
}, timeout).Should(BeTrue())
})

It("should delete machine if retry limit reached and reboot timed out", func() {
It("should delete machine if retry limit reached and reboot timed out (bm)", func() {
By("creating hetznerBareMetalRemediation object")
Expect(testEnv.Create(ctx, hetznerBareMetalRemediation)).To(Succeed())

Expand Down
6 changes: 4 additions & 2 deletions controllers/hetznercluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,8 @@ func (r *HetznerClusterReconciler) reconcileNormal(ctx context.Context, clusterS
hetznerCluster := clusterScope.HetznerCluster

// If the HetznerCluster doesn't have our finalizer, add it.
controllerutil.AddFinalizer(hetznerCluster, infrav1.ClusterFinalizer)
controllerutil.AddFinalizer(hetznerCluster, infrav1.HetznerClusterFinalizer)
controllerutil.RemoveFinalizer(hetznerCluster, infrav1.DeprecatedHetznerClusterFinalizer)
guettli marked this conversation as resolved.
Show resolved Hide resolved
if err := clusterScope.PatchObject(ctx); err != nil {
return reconcile.Result{}, err
}
Expand Down Expand Up @@ -366,7 +367,8 @@ func (r *HetznerClusterReconciler) reconcileDelete(ctx context.Context, clusterS
}

// Cluster is deleted so remove the finalizer.
controllerutil.RemoveFinalizer(clusterScope.HetznerCluster, infrav1.ClusterFinalizer)
controllerutil.RemoveFinalizer(clusterScope.HetznerCluster, infrav1.HetznerClusterFinalizer)
controllerutil.RemoveFinalizer(clusterScope.HetznerCluster, infrav1.DeprecatedHetznerClusterFinalizer)

return reconcile.Result{}, nil
}
Expand Down
14 changes: 9 additions & 5 deletions pkg/secrets/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ import (

const (
// SecretFinalizer is the finalizer for secrets.
SecretFinalizer = infrav1.ClusterFinalizer + "/secret"
SecretFinalizer = "infrastructure.cluster.x-k8s.io/caph-secret"

// DeprecatedSecretFinalizer contains the old string.
// The controller will automatically update to the new string.
DeprecatedSecretFinalizer = infrav1.DeprecatedHetznerClusterFinalizer + "/secret"
)

// SecretManager is a type for fetching Secrets whether or not they are in the
Expand Down Expand Up @@ -91,8 +95,8 @@ func (sm *SecretManager) claimSecret(ctx context.Context, secret *corev1.Secret,
}
}

if addFinalizer && !utils.StringInList(secret.Finalizers, SecretFinalizer) {
secret.Finalizers = append(secret.Finalizers, SecretFinalizer)
if addFinalizer && (controllerutil.AddFinalizer(secret, SecretFinalizer) ||
controllerutil.RemoveFinalizer(secret, DeprecatedSecretFinalizer)) {
needsUpdate = true
}

Expand Down Expand Up @@ -180,8 +184,8 @@ func (sm *SecretManager) ReleaseSecret(ctx context.Context, secret *corev1.Secre

// remove finalizer from secret to allow deletion if no other owner exists
if !foundOtherHetznerClusterOwner {
secret.Finalizers = utils.FilterStringFromList(
secret.Finalizers, SecretFinalizer)
controllerutil.RemoveFinalizer(secret, SecretFinalizer)
controllerutil.RemoveFinalizer(secret, DeprecatedSecretFinalizer)
}

secret.OwnerReferences = newOwnerRefs
Expand Down
4 changes: 3 additions & 1 deletion pkg/services/baremetal/host/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
"sigs.k8s.io/cluster-api/util/record"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1"
Expand Down Expand Up @@ -1803,7 +1804,8 @@ func (s *Service) actionDeprovisioning(_ context.Context) actionResult {
}

func (s *Service) actionDeleting(_ context.Context) actionResult {
s.scope.HetznerBareMetalHost.Finalizers = utils.FilterStringFromList(s.scope.HetznerBareMetalHost.Finalizers, infrav1.BareMetalHostFinalizer)
controllerutil.RemoveFinalizer(s.scope.HetznerBareMetalHost, infrav1.BareMetalHostFinalizer)
controllerutil.RemoveFinalizer(s.scope.HetznerBareMetalHost, infrav1.DeprecatedBareMetalHostFinalizer)
return deleteComplete{}
}

Expand Down
11 changes: 0 additions & 11 deletions pkg/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,17 +172,6 @@ func StringInList(list []string, strToSearch string) bool {
return false
}

// FilterStringFromList produces a new string slice that does not
// include the strToFilter argument.
func FilterStringFromList(list []string, strToFilter string) (newList []string) {
for _, item := range list {
if item != strToFilter {
newList = append(newList, item)
}
}
return
}

// GenerateName takes a name as string pointer. It returns name if pointer is not nil, otherwise it returns fallback with random suffix.
func GenerateName(name *string, fallback string) string {
if name != nil {
Expand Down
23 changes: 0 additions & 23 deletions pkg/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,29 +177,6 @@ var _ = Describe("StringInList", func() {
}))
})

var _ = Describe("FilterStringFromList", func() {
type testCaseFilterStringFromList struct {
list []string
str string
expectedOutcome []string
}
DescribeTable("Test filter string from list",
func(tc testCaseFilterStringFromList) {
out := utils.FilterStringFromList(tc.list, tc.str)
Expect(out).To(Equal(tc.expectedOutcome))
},
Entry("entry1", testCaseFilterStringFromList{
list: []string{"a", "b", "c"},
str: "a",
expectedOutcome: []string{"b", "c"},
}),
Entry("entry2", testCaseFilterStringFromList{
list: []string{"a", "b", "c"},
str: "d",
expectedOutcome: []string{"a", "b", "c"},
}))
})

var _ = Describe("Test removeOwnerRefFromList", func() {
type testCaseRemoveOwnerRefFromList struct {
RefList []metav1.OwnerReference
Expand Down
Loading