Adoption design

[matthchr]

A design for how the operator will "adopt" resources that existed in Azure before they were created in Kubernetes.

[matthchr]

I'm honestly pretty torn on this one. @sakthi-vetrivel curious about your thoughts on this as well.

[jonnylangefeld]

If would have a vote on this I'd vote for option 1. That's what I would expect from a controller and I guess the whole point of a "smart guess for deletion policy" is to match what a user would expect.
I think it makes it more homogenous across all resources in a set of resources you apply and also doesn't leave room for surprises. I don't think we have to protect the users from deletion, because it is equivalent to ARM templates or SDKs as you said. Since there's no UI involved there's not going to be a "are you sure you want to delete?" pop-up, but someone intentionally calls the delete API endpoint. In my opinion it's the same whether I call the ARM API, use the SDK or cal a k8s API endpoint to delete a resource.

[jonnylangefeld]

fully agree with this.

[theunrepentantgeek]

I fully agree that there's no ambiguity that kubectl delete means remove the resource from the cluster.

That said, I do think that different users will have different expectations about whether deletion from the cluster necessarily requires deletion from Azure as a natural consequence.

[matthchr]

This is related to #1465

[theunrepentantgeek]

I fully agree that there's no ambiguity that kubectl delete means remove the resource from the cluster.

That said, I do think that different users will have different expectations about whether deletion from the cluster necessarily requires deletion from Azure as a natural consequence.

[nojnhuh]

Would something like reconcile-policy: skip, reconcile-policy-if-not-exists: manage be possible to express using this method? What would happen in that case for a resource that does not already exist?

(Asking generally, I don't think we'd do something like that in CAPZ)

[matthchr]

Great question. I've expanded the disadvantages section to talk about two types of possible mitigation here (up from 1).

The second option is to allow both and have reconcile-policy-if-exists always overwrite reconcile-policy. The only downside I see to this is that it could cause some gitops flows issue as we might overwrite the annotation they provided with the annotation in reconcile-policy-if-exists, causing their checked-in spec to differ from the active spec (annotation mismatch). It's a minor issue but at the same time I am not clear on what exact scenarios users would really want to set both in practice, for example your example scenario feels pretty contrived. One that seems possibly valid is: reconcile-policy: detach-on-delete and reconcile-policy-if-exists: skip...

I've updated the decision section to lean in the direction of the first option (webhook prevention) as we can always expand it later but we can't undo letting users set both if we do it now, but I'm open to other opinions on that.

[theunrepentantgeek]

I agree with your conclusions.

[codecov-commenter]

Codecov Report

Merging #2067 (83de00e) into main (16489fa) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2067      +/-   ##
==========================================
- Coverage   54.48%   54.48%   -0.01%     
==========================================
  Files        1228     1228              
  Lines      542444   542444              
==========================================
- Hits       295552   295534      -18     
- Misses     198530   198544      +14     
- Partials    48362    48366       +4     

see 8 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[matthchr]

/ok-to-test sha=9537ae7

[matthchr]

/ok-to-test sha=9537ae7