Run a cleanup plan when deleting instances

[nfnt]

What this PR does / why we need it:
If an operator provides a cleanup plan, this plan is run when the operator's instance is deleted from the cluster. This is achieved by detecting an instance deletion in the controller and adding a finalizer. If there is another plan currently in progress, this will be superseded by the cleanup plan. This allows the deletion of instances where a plan fails to complete.

Fixes #1046

[alenkacz]

I would like to keep the controller code as compact as possible since it's the most important loop we have and isolate all the logic into the helper methods :) But other than that looking good!

Also there's one more case I need to think about to be sure we don't get stuck there :)

• we have finalizer on instance
• someone deletes the instance
• we run cleanup plan
• we store status with the finished plan execution
• request to remove the finalizer fails and we retry the reconcile
• now we don't re-run cleanup because it was already run but nothing is preventing e.g. other plan to be run here? https://github.com/kudobuilder/kudo/pull/1108/files#diff-45e4638e2c718ccd5d08635baec770b1R179

I am basically just trying to think if there's a way that we can remain stuck with the finalizer in, which is the most frustrating thing :) but I think we might be fine :)

[alenkacz]

One more thing :)

[nfnt]

Thanks for the detailed case that isn't covered yet by the code. I'll change the logic to look like this:

• First, only add the finalizer if the cleanup plan isn't complete.
• Later, if there is a finalizer, remove it if the cleanup plan is complete.
  This is close to the current approach that checks if the last completed plan was the cleanup plan. But by chosing a more general approach, we'll cover reconciliation edge cases, like the one you described.

[alenkacz]

Okay, I am just at the edge of approving 😛 sorry. I would just like to minimize the update requests from 2 to 1 since I think that's possible. The rest is just nits :)

[alenkacz]

Thanks, nice job! Don't forget about docs, please :)

[ANeumann82]

Looks good to me. One thing I was thinking about was changes in operators:

• When a new operatorVersion contains a cleanup plan that was not existing in the previous version, the finalizer should be added automatically on the next reconcile, correct?
• If a cleanup plan gets removed from an operatorVersion, the finalizer still needs to be removed at some point. I've added a comment at maybeRemoveFinalizer. It's and edge case though.

[nfnt]

@ANeumann82 Yes, the finalizer will be added automatically. But with the current code it won't get removed automatically. That's a good catch and is easy to fix, I'll add a fix.

[kensipe]

My greatest concern here isn't with this code (which lgtm!).
We are developing a list of special plan names which we need to make absolutely clear to operator developers. the name "deploy" and now "cleanup" are special... they are not arbitrary. We need to capture in our documentation these details and the semantic behaviors of these special named plans.

[kensipe]

LGTM

[nfnt]

@kensipe 💯, this needs to be clear from the documentation (that I'll work on today). Also, operator devs should expect that the cleanup plan may fail, e.g. when trying to remove resources that haven't been created yet because the operator is stuck in deploy.