WIP: Introduce an etcd operator leader status field

[ironcladlou]

This patch explores adding an etcd operator status field which reports
leader member information. Exposing this would allow, for example, smarter
decision-making by the MCO regarding reboot ordering by providing a hint
to help minimize disruption via excessive leader changes.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ironcladlou
To complete the pull request process, please assign knobunc
You can assign the PR to them by writing /assign @knobunc in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ironcladlou]

See the discussion in openshift/machine-config-operator#1897 for more details about one way this could be useful.

[cgwalters]

Shouldn't this be e.g. NodeRef *corev1.ObjectReference ?

[deads2k]

Shouldn't this be e.g. NodeRef *corev1.ObjectReference ?

No. We encourage the creation of specific reference types. See the reasoning in https://github.com/kubernetes/api/blob/master/core/v1/types.go#L5172-L5186

@cgwalters

[cgwalters]

Ah, I see. Hmm. So, since there's nothing more we care about here to a node than its name, does that argue for keeping it as a string?

[openshift-ci-robot]

@ironcladlou: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/verify	06b1a7d	link	/test verify

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[ironcladlou]

P.S., this is for now to help promote discussion and experimentation — we haven't actually done the work/measurement to prove the cited use case has a benefit with expanding the API for

[deads2k]

@ironcladlou what about something like

status struct{
  conditions
  []EtcdMembers
}

EtcdMembers struct{
  name string
  node string
  status string // learning,unhealthy,not-a-member,healthy
  memberType string // leader,follower (whatever this is)
  fsyncP99InLastMinute string
  peerLatencyP99InLastMinute string

  // or whatever it is you want
}

[cgwalters]

OK so I'm about at the point in openshift/machine-config-operator#1897 work where I'm blocking on this, because I really want to test OS update things after we're only updating etcd followers first.

I starting looking at hacking something into the MCD for this but it's ugly.

[cgwalters]

If we don't land this my tentative thoughts are:

• Patch the MCD to detect if it's on control plane, if so start watching the local etcd and find out if the current node is a leader
• If it is a leader add mco.openshift.io/etcdleader: "" label to node, if not remove label if exists

In playing with this what I'm stumbling over is getting the right creds set up to talk to etcd, and I don't know if there's a non-polling way to watch for leader changes.

The rest of the logic for managing upgrade order would be in the MCC and proceed basically the same.

[cgwalters]

You know what may be far simpler (and avoid the "etcd writes its state to kube which writes to etcd" issue) is having the etcd pod write out something extremely simple like /run/etcd/status.json or whatever, then the MCD pod on the control plane can use inotify to watch it. Or we can set up some sort of local read-only communication channel.

Anyways for now I wrote some hacky code that reuses the certs from the apiserver in the MCD
openshift/machine-config-operator#1946

[cgwalters]

Or, rather than trying to extend the API here we could use a node annotation.

[openshift-bot]

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[openshift-bot]

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

[openshift-bot]

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

[openshift-ci-robot]

@openshift-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.