Support leader election for Connect-Inject controller #903
Conversation
| @@ -19,7 +19,7 @@ metadata: | |||
| heritage: {{ .Release.Service }} | |||
| release: {{ .Release.Name }} | |||
| spec: | |||
| replicas: 1 | |||
| replicas: 3 | |||
There was a problem hiding this comment.
curious what our thoughts are on this? will 2 cause election issues?
wondering if we bound it to # of server replicas or something, in case this is a demo environment?
There was a problem hiding this comment.
2 should also be fine. I put up 3 as a shot in the dark. the number of instances here are irrelevant. If you have a single instance running, it will assume it is the leader as it does not use a "consensus" protocol to establish leadership.
There was a problem hiding this comment.
Ah, interesting!
Okay thanks for the clarification. I'm just trying to think of how this might look if we have a demo environment where we've got a single kind node or something and then three of these try to spin up? 2 even seems too high in that case, but would be ideal in any other case. thoughts?
There was a problem hiding this comment.
I think 2 makes more sense to me. Demo env is tough, it definitely doesn't need 2 but folks won't change this for prod either and we default to 3 servers for the same reason. We could handle demo env another way maybe via profiles.
There was a problem hiding this comment.
Yeah, we're definitely pretty inconsistent. Although, if I were to guess it seems that we're leaning towards making it prod-ready by default rather than demo by default because servers and all gateways start with multiple instances. The only components that don't are sync-catalog, connect-injector, and controller. Sync catalog and injector could not run more than one instance, and for the controller we could probably switch too especially if we're thinking to combine it with the endpoints controller.
I think it'd make sense to default to a more HA setting (with 2 instances).
There was a problem hiding this comment.
Agree! 2 sounds good then!
kschoche
force-pushed
the
tproxy-remove-cleanup-hc-controller-flags
branch
from
0f42db9 to
ea056a4
Compare
thisisnotashwin
force-pushed
the
add-leader-election
branch
from
265adb5 to
799f32d
Compare
thisisnotashwin
force-pushed
the
add-leader-election
branch
from
799f32d to
74a8bcb
Compare
thisisnotashwin
force-pushed
the
add-leader-election
branch
from
74a8bcb to
01ad8c1
Compare
- Leader election required creating a configmap that is owned by the instance of the controller that is the leader. This required RBAC to perform CRUD operations on configmaps to the controller. - Additionally the latest version of the controller runtime requires the controller to read "leases" from kubernetes and we now have to support that for the connect-inject controller. - Increase the number of replicas of the connect inject deployment to 3 as we can now be HA.
thisisnotashwin
force-pushed
the
add-leader-election
branch
from
01ad8c1 to
6a91a8c
Compare
* Support leader election for Connect-Inject controller - Leader election required creating a configmap that is owned by the instance of the controller that is the leader. This required RBAC to perform CRUD operations on configmaps to the controller. - Additionally the latest version of the controller runtime requires the controller to read "leases" from kubernetes and we now have to support that for the connect-inject controller. - Increase the number of replicas of the connect inject deployment to 2 as we can now be HA.
* Support leader election for Connect-Inject controller - Leader election required creating a configmap that is owned by the instance of the controller that is the leader. This required RBAC to perform CRUD operations on configmaps to the controller. - Additionally the latest version of the controller runtime requires the controller to read "leases" from kubernetes and we now have to support that for the connect-inject controller. - Increase the number of replicas of the connect inject deployment to 2 as we can now be HA.
Changes proposed in this PR: