test_concurrency makes no sense anymore. There's no batching since the
join_loop was refactored. The same is true for test_uuids.

Georgy Moshkin authored 2 years ago and Georgy Moshkin committed 2 years ago

deactivation means that instance
- is demoted to learner (if it wasn't one already)
- is marked "inactive" so that it's ignored when determining
    the number of voters required for the cluster

Yaroslav Dynnikov authored 2 years ago and Yaroslav Dynnikov committed 2 years ago

Bootstrapping the replication was implemented in 2dac77c5. But it was
configured on the new instance only. The old instance (that joined
earlier) couldn't update `box.cfg({replication})` until now.

Close https://git.picodata.io/picodata/picodata/picodata/-/issues/52

If proc_discover is invoked after raft node was initialized but before
raft leader was elected, it would return an error before this commit.
Because of that it was impossible to restart the whole cluster at once.

This commit change proc_discover such that in case leader_id is not
ready, the normal discovery algorithm takes place.

Closes #93

Restarting both instances doesn't work yet, to be fixed later.

Close https://git.picodata.io/picodata/picodata/picodata/-/issues/90

Since commit d87dd4ca `leader_id` became an `Option`, so the `None`
value isn't rendered in the `picolib.raft_status` response:

```python
status={'is_ready': False, 'raft_state': 'Follower', 'id': 1}
```

It makes pytest complain about missing argument:

```
cluster2 = Cluster("127.0.0.1:3300", n=2)

    def test_restart_leader(cluster2: Cluster):
        i1, i2 = cluster2.instances
        i1.assert_raft_status('Leader')
        i2.assert_raft_status('Follower')

        i1.restart()
>       i1.wait_ready()

test/int/test_joining.py:209:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../.local/share/virtualenvs/picodata-6sv6l6y-/lib/python3.10/site-packages/funcy/decorators.py:45:
in wrapper
    return deco(call, *dargs, **dkwargs)
../../.local/share/virtualenvs/picodata-6sv6l6y-/lib/python3.10/site-packages/funcy/flow.py:127:
in retry
    return call()
../../.local/share/virtualenvs/picodata-6sv6l6y-/lib/python3.10/site-packages/funcy/decorators.py:66:
in __call__
    return self._func(*self._args, **self._kwargs)
test/int/conftest.py:305: in wait_ready
    status = self._raft_status()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = Instance(i1, listen=127.0.0.1:3301)

    def _raft_status(self) -> RaftStatus:
        status = self.call("picolib.raft_status")
        assert isinstance(status, dict)
        eprint(f"{status=}")
>       return RaftStatus(**status)
E       TypeError: RaftStatus.__init__() missing 1 required positional
argument: 'leader_id'

test/int/conftest.py:280: TypeError
```

This patch fixes the failure message:

```
self = Instance(i1, listen=127.0.0.1:3301)

    @funcy.retry(tries=20, timeout=0.1)
    def wait_ready(self):
        status = self._raft_status()
>       assert status.is_ready
E       AssertionError: assert False
E        +  where False = RaftStatus(id=1, raft_state='Follower',
is_ready=False, leader_id=None).is_ready

test/int/conftest.py:306: AssertionError
```

* Make `--cluster-id` CLI mandatory.
* Handle cluster_id mismatch in raft_join.
  When an instance attempts to join the cluster and the instances's
  `--instance-id` parameter mismatches the cluster_id of the cluster
  an error is raised inside the raft_join handler.

Pytest has a feature to segregate setup, test, and teardown logs. The
setup phase is considered to be an intialization of the fixtures. In
order to split logs properly `cluster.deploy()` is now called inside a
fixture.

It's already formatted in conformity to usual `cargo test`.
Also, remove unused command-line arguments from `picodata test` command.

Close https://git.picodata.io/picodata/picodata/picodata/-/issues/61

The intention is to eliminate ambiguities in the `Instance` API. Make
it more like `subprocess` module (as regards `kill` and `terminate`
functions).

Behavior of `killpg` slightly differs in Mac and Linux. For some reason,
`killpg` returns error EPERM when sending a signal to a zomibie process.
And that is the reason of `test_process_management` failure on mac -
there's a small gap between killing child and and subreaper calls
`waitpid`.

Now pytest handles this exception properly.

Close https://git.picodata.io/picodata/picodata/picodata/-/issues/70

See also:

- Stackoverflow: Why would `killpg` return "not permitted" when ownership is correct?
  https://stackoverflow.com/questions/12521705/why-would-killpg-return-not-permitted-when-ownership-is-correct

- Linux `man 2 killpg`:
  https://linux.die.net/man/2/killpg#Notes

  > Notes
  >
  > There are various differences between the permission checking in
  > BSD-type systems and System V-type systems. See the POSIX rationale
  > for kill(). A difference not mentioned by POSIX concerns the return
  > value EPERM: BSD documents that no signal is sent and EPERM returned
  > when the permission check failed for **at least one** target process,
  > while POSIX documents EPERM only when the permission check failed for
  > **all** target processes.

- MacOS `man 2 killpg`:
  https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/killpg.2.html

  > [EPERM] The sending process is not the super-user and
  >         **one or more** of the target processes has an effective
  >         user ID different from that of the sending process.

- Linux `man 2 kill`:
  https://linux.die.net/man/2/kill

  >  EPERM  The process does not have permission to send the signal
  >         *to any* of the target processes.
  >

- Process states in Linux:
  https://kerneltalks.com/linux/process-states-in-linux/

- Reproduce killpg returning EPERM on MacOS:
  https://git.picodata.io/picodata/picodata/picodata/-/snippets/7

Pytest supports running tests in parallel using the `xdist` plugin. In
order to support it in Picodata, one should avoid ports collision. It
assigns each worker a dedicated IP address `127.7.n.1`, where
`n = xdist_worker_number`. Unfortunately, it doesn't work on MacOS,
because Mac doesn't provide any loopback aliases except `127.0.0.1`
by default.

This patch provides another address generation logics. The `subnet`
parameter is superseeded with a `base_port`, that is `3300 + n * 100`.
In this way, every pytest (xdist) worker gets a dedicated port range
`[3301, 3399]`, `[3401, 3499]` and so on.

Close https://git.picodata.io/picodata/picodata/picodata/-/issues/65

When bootstrapping an instance, there're two possible execution paths -
`start_boot` and `start_join`. While `start_join` takes all uuids from
JoinResponse, `start_boot` already deals with a bootstrapped `box.cfg`
(it's done in `start_discover`, refer to [1]). In order to make uuids
consistent across `box.cfg` and topology module, `start_boot` stage is
preceded with rebootstrap.

This case is also covered with a pytest.

- [1] doc/clustering.md

Follow-up for https://git.picodata.io/picodata/picodata/picodata/-/issues/50

Implementation of `net_box` in `tarantool-module` resolves hostnames
with a `to_socket_addrs` function that is blocking. Pytest uses
fake addresses in one test, and sometimes it results in 5-second
blockage and consequent test failure.

This patch only provides a workaround. It makes a connection to fail
even before the blocking DNS request.

See also:
- https://git.picodata.io/picodata/picodata/tarantool-module/-/issues/81

Before this patch, pytest used to launch all instances in a clean
environment. It prevented running with `PICODATA_LOG_LEVEL=verbose`.

Yaroslav Dynnikov authored 2 years ago and Yaroslav Dynnikov committed 2 years ago

This patch covers one more case when discovery request is handled
by an instance that has the discovery module unitialized.