This causes you to skip over the switch statement below, and in particular the "case found && !send" so you do not end up deleting the namespace in an underlying cluster when you should.

oops, a mistake. will fix. Thanks for catching this.

nit: Why not just 60 seconds, or one minute?

I kind of forgot why i did it like that. will change.

This seems very short, and requires exponential backoff. There are standard libraries to do the right thing here.

We should be initializing the controller-manager as early as possible. controller-manager depends on api-server and they both are started independently. So either one may start functioning before the other. If we introduce exponential backoff, i am worried it may induce unnecessary delay in initializing controller-manager. Also it is not the critical path and we are just in initialization stage.
I would increase the apiserverRetryInterval to 2 seconds. and the apiserverWaitTimeout to 2 minutes. Is this fine?

ok

This needs to be configurable, and should be an explicitly reserved name. As it stands right now, if the user has a namespace called federation in any of their underlying clusters, these will be automatically deleted (because what you're saying here is that you only want it in the federation, not in any of the underlying clusters).

Agree with your comment. I would make it configurable. Also what could be the name for such a namespace in case of default. is federation name fine? or should i changed it to something like federation-only

Yes, I think federation-only is fine as a default.

You should only do this if the user has requested an HA federation control plane, otherwise you have an unnecessary namespace hanging around, for no reason.

Agree with the concern, but even in case of non-HA federation control plane, it has one purpose to wait for the Api Server and to establish it is working fine. In the current scenario, without this. the federation-controller-manager dies and another pod is brought up. This is documented in this issue #44490

Now creating the federation-only namespace only in HA scenario. PTAL

ok, thanks.

This should be called "ensureFederationNamespace" because that's what it does. If the namespace already exists, it does nothing, otherwise it tries to create it.

Agree, will change

Thanks.

I don't understand why this is needed. It's already in run()?

agree, its an overkill. will remove it.

This is normal, expected behaviour, right? In that case we should not be logging a fatal error here.

So you die if you lose the leader election here? Are relying on Kubernetes to automatically restart this pod to try again? This seems to introduce potential delays and also it seems brittle. What if Kubernetes decides to stop restarting this pod because it's failed too often? And what if, because this pod is dead, Kubernetes decides to schedule another pod on this node, and uses up all the resources? I guess in summary I don't understand the design for how leader election is supposed to work. Can you point me to the design doc?

if the instance which is leader loses leader election it would die and restart. potentially it has encountered some issues due to which it could not renew the leadership lease. Also it should be ok if it restarts as there are standby instances which could acquire lease immediately after one is available.
@quinton-hoole, there was no design for leader-election for controller-manager as it was thought to be trivial. Already there are multiple examples available on how to implement it. here is the one for kube-controller-manager

However there is Federation control plane HA design, which we need to revisit in v1.8 and gather acceptance from community. https://docs.google.com/document/d/18kB5W401WBhEjUOOXQnBb-nx3bFXK8pOCHbMbdXuBMg/edit?usp=sharing

OK, that makes sense. If the kubelet doesn't restart the pod, then the replicset controller will, perhaps on a different node. Sorry, I was being dumb.