Skip to content

lvmcluster: Clean up stale node finalizers and annotations #1937

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.

Sign up for GitHub

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

Open
jlhuilier-1a wants to merge 1 commit into
base: main
Choose a base branch
from
Open

lvmcluster: Clean up stale node finalizers and annotations #1937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 3 additions & 39 deletions internal/controllers/lvmcluster/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"errors"
"fmt"
"os"
"strings"
"time"

snapapi "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1"
Expand All @@ -31,7 +30,6 @@ import (
"github.com/openshift/lvm-operator/v4/internal/controllers/constants"
"github.com/openshift/lvm-operator/v4/internal/controllers/lvmcluster/logpassthrough"
"github.com/openshift/lvm-operator/v4/internal/controllers/lvmcluster/resource"
"github.com/openshift/lvm-operator/v4/internal/controllers/vgmanager"
topolvmv1 "github.com/topolvm/topolvm/api/v1"
corev1 "k8s.io/api/core/v1"
storagev1 "k8s.io/api/storage/v1"
Expand Down Expand Up @@ -173,16 +171,13 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu

// The resource was deleted
if !lvmCluster.DeletionTimestamp.IsZero() {
// check for stale vgmanager finalizer in case if node deleted but vg still exist

// Get cluster nodes for deletion checks
nodes, err := r.clusterNodes(ctx)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to get unhealthy nodes: %w", err)
return ctrl.Result{}, fmt.Errorf("failed to get cluster nodes: %w", err)
}

err = r.checkStaleNodeFinalizers(ctx, nodes)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to check for deleted nodes: %w", err)
}
// Check for existing LogicalVolumes
lvsExist, err := r.logicalVolumesExist(ctx, nodes)
if err != nil {
Expand Down Expand Up @@ -439,37 +434,6 @@ func (r *Reconciler) logicalVolumesExist(ctx context.Context, nodes map[string]s
return false, nil
}

func (r *Reconciler) checkStaleNodeFinalizers(ctx context.Context, nodes map[string]struct{}) error {
volumeGroups := &lvmv1alpha1.LVMVolumeGroupList{}
err := r.List(ctx, volumeGroups, &client.ListOptions{Namespace: r.Namespace})
if k8serrors.IsNotFound(err) {
return nil
}

if err != nil {
return fmt.Errorf("failed to list volume groups: %w", err)
}

for _, vg := range volumeGroups.Items {
for _, finalizer := range vg.Finalizers {
if !strings.HasPrefix(finalizer, vgmanager.NodeCleanupFinalizer) {
continue
}

nodeName := strings.TrimPrefix(finalizer, vgmanager.NodeCleanupFinalizer+"/")
if _, ok := nodes[nodeName]; !ok {
controllerutil.RemoveFinalizer(&vg, finalizer)
err = r.Update(ctx, &vg)
if err != nil {
return fmt.Errorf("failed to delete finalizer from volumegroup %s, error: %w", vg.Name, err)
}
}
}
}

return nil
}

func (r *Reconciler) processDelete(ctx context.Context, instance *lvmv1alpha1.LVMCluster) error {
if controllerutil.ContainsFinalizer(instance, lvmClusterFinalizer) {
resourceDeletionList := []resource.Manager{
Expand Down
85 changes: 82 additions & 3 deletions internal/controllers/node/removal/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ import (

lvmv1alpha1 "github.com/openshift/lvm-operator/v4/api/v1alpha1"
"github.com/openshift/lvm-operator/v4/internal/controllers/constants"
"github.com/openshift/lvm-operator/v4/internal/controllers/vgmanager"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/utils/ptr"
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/handler"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
Expand All @@ -33,9 +35,11 @@ func NewReconciler(client client.Client, namespace string) *Reconciler {
//+kubebuilder:rbac:groups=lvm.topolvm.io,resources=lvmvolumegroupnodestatuses,verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups=lvm.topolvm.io,resources=lvmvolumegroupnodestatuses/status,verbs=get;update;patch
//+kubebuilder:rbac:groups=lvm.topolvm.io,resources=lvmvolumegroupnodestatuses/finalizers,verbs=update
//+kubebuilder:rbac:groups=lvm.topolvm.io,resources=lvmvolumegroups,verbs=get;list;watch;update;patch

// Reconcile takes care of watching a LVMVolumeGroupNodeStatus, and reacting to a node removal request by deleting
// the unwanted LVMVolumeGroupNodeStatus that was associated with the node.
// the unwanted LVMVolumeGroupNodeStatus that was associated with the node, as well as cleaning up any finalizers
// and annotations on LVMVolumeGroups that reference the deleted node.
// It does nothing on active Nodes. If it can be assumed that there will always be only one node (SNO),
// this controller should not be started.
func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
Expand All @@ -51,9 +55,21 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
err := r.Get(ctx, client.ObjectKeyFromObject(nodeStatus), node)

if errors.IsNotFound(err) {
logger.Info("node not found, removing LVMVolumeGroupNodeStatus", "node", nodeStatus.GetName())
nodeName := nodeStatus.GetName()
logger.Info("node not found, cleaning up node resources", "node", nodeName)

// Clean up finalizers and annotations for this specific node
if err := r.cleanupNodeFinalizers(ctx, nodeName); err != nil {
return ctrl.Result{}, fmt.Errorf("failed to cleanup finalizers for node %s: %w", nodeName, err)
}

if err := r.cleanupNodeAnnotations(ctx, nodeName); err != nil {
return ctrl.Result{}, fmt.Errorf("failed to cleanup annotations for node %s: %w", nodeName, err)
}

// Delete the NodeStatus CR
if err := r.Delete(ctx, nodeStatus); err != nil {
return ctrl.Result{}, fmt.Errorf("error deleting LVMVolumeGroupNodeStatus for Node %s: %w", nodeStatus.GetName(), err)
return ctrl.Result{}, fmt.Errorf("error deleting LVMVolumeGroupNodeStatus for Node %s: %w", nodeName, err)
}
logger.Info("initiated LVMVolumeGroupNodeStatus deletion", "nodeStatus", client.ObjectKeyFromObject(nodeStatus))

Expand Down Expand Up @@ -113,3 +129,66 @@ func removeDeleteProtectionFinalizer(status *lvmv1alpha1.LVMVolumeGroupNodeStatu
}
return false
}

// cleanupNodeFinalizers removes finalizers for the deleted node from all LVMVolumeGroups
func (r *Reconciler) cleanupNodeFinalizers(ctx context.Context, nodeName string) error {
logger := log.FromContext(ctx)

volumeGroups := &lvmv1alpha1.LVMVolumeGroupList{}
if err := r.List(ctx, volumeGroups, &client.ListOptions{Namespace: r.Namespace}); err != nil {
if errors.IsNotFound(err) {
return nil
}
return fmt.Errorf("failed to list volume groups: %w", err)
}

finalizerToRemove := vgmanager.NodeCleanupFinalizer + "/" + nodeName

for _, vg := range volumeGroups.Items {
if controllerutil.ContainsFinalizer(&vg, finalizerToRemove) {
controllerutil.RemoveFinalizer(&vg, finalizerToRemove)
if err := r.Update(ctx, &vg); err != nil {
return fmt.Errorf("failed to remove finalizer from volumegroup %s: %w", vg.Name, err)
}
logger.Info("removed node finalizer from VolumeGroup",
"volumeGroup", vg.Name,
"node", nodeName)
}
}

return nil
}

// cleanupNodeAnnotations removes wiped-devices annotations for the deleted node from all LVMVolumeGroups
func (r *Reconciler) cleanupNodeAnnotations(ctx context.Context, nodeName string) error {
logger := log.FromContext(ctx)

volumeGroups := &lvmv1alpha1.LVMVolumeGroupList{}
if err := r.List(ctx, volumeGroups, &client.ListOptions{Namespace: r.Namespace}); err != nil {
if errors.IsNotFound(err) {
return nil
}
return fmt.Errorf("failed to list volume groups: %w", err)
}

annotationKey := constants.DevicesWipedAnnotationPrefix + nodeName

for _, vg := range volumeGroups.Items {
if vg.Annotations == nil {
continue
}

if _, exists := vg.Annotations[annotationKey]; exists {
delete(vg.Annotations, annotationKey)
if err := r.Update(ctx, &vg); err != nil {
return fmt.Errorf("failed to remove annotation from volumegroup %s: %w", vg.Name, err)
}
logger.Info("removed wiped-devices annotation from VolumeGroup",
"volumeGroup", vg.Name,
"node", nodeName,
"annotation", annotationKey)
}
}

return nil
}