Skip to content

Commit

Permalink
Merge pull request #617 from suleymanakbas91/nodestatus-removal
Browse files Browse the repository at this point in the history
OCPBUGS-32954: OCPBUGS-32753: Remove LVMVolumeGroupNodeStatus CRs when LVMCluster CR is removed
  • Loading branch information
openshift-merge-bot[bot] committed May 6, 2024
2 parents 2cc9880 + 0045d70 commit a0d9d60
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 34 deletions.
43 changes: 22 additions & 21 deletions internal/controllers/lvmcluster/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,26 +168,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
return ctrl.Result{}, fmt.Errorf("failed to introspect running pod image: %w", err)
}

result, reconcileError := r.reconcile(ctx, lvmCluster)

statusError := r.updateLVMClusterStatus(ctx, lvmCluster)

// Reconcile errors have higher priority than status update errors
if reconcileError != nil {
return result, reconcileError
} else if statusError != nil && !k8serrors.IsNotFound(statusError) {
return result, fmt.Errorf("failed to update LVMCluster status: %w", statusError)
} else {
return result, nil
}
}

// errors returned by this will be updated in the reconcileSucceeded condition of the LVMCluster
func (r *Reconciler) reconcile(ctx context.Context, instance *lvmv1alpha1.LVMCluster) (ctrl.Result, error) {
logger := log.FromContext(ctx)

// The resource was deleted
if !instance.DeletionTimestamp.IsZero() {
if !lvmCluster.DeletionTimestamp.IsZero() {
// Check for existing LogicalVolumes
lvsExist, err := r.logicalVolumesExist(ctx)
if err != nil {
Expand All @@ -197,19 +179,37 @@ func (r *Reconciler) reconcile(ctx context.Context, instance *lvmv1alpha1.LVMClu
if lvsExist {
waitForLVRemoval := time.Second * 10
err := fmt.Errorf("found PVCs provisioned by topolvm, waiting %s for their deletion", waitForLVRemoval)
r.WarningEvent(ctx, instance, EventReasonErrorDeletionPending, err)
r.WarningEvent(ctx, lvmCluster, EventReasonErrorDeletionPending, err)
// check every 10 seconds if there are still PVCs present
return ctrl.Result{RequeueAfter: waitForLVRemoval}, nil
}

logger.Info("processing LVMCluster deletion")
if err := r.processDelete(ctx, instance); err != nil {
if err := r.processDelete(ctx, lvmCluster); err != nil {
// check every 10 seconds if there are still PVCs present or the LogicalVolumes are removed
return ctrl.Result{Requeue: true}, fmt.Errorf("failed to process LVMCluster deletion: %w", err)
}
return reconcile.Result{}, nil
}

result, reconcileError := r.reconcile(ctx, lvmCluster)

statusError := r.updateLVMClusterStatus(ctx, lvmCluster)

// Reconcile errors have higher priority than status update errors
if reconcileError != nil {
return result, reconcileError
} else if statusError != nil && !k8serrors.IsNotFound(statusError) {
return result, fmt.Errorf("failed to update LVMCluster status: %w", statusError)
} else {
return result, nil
}
}

// errors returned by this will be updated in the reconcileSucceeded condition of the LVMCluster
func (r *Reconciler) reconcile(ctx context.Context, instance *lvmv1alpha1.LVMCluster) (ctrl.Result, error) {
logger := log.FromContext(ctx)

if updated := controllerutil.AddFinalizer(instance, lvmClusterFinalizer); updated {
if err := r.Client.Update(ctx, instance); err != nil {
return reconcile.Result{}, fmt.Errorf("failed to update LvmCluster with finalizer: %w", err)
Expand Down Expand Up @@ -421,6 +421,7 @@ func (r *Reconciler) processDelete(ctx context.Context, instance *lvmv1alpha1.LV
resourceDeletionList := []resource.Manager{
resource.TopoLVMStorageClass(),
resource.LVMVGs(),
resource.LVMVGNodeStatus(),
resource.CSIDriver(),
resource.VGManager(),
}
Expand Down
31 changes: 18 additions & 13 deletions internal/controllers/lvmcluster/controller_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,20 +206,7 @@ var _ = Describe("LVMCluster controller", func() {

Context("Reconciliation on deleting the LVMCluster CR", func() {
It("should reconcile LVMCluster CR deletion", func(ctx context.Context) {

// delete lvmVolumeGroupNodeStatus as it should be deleted by vgmanager
// and if it is present lvmcluster reconciler takes it as vg is present on node
// we will now remove the node which will cause the LVM cluster status to also lose that vg
By("confirming absence of lvm cluster CR and deletion of operator created resources")
Eventually(func(ctx context.Context) error {
return k8sClient.Get(ctx, client.ObjectKeyFromObject(lvmVolumeGroupNodeStatusIn), lvmVolumeGroupNodeStatusIn)
}).WithContext(ctx).Should(Succeed())
Expect(k8sClient.Delete(ctx, nodeIn)).Should(Succeed())
// deletion of LVMCluster CR and thus also the NodeStatus through the removal controller
Eventually(func(ctx context.Context) error {
return k8sClient.Get(ctx, client.ObjectKeyFromObject(lvmVolumeGroupNodeStatusIn), lvmVolumeGroupNodeStatusIn)
}).WithContext(ctx).Should(Satisfy(errors.IsNotFound))

// deletion of LVMCluster CR
By("deleting the LVMClusterCR")
Expect(k8sClient.Delete(ctx, lvmClusterOut)).Should(Succeed())
Expand Down Expand Up @@ -247,6 +234,11 @@ var _ = Describe("LVMCluster controller", func() {
return k8sClient.Get(ctx, lvmVolumeGroupName, lvmVolumeGroupOut)
}).WithContext(ctx).Should(Satisfy(errors.IsNotFound))

By("confirming absence of LVMVolumeGroupNodeStatus Resource")
Eventually(func(ctx context.Context) error {
return k8sClient.Get(ctx, client.ObjectKeyFromObject(lvmVolumeGroupNodeStatusIn), lvmVolumeGroupNodeStatusIn)
}).WithContext(ctx).Should(Satisfy(errors.IsNotFound))

By("confirming absence of TopolvmStorageClasses")
for _, scName := range scNames {
Eventually(func(ctx context.Context) error {
Expand All @@ -258,6 +250,19 @@ var _ = Describe("LVMCluster controller", func() {
Eventually(func(ctx context.Context) error {
return k8sClient.Get(ctx, lvmClusterName, lvmClusterOut)
}).WithContext(ctx).Should(Satisfy(errors.IsNotFound))

// create lvmVolumeGroupNodeStatus again to test the removal by the node controller
lvmVolumeGroupNodeStatusIn.ResourceVersion = ""
Expect(k8sClient.Create(ctx, lvmVolumeGroupNodeStatusIn)).Should(Succeed())
By("verifying NodeStatus is created again")
Eventually(func(ctx context.Context) error {
return k8sClient.Get(ctx, client.ObjectKeyFromObject(lvmVolumeGroupNodeStatusIn), lvmVolumeGroupNodeStatusIn)
}).WithContext(ctx).Should(Succeed())
// we will now remove the node which will trigger deletion of the NodeStatus through the node removal controller
Expect(k8sClient.Delete(ctx, nodeIn)).Should(Succeed())
Eventually(func(ctx context.Context) error {
return k8sClient.Get(ctx, client.ObjectKeyFromObject(lvmVolumeGroupNodeStatusIn), lvmVolumeGroupNodeStatusIn)
}).WithContext(ctx).Should(Satisfy(errors.IsNotFound))
})
})

Expand Down
104 changes: 104 additions & 0 deletions internal/controllers/lvmcluster/resource/lvm_volumegroupnodestatus.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/*
Copyright © 2023 Red Hat, Inc.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package resource

import (
"context"
"fmt"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

lvmv1alpha1 "github.com/openshift/lvm-operator/api/v1alpha1"

"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"
)

const (
lvmVGNodeStatusName = "lvmvgnodestatus"
)

func LVMVGNodeStatus() Manager {
return lvmVGNodeStatus{}
}

type lvmVGNodeStatus struct{}

// lvmVGNodeStatus unit satisfies resourceManager interface
var _ Manager = lvmVGNodeStatus{}

func (l lvmVGNodeStatus) GetName() string {
return lvmVGNodeStatusName
}

// EnsureCreated is a noop. This will be created by vg-manager.
func (l lvmVGNodeStatus) EnsureCreated(r Reconciler, ctx context.Context, cluster *lvmv1alpha1.LVMCluster) error {
return nil
}

func (l lvmVGNodeStatus) EnsureDeleted(r Reconciler, ctx context.Context, _ *lvmv1alpha1.LVMCluster) error {
logger := log.FromContext(ctx).WithValues("resourceManager", l.GetName())
vgNodeStatusList := &lvmv1alpha1.LVMVolumeGroupNodeStatusList{}
if err := r.List(ctx, vgNodeStatusList, client.InNamespace(r.GetNamespace())); err != nil {
return fmt.Errorf("failed to list LVMVolumeGroupNodeStatus: %w", err)
}

var volumeGroupNodeStatusesPendingDelete []string
for _, nodeItem := range vgNodeStatusList.Items {
var volumeGroupsPendingDelete []string
for _, item := range nodeItem.Spec.LVMVGStatus {
lvmVolumeGroup := &lvmv1alpha1.LVMVolumeGroup{
ObjectMeta: metav1.ObjectMeta{
Name: item.Name,
Namespace: r.GetNamespace(),
},
}

if err := r.Get(ctx, client.ObjectKeyFromObject(lvmVolumeGroup), lvmVolumeGroup); err != nil {
if errors.IsNotFound(err) {
continue
}
return err
}

logger.Info("waiting for LVMVolumeGroup to be deleted", "volumeGroup", client.ObjectKeyFromObject(lvmVolumeGroup),
"finalizers", lvmVolumeGroup.GetFinalizers(), "LVMVolumeGroupNodeStatusName", nodeItem.GetName())

volumeGroupsPendingDelete = append(volumeGroupsPendingDelete, lvmVolumeGroup.GetName())
}

if len(volumeGroupsPendingDelete) > 0 {
volumeGroupNodeStatusesPendingDelete = append(volumeGroupNodeStatusesPendingDelete, nodeItem.GetName())
continue
}

if !nodeItem.GetDeletionTimestamp().IsZero() {
return fmt.Errorf("the LVMVolumeGroupNodeStatus %s is still present, waiting for deletion", nodeItem.GetName())
}
if err := r.Delete(ctx, &nodeItem); err != nil {
return fmt.Errorf("failed to delete LVMVolumeGroupNodeStatus %s: %w", nodeItem.GetName(), err)
}

logger.Info("initiated LVMVolumeGroupNodeStatus deletion", "LVMVolumeGroupNodeStatusName", nodeItem.GetName())
}

if len(volumeGroupNodeStatusesPendingDelete) > 0 {
return fmt.Errorf("waiting for LVMVolumeGroups to be removed before removing LVMVolumeGroupNodeStatuses: %v", volumeGroupNodeStatusesPendingDelete)
}

return nil
}

0 comments on commit a0d9d60

Please sign in to comment.