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

Finalizer helper #291

Closed
nightkr opened this issue Jul 24, 2020 · 4 comments · Fixed by #475
Closed

Finalizer helper #291

nightkr opened this issue Jul 24, 2020 · 4 comments · Fixed by #475
Labels
api Api abstraction related runtime controller runtime related

Comments

@nightkr
Copy link
Member

nightkr commented Jul 24, 2020

Should:

  • Add finalizer to resource when first seen
  • Have a way to disambiguate apply (create/update) vs should finalize vs already finalized.
  • Remove finalizer after finalization is done

See #290 for motivation. Make sure to mention in Controller's documentation.

@nightkr nightkr added runtime controller runtime related api Api abstraction related labels Jul 24, 2020
@clux
Copy link
Member

clux commented Feb 10, 2021

@lfrancke
Copy link
Contributor

You're welcome to use any of our code.
I can also contribute myself but this is/was my first real Rust project so it's probably not in perfect shape.

@clux clux added the utils extra utility modules label Feb 21, 2021
@aquarhead
Copy link
Contributor

From what I understand, unless kube-runtime move away from the "all-in-one" reconcile function (to, for example, a "resource" based approach), the best it can provide is extracting checking of deletionTimestamp and removing finalizer according to result of a user provided function. Which would still require the user to set the finalizer correctly by hand during reconcile.

The function in my head would look like this:

async fn cleanup(root: CRD, ctx: Context) -> Result<ReconcilerAction, Err> {
  // delete external resource
}

Controller::new(api, lp)
  .owns(child_api, lp)
  .finalizer("example.com/finalizer", cleanup)
  .run(...)
  .await?;

Which should be fairly easy to implement, as it just injects a bit of logic before reconcile. I think even in this form it has value in educating finalizer as the ideal way for handling external resources, especially since this is the common use case for controller/operators.

This is the same as how "child resources" are handled currently. (unless I'm missing something?) Which requires user to manually ensure resources created during reconcile has correct labels so that it matches those used in Controller::owns(), not only to mention the ownerReferences has to be maintained manually as well.

should finalize vs already finalized.

I'm not sure this is possible/desired, since most likely you'll need external state to realize whether the clean up is finished or not. Unless you just mean from the k8s resource's point of view, in that case it should be trivial to just avoid calling the user-provided closure when the corresponding finalizer does not exist anymore.

@nightkr
Copy link
Member Author

nightkr commented Mar 8, 2021

We could provide a finalizable wrapper for reconcile, that looks something like this:

async fn finalizable<K, ApplyFut, CleanupFut>(api: &Api<K>, finalizer: &str, obj: K, apply: impl Fn(K) -> impl ApplyFut, cleanup: Fn(K) -> impl CleanupFut) -> Result<ReconcilerResult> {
    if obj.meta().finalizers.contains(finalizer) {
        if obj.meta().deletion_timestamp == None {
            apply(obj).await
        } else {
            let res = cleanup(obj).await?;
            // .. remove finalizer
            Ok(res)
        }
    } else {
        if obj.meta().deletion_timestamp == None {
            // add finalizer
        }
        Ok(ReconcilerResult { requeue_at: None })
    }
}

I could see eventually baking that into Controller, but IMO it'd be hard to do backwards-compatibly without turning the API into a bit of a footgun.

@clux clux removed the utils extra utility modules label Jun 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Api abstraction related runtime controller runtime related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants