search

cilium / design-cfps

Repo to store Cilium CFP design docs
Apache License 2.0
31 stars 26 forks source link

CFP-27752: Operator Manages Cilium Identities #19

Closed hacktivist123 closed 2 months ago

hacktivist123 commented 9 months ago

This PR stores the Operator manages cilium identities CFP in Git to preserve it for future reference

Author of the CFP: @dlapcevic

joestringer commented 9 months ago

Thanks for preparing this. I think this can adopt CFP number 27752 under Cilium, per the CFP numbering guidelines and https://github.com/cilium/cilium/issues/27752.

hacktivist123 commented 9 months ago

Updated @joestringer

xmulligan commented 3 months ago

@dlapcevic are you still working on this CFP? Asking because we now have statuses for CFPs https://github.com/cilium/design-cfps?#status

dlapcevic commented 3 months ago

@dlapcevic are you still working on this CFP? Asking because we now have statuses for CFPs https://github.com/cilium/design-cfps?#status

We're still working on it. Most of it is done. cilium-operator code is merged. We'll be sending in the cilium-agent changes now.

xmulligan commented 3 months ago

Once that is done, would you mind updating it so we can merge it as released?

dlapcevic commented 3 months ago

Once that is done, would you mind updating it so we can merge it as released?

Yes. I'll do it. Thanks.

joestringer commented 3 months ago

Oh and also if you plan on moving forward with the code / PRs first and aiming to just update this CFP when it is at "released" status that's ~fine. Just note that anyone reviewing the code proposals with the CFP in the current state may become confused given the divergences between the implementation and the CFP. This was certainly a pain point for me as I tried to grasp how the PRs mapped to the CFP and what we really came to consensus on.

dlapcevic commented 3 months ago

Oh and also if you plan on moving forward with the code / PRs first and aiming to just update this CFP when it is at "released" status that's ~fine. Just note that anyone reviewing the code proposals with the CFP in the current state may become confused given the divergences between the implementation and the CFP. This was certainly a pain point for me as I tried to grasp how the PRs mapped to the CFP and what we really came to consensus on.

I see. I responded to your comments in order to clarify the potential confusion. All that we've done so far, and the biggest part of the implementation was in the cilium-operator, but I still see how it might cause confusion since we didn't completely clarify if cilium-operator updates CEPs or not, and if it creates CIDs based on Pod or CEP resources.

dlapcevic commented 2 months ago

Hi @hacktivist123, could you please help us move this forward, by updating the PR based on the few most recent comments?

Once it's done we can merge this with status "Implementable", which I expect soon to be done.

joestringer commented 2 months ago

@hacktivist123 @dlapcevic maybe it makes sense for you to open a fresh PR for this, and we close this one? I don't think it really makes sense for you to relay every change you want to make through @hacktivist123 just to update the CFP.

dlapcevic commented 2 months ago

@hacktivist123 @dlapcevic maybe it makes sense for you to open a fresh PR for this, and we close this one? I don't think it really makes sense for you to relay every change you want to make through @hacktivist123 just to update the CFP.

Sounds good. I'll open a new PR for it. Thanks.

dlapcevic commented 2 months ago

I created a new PR for this CFP: https://github.com/cilium/design-cfps/pull/59

I applied the suggestions from this PR. I removed parts of it that are not relevant to the actual implementation (Temp ID, CEP updates). I greatly reduced the size of it, as the scope was reduced and the discussion will be left here and in the CFP google doc.

xmulligan commented 2 months ago

Thanks, I'll close this one then