Page MenuHomeVyOS Platform

Make config scripts more testable
Closed, ResolvedPublic

Description

(created for https://github.com/vyos/vyos-1x/pull/254)

This complaint comes from me not having a sacrificial VyOS setup for testing DHCP.

Seeing how configuring DHCP in VyOS is a matter of turning a text file into some text files, I was rather surprised that the scripts can't run in a standalone fashion. I managed to actually achieve this in the corresponding pull request.

Fixing this wider opens up the possibility to do some integration testing for individual config scripts, e.g. in a CI system.

Details

Difficulty level
Unknown (require assessment)
Version
-
Why the issue appeared?
Will be filled on close
Is it a breaking change?
Perfectly compatible

Event Timeline

rhn created this object in space S1 VyOS Public.

I managed to make the DHCP configuration testable, although the code has some hardcoded paths which I didn't want to mess with too much, so my addition may be suboptimal.

The main benefit is that the (janky, experimental) standalone test in https://github.com/vyos/vyos-1x/commit/77f1f095f4fbb9164751c6678e213bab2ab17cac works and is able to catch errors without having to roll the full VyOS environment.

The config file supplied with my comment shows how a bug could be caught: the generated template has a bad NS server entry. I'm not sure if this is an actual bug, because there is no good-known config for the git version of VyOS. Anyway, with those changes it's at least *possible* to start thinking about easily testable configs.

Hi @rhn,

that feels like you wan't to achieve the same goal as https://github.com/vyos/vyos-1x/blob/current/smoketest/scripts/cli/test_service_dhcp-server.py
I think the easiest way would be to just extend the smoketest with your tests that you find missing.

That python script can be executed standalone on your running installation - and is triggered on each CI run.

As far as I can tell (from README), smoke tests are meant more to be integration than unit tests:

Runtime (Smoketests)

Runtime tests are executed by the CI system on a running VyOS instance inside
QEMU.

That's somewhat the same thing, but it requires a running installation - the entire reason I decided to create this patch is that I did not have a running installation, and I could not be bothered creating one.

The difference is that without a running installation, the code-test-fix loop is much shorter, as my test requires only Python and the dhcpd program. I have some complaints in the pipeline, and I'm almost sure that I'm not going to try fixing them if I need to understand how to set up an image from git sources first.

That is a noble idea. There are some build time tests available here: https://github.com/vyos/vyos-1x/tree/current/src/tests - maybe this helps

Thanks, that makes sense. Do you think the current outstanding change could be merged in? It's not resulting in any incompatibilities (that I'm aware of), and while it doesn't fix the problem entirely on its own (doesn't include infrastructure work), it improves the code under test.

As for infrastructure, I can see that the tests you linked are meant to run on a VyOS system, but are much closer to being cross-platform. I imagine a couple of tasks that could open proper unit testing. In no particular order:

  • adjust the linked tests so that they also run from a source checkout
  • figure out how to drop the dependency on dynamic libraries, or build them specifically for tests
  • implement the test from my checkout in a clean way, similar to the linked tests (or as part of them)
  • write a readme for unit testing (maybe even before it's clean of hacks)

How does that sound?

Well the commit you outlined alters file ins src/conf_mode - that directory is reserved for CLI configuration scripts.

It could act as a good start but would require more amount of generalisation as we would not only want to use it for DHCp but all kind of services. Maybe it will result in a combined effort of migrating your idea and smoketests.

Unfortunately I am not into deep with all that configuration backend stuff.

more amount of generalisation as we would not only want to use it for DHCp but all kind of services

Absolutely. I started with dhcp because I don't want to run guns ablaze into a codebase that someone else structured and might be happy with.

The feedback I'm looking for right now is whether this start is good (so mergeable now), needs adjustment (so mergeable when I fix what?), or not mergeable until completed. From experience, the larger the rework is, the way greater the chance it never gets finished (due to upstream conflicts, amount of code to review at once, lack of testing, and motivation). I hope to rework things in tiny pieces, and would like to know what the status of this piece is from the maintainers' POV.

Bump - is anyone interested in reviewing this or at least discussing direction? It's difficult to contribute to stalled projects.

dmbaturin claimed this task.
dmbaturin added a subscriber: dmbaturin.

With the current smoke test infrastructure, I'm inclined to call the original task resolved.