To unify the command handling, utilize EnvCmd() to handle rndc commands:
1. Remove isctest.rndc abstractions. They were intended for an upcoming
python-only implementation. A couple of years later, it doesn't seem
to be coming any time soon, so let's stick with the interface that
makes sense today, i.e. use the same command handling interface
everywhere.
2. Remove the specialized rndc.log in favor of the generic logging
already implemented by isctest.run.cmd(). I believe the cause of the
many rndc(log=False) invocations was that nobody wanted this extra
file. Yet, logging everything by default makes sense for debugging,
unless there's a good reason not to. In almost all cases, logging was
switched to the default (enabled).
3. With the NamedInstance.rndc() call now returning CmdResult rather
than combined stdout+stderr string, adjust all the invocations to use
`.out` or `.err` as necessary.
4. Replace some manual rndc invocation and its base argument
construction with the standardized nsX.rndc() call.
5. In cases where rndc is expected to fail, utilize
raise_on_exception=False and check the `.rc` from the result, rather
than handling an exception.
6. In addzone/tests_rndc_deadlock.py, refactor the test slightly to
avoid using EnvCmd() entirely to avoid spamming the logs. This test
calls rndc in a loop from multiple threads and such test case is an
exception which doesn't warrant changing the `isctest.run.cmd()`
implementation.
It's a fairly common pattern to use regular expression in our tests.
Instead of using the fairly verbose re.compile(), import that function
as Re() instead to allow for more brevity in the test syntax.
The configloading system script attempts multiple `rndc
{reconfig,reload}` commands without ensuring the system left
exclusive mode; which normally raise an RNDC error as the server is
currently reloading already. This used to work because the request was
enqueued while the server was in exclusive mode, and was processed
after the server `reload_status` was reset to `NAMED_RELOAD_DONE`.
Due to the fact the exclusive mode is not retaken after
`apply_configuration()` by `load_zones()`, the scheduling of
pending tasks is changed and, regularly, the RNDC command sent by the
test is processed before `NAMED_RELOAD_DONE` is set. This is the same
kind of issue the views system tests had, solved by
`4b2dcb3128fbd5af4609a5a73aeeee1f93bde237`
Fix the problem by waiting for a log line matching the end of
the reloading phase.
Adds a log-based test ensuring that when a reconfiguration fails inside
the view configuration, the newly created view are always detached
before the exclusive mode is ended.
In order to have a (minimal) test ensuring we don't move back
`apply_configuration` subroutines which can be done before the exclusive
lock is taken, `APPLY_CONFIGURATION_SUBROUTINE_LOG` macro is added and
used for the few subroutines already extracted from the exclusive mode.
Those expected logs are added in `configloading` system test checks.
Many of our test cases only use a single NamedInstance from the
`servers` fixture. Introduce `nsX` helper fixtures to simplify these
tests and reduce boilterplate code further.
Specifically, the test no longer has to either define its own variable
to extract a single server from the list, or use the longer
servers["nsX"] syntax. While this may seem minor, the amount of times it
is repeated across the tests justifies the change. It also promotes
using more explicit server identification, i.e. `nsX`, rather than
generic `server`. This also improves the clarity of the tests and may be
helpful in traceback during debugging as well.
- Use WatchLog.wait_for_sequence() for the configloading test.
- Omit artifacts check, as it seems quite useless for this test case.
- Join all the tests together. The test case is fairly simple here and
this is the easiest way to ensure the log will be in a predictable
state for all tests. Previously, there was no way to ensure
test_configloading_loading() won't be executed after the other tests,
which would render the check moot. It could also be separated into
its own module, but that seems excessive for a simple test case like
this.
- Use jinja2 template for named.conf and remove setup.sh.
- Remove README and put the relevent comment directly next to the test.
- Remove _sh_ from the test filename to uphold the naming convention.
