ETCD-319: adding transient systemd unit for etcd

[dusk125]

This is the first step to get etcd running un-bundled from MicroShift; this change makes etcd run in a transient systemd unit whose lifetime is bound to the MicroShift execution. Also, as a temporary measure, I added a subcommand to MicroShift that just runs the bundled etcd; this is so we can test etcd's execution with the systemd unit and this will be removed and replaced by an invocation of the etcd binary that will be laid down by an rpm.

The end goal, as described in this Microshift Enhancement is to remove the current embedded etcd service and replace it with a service that launches etcd in a transient systemd unit.

[dusk125]

@haircommander Would you mind taking a look at this; mainly that this will be able to do what I've outlined above please?

[haircommander]

added a review comment on the proposal, this doesn't seem like the right direction to me (though I certainly have missed many conversations)

[dusk125]

The most recent update adds @deads2k suggestion to have the transient systemd unit launch the currently bundled etcd so that we can start testing the systemd launching mechanism while we work to get the rpm with etcd laid down. Once we have etcd installing from an rpm, I'll update this to completely remove bundled etcd and edit the systemd-run command to launch the rpm etcd.

[deads2k]

this lgtm

Is there a specific reason it's a WIP?

[deads2k]

This PR as it stands

1. starts the transient systemd unit
2. ties the systemd unit to the lifecycle of microshift.service so termination of microshift graceful and non-graceful (kill -9) also terminates etcd
3. does not bring in a new RPM dependency (yet). that is desired in the near future, but this gives the proper systemd lifecycle and process separation to reduce the risk on introducing the new RPM
4. passes the existing configuration arguments to the separate etcd process

/lgtm

[dhellmann]

/hold

The team needs to deliver a build to the customer in the next week. Let's wait to bring this change in until at least that is completed.

[dusk125]

/retest

[pmtk]

/test e2e-openshift-conformance-sig-api-machinery

[pmtk]

This looks good to me, I'd like others to take a look
@dhellmann @pacevedom @copejon

[dhellmann]

We've been trying to improve the git log messages with new changes. You could consider adding some of the detail from the PR message to the commit message.

I had a couple of questions inline, but nothing I think should block this.

/hold cancel
/approve

I'll leave final approval for @pmtk.

[dhellmann]

The main thing we wanted was to always use the microshift-etcd binary in the same directory as the microshift binary, to support the developer use case. Does it matter if that developer binary is run in a systemd unit? Would always using system simplify anything below?

[pmtk]

That's a good idea. Only thing is that etcd logs would be separate, but I don't think we need them that much in day to day development

[dhellmann]

Maybe this is another area to simplify after the initial version is merged?

[dusk125]

IIRC, there was some pushback in a previous meeting about not executing etcd as a systemd unit in development, I don't remember the reason given.

[dhellmann]

I may have been worried that it wouldn't work, or wouldn't stop, or something. I don't have a preference, as long as it works. We can land it this way and experiment with removing it when we have some time.

[pmtk]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, dhellmann, dusk125, pmtk

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [dhellmann,pmtk]

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

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD 61e5389 and 2 for PR HEAD 3e6917b in total

[openshift-ci]

@dusk125: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/periodic-ocp-4.13-images	4c4f1f59026f99124b96176ed25027b83bedbf03	link	true	/test periodic-ocp-4.13-images

Full PR test history. Your PR dashboard.

Details

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.

[dusk125]

/reset-required

[dusk125]

/retest-required