-
Notifications
You must be signed in to change notification settings - Fork 15
Resource Deletion Concerns #101
Comments
As for the proposal to implement a Webhook Admission controller to prevent modification of the |
Any other thoughts on this? |
Hi @sebastianroesch, thanks for raising this issue. This is something we have been thinking about internally for a few weeks since we had an incident in which the GitTrack managing our Forgive me if I have misunderstood, but #98 and #100 aim to solve the same problem as far as I can tell? When designing Faros, we had intended for Faros to be the owner of each of the resources and therefore, if the GitTrack were to be deleted, the child resources should also be deleted. There are however, occasions where you may not want this to happen as you have raised. I like the idea of #100 personally and think this would probably be the best way to go about solving this issue. We could add the There are three cases to handle then:
What do you think of this plan? Does this make sense? Unfortunately, there is a kind of prerequisite to this work being done. This would be a change in the API of the I'm semi tempted to make a release branch that we can continue to release on while we work on the migration, allowing us to start work on these breaking changes but leave the migration logic until later, would that work do you think? |
I like I think the idea of separating #98 and #100 is just to iterate and get the simplest change possible into Maybe I'm missing something regarding kubernetes-sigs/controller-runtime#351, but could we just make I'd like to avoid branching further. I'm a big fan of keeping one code line as much as possible. I realize that this is tricky with larger changes as well as the async nature of OSS. I still think we should try and break this down into smaller changes if possible and keep them in |
Naming is incredibly hard! Do we have any alternate suggestions for these names? Do they need changing? Are
Yeah you're right here, I'm being a bit overly cautious! I was wondering what would happen if you start adding random fields to objects when they aren't in the OpenAPI schema, turns out, nothing! It just accepts them anyway. So yeah, we can totally just add this field to the existing API group, ignore what I said earlier 🙈
Yep, I agree completely, was just trying to think of a nice way to workaround the issue of waiting for the API group to bump but we don't need to so Ignore that. Thanks for the sanity check btw @tshak ! |
Thanks @JoelSpeed for following up on this. #98 is concerned with the cascade-deletion of all resources when the
I like the name
Maybe I'm not understanding correctly how Finalizers and GC works. The
My impression is that this makes sense, except for cases where, as proposed, an |
So I think we can kill 2 birds with 1 stone here 😄
The reason I think we can Kill two birds with one stone is because of the Finalizers! So, the theory here is that we make our own cascade using the If Does that make any more sense?
Where is this proposed? I don't think we should be modifying the existing behaviour of adding |
I finally found the time to process this, and I think it's a good proposal. This should address all our concerns. Thanks @JoelSpeed for the clarification. |
We're concerned with resources being deleted in production environments. We consider the current Faros approach a risk and therefore have introduced proposals that would mitigate this risk.
This issue summarises those concerns and proposals. I'm looking forward to your take on those and we're happy to work on those proposals.
GitTrack
resource deletes all resources managed by Faros: Prevent deleting all deployed resources when GitTrack resources deleted #98--cascade=false
not working with Faros: Faros does not seem to respect --cascade=false when deleting GitTrack resource #99Our previous issue #92 already hinted at some of those concerns, but addresses the wrong issue.
The text was updated successfully, but these errors were encountered: