etcd icon indicating copy to clipboard operation
etcd copied to clipboard

Published 20 hours ago • verified publisher icon etcd-io

functional: remove SIGQUIT_ETCD_AND_REMOVE_DATA_AND_STOP_AGENT command

Open lavacat opened this issue 3 years ago • 6 comments

Problem: both SIGQUIT_ETCD_AND_REMOVE_DATA_AND_STOP_AGENT and test.sh will attempt to stop agents and remove directories.

Solution: since test.sh creates directories and starts test, it should be responsible for cleanup.

See https://github.com/etcd-io/etcd/issues/14384

Signed-off-by: Bogdan Kanivets [email protected]

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

lavacat avatar Aug 25 '22 22:08 lavacat

@lavacat please fix the fmt pipeline error

ahrtr avatar Aug 25 '22 23:08 ahrtr

It seems that the etcd data directories will never be removed? What will happen if we restart the functional test when it fails?

ahrtr avatar Sep 13 '22 02:09 ahrtr

It seems that the etcd data directories will never be removed? What will happen if we restart the functional test when it fails?

This line in test.sh will remove data dirs at the start of the tests. https://github.com/etcd-io/etcd/blob/main/scripts/test.sh#L159

lavacat avatar Sep 13 '22 06:09 lavacat

It seems that the etcd data directories will never be removed? What will happen if we restart the functional test when it fails?

This line in test.sh will remove data dirs at the start of the tests. https://github.com/etcd-io/etcd/blob/main/scripts/test.sh#L159

I am thinking probably it makes more sense to stop etcd and cleanup the data if the test is successful; otherwise keep the data. Proposed changes:

  1. Update the (*Cluster) Run to return an error, and TestFunctional sends a Operation_SIGQUIT_ETCD_AND_REMOVE_DATA to the agent if the returned error is nil.
  2. The test may panic, so you need to use recover() to check whether (*Cluster) Run is successful or not.

ahrtr avatar Sep 15 '22 20:09 ahrtr

This line in test.sh will remove data dirs at the start of the tests. https://github.com/etcd-io/etcd/blob/main/scripts/test.sh#L159

This should be just a remedy instead of a formal solution. Please consider to update this PR per https://github.com/etcd-io/etcd/pull/14387#issuecomment-1248574599

ahrtr avatar Sep 21 '22 08:09 ahrtr

@lavacat It's OK to resolve the comment in a separate PR, so that we can get this one merged firstly. Please let me know whether you want to resolve it in this PR or in a separate PR.

ahrtr avatar Sep 22 '22 09:09 ahrtr

Since there is no response for a long time, so let me merge this PR firstly and take care of the comment in a separate PR.

ahrtr avatar Oct 09 '22 00:10 ahrtr