Skip to content

Commit

Permalink
Handle finalizer removal (#471)
Browse files Browse the repository at this point in the history
* Handle finalizer removal

Signed-off-by: John Collier <[email protected]>

* Remove finalizers on next reconcile

Signed-off-by: John Collier <[email protected]>

* Fix typo

Signed-off-by: John Collier <[email protected]>

---------

Signed-off-by: John Collier <[email protected]>
  • Loading branch information
johnmcollier authored May 9, 2024
1 parent cad2834 commit 55667ac
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 0 deletions.
33 changes: 33 additions & 0 deletions controllers/application_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@ import (

k8sErrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/util/retry"
"k8s.io/client-go/util/workqueue"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"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/handler"
"sigs.k8s.io/controller-runtime/pkg/predicate"
Expand All @@ -49,6 +51,8 @@ import (
logutil "github.com/redhat-appstudio/application-service/pkg/log"
)

const appFinalizerName = "application.appstudio.redhat.com/finalizer"

// ApplicationReconciler reconciles a Application object
type ApplicationReconciler struct {
client.Client
Expand Down Expand Up @@ -86,6 +90,25 @@ func (r *ApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
return reconcile.Result{}, err
}

// If the resource still has the finalizer attached to it, just remove it so deletion doesn't get blocked
if containsString(application.GetFinalizers(), appFinalizerName) {
// remove the finalizer from the list and update it.
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
var currentApplication appstudiov1alpha1.Application
err := r.Get(ctx, req.NamespacedName, &currentApplication)
if err != nil {
return err
}

controllerutil.RemoveFinalizer(&currentApplication, appFinalizerName)

err = r.Update(ctx, &currentApplication)
return err
})
if err != nil {
return ctrl.Result{}, err
}
}
log.Info(fmt.Sprintf("Starting reconcile loop for %v", req.NamespacedName))
// If devfile hasn't been generated yet, generate it
// If the devfile hasn't been generated, the CR was just created.
Expand Down Expand Up @@ -232,3 +255,13 @@ func (r *ApplicationReconciler) SetupWithManager(ctx context.Context, mgr ctrl.M
})).
Complete(r)
}

// Helper functions to check and remove string from a slice of strings.
func containsString(slice []string, s string) bool {
for _, item := range slice {
if item == s {
return true
}
}
return false
}
33 changes: 33 additions & 0 deletions controllers/application_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,39 @@ var _ = Describe("Application controller", func() {
})
})

Context("Application CR with finalizer", func() {
It("Should delete successfully", func() {
ctx := context.Background()

applicationName := HASAppName + "5"

hasApp := &appstudiov1alpha1.Application{
TypeMeta: metav1.TypeMeta{
APIVersion: "appstudio.redhat.com/v1alpha1",
Kind: "Application",
},
ObjectMeta: metav1.ObjectMeta{
Name: applicationName,
Namespace: HASAppNamespace,
Finalizers: []string{appFinalizerName},
},
Spec: appstudiov1alpha1.ApplicationSpec{
DisplayName: DisplayName,
Description: Description,
},
}

Expect(k8sClient.Create(ctx, hasApp)).Should(Succeed())

// Look up the has app resource that was created.
// num(conditions) may still be < 1 on the first try, so retry until at least _some_ condition is set
hasAppLookupKey := types.NamespacedName{Name: applicationName, Namespace: HASAppNamespace}

// Delete the specified resource
deleteHASAppCR(hasAppLookupKey)
})
})

})

// deleteHASAppCR deletes the specified hasApp resource and verifies it was properly deleted
Expand Down
23 changes: 23 additions & 0 deletions controllers/component_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/builder"
"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 @@ -58,6 +59,8 @@ import (
"github.com/spf13/afero"
)

const compFinalizerName = "component.appstudio.redhat.com/finalizer"

// ComponentReconciler reconciles a Component object
type ComponentReconciler struct {
client.Client
Expand Down Expand Up @@ -104,6 +107,26 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
return ctrl.Result{}, err
}

// If a resource still has the finalizer attached from it, just remove it so deletion doesn't get blocked
if containsString(component.GetFinalizers(), compFinalizerName) {
// remove the finalizer from the list and update it.
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
var currentComponent appstudiov1alpha1.Component
err := r.Get(ctx, req.NamespacedName, &currentComponent)
if err != nil {
return err
}

controllerutil.RemoveFinalizer(&currentComponent, compFinalizerName)

err = r.Update(ctx, &currentComponent)
return err
})
if err != nil {
return ctrl.Result{}, err
}
}

_, prevErrCondition := checkForCreateReconcile(component)

// Get the Application CR
Expand Down
44 changes: 44 additions & 0 deletions controllers/component_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1997,6 +1997,50 @@ var _ = Describe("Component controller", func() {
deleteHASAppCR(hasAppLookupKey)
})
})

Context("Component CR with finalizer", func() {
It("Should delete successfully", func() {
ctx := context.Background()

applicationName := HASAppName + "31"
componentName := HASCompName + "31"

createAndFetchSimpleApp(applicationName, HASAppNamespace, DisplayName, Description)

hasComp := &appstudiov1alpha1.Component{
TypeMeta: metav1.TypeMeta{
APIVersion: "appstudio.redhat.com/v1alpha1",
Kind: "Component",
},
ObjectMeta: metav1.ObjectMeta{
Name: componentName,
Namespace: HASAppNamespace,
Finalizers: []string{compFinalizerName},
},
Spec: appstudiov1alpha1.ComponentSpec{
ComponentName: ComponentName,
Application: applicationName,
Source: appstudiov1alpha1.ComponentSource{
ComponentSourceUnion: appstudiov1alpha1.ComponentSourceUnion{
GitSource: &appstudiov1alpha1.GitSource{
URL: SampleGitlabRepoLink,
},
},
},
},
}
Expect(k8sClient.Create(ctx, hasComp)).Should(Succeed())

hasCompLookupKey := types.NamespacedName{Name: componentName, Namespace: HASAppNamespace}
hasAppLookupKey := types.NamespacedName{Name: applicationName, Namespace: HASAppNamespace}

// Delete the specified HASComp resource
deleteHASCompCR(hasCompLookupKey)

// Delete the specified HASApp resource
deleteHASAppCR(hasAppLookupKey)
})
})
})

type updateChecklist struct {
Expand Down

0 comments on commit 55667ac

Please sign in to comment.