kea_command: new module to access an ISC KEA server

[mirabilos]

Please be gentle, this is my first Ansible module, but do tell me how to improve it to meet the same standards as other modules ;)

SUMMARY

This module can be used to access the JSON API of a KEA DHCP4, DHCP6, DDNS or other services in a generic way, without having to manually format the JSON, with response error code checking.

It directly accesses the Unix Domain Socket API so it needs to execute on the system the server is running, with superuser privilegues, but without the hassle of wrapping it into HTTPS and password auth (or client certificates).

ISSUE TYPE

• New Module/Plugin Pull Request

COMPONENT NAME

kea_command

ADDITIONAL INFORMATION

The integration test uses a predefined setup for convenience (i.e. so I can rely on a system setup I know and have tested); this in no way limits the usability of the module itself, it merely makes it easier to configure the “server component” of the integration test.

[mirabilos]

ERROR: Found 1 metaclass-boilerplate issue(s) which need to be resolved:
ERROR: plugins/modules/kea_command.py:0:0: missing: metaclass = type (75%)

Huh.

1. why does the sanity test not show this when ran locally?
2. isn’t that just for Python compatibility, but all supported releases require Python3 now anyway, and thus redundant?

[felixfontein]

Thanks for your contribution!

ERROR: Found 1 metaclass-boilerplate issue(s) which need to be resolved:
ERROR: plugins/modules/kea_command.py:0:0: missing: metaclass = type (75%)

Huh.

1. why does the sanity test not show this when ran locally?

2. isn’t that just for Python compatibility, but all supported releases require Python3 now anyway, and thus redundant?

The sanity test was removed from ansible-core 2.17 I think (since 2.17 dropped support for Python 2.7 on the target as well), but this collection still supports ansible-core 2.16, whose sanity tests still require it, and which also still supports Python 2.7 on the target. (We'll remove support for ansible-core 2.16 in the next major release in ~November this year. From then on it's no longer needed.)

[mirabilos]

Felix Fontein dixit:

this collection still supports ansible-core 2.16, whose sanity tests still require it, and which also still supports Python 2.7 on the target.

Ah, okay. I will try to port this thing to Python then, although the service it accesses is recent enough it’s normally available only on distros that ship only py3k. It’s a bit tricky as I seem to have to rely on the CI, as the sanity tests from the documentation pass when I run them locally.

[felixfontein]

It’s a bit tricky as I seem to have to rely on the CI, as the sanity tests from the documentation pass when I run them locally.

You can install ansible-core 2.16 locally and use its ansible-test, but note that it will refuse to run if you don't run it with Python 3.10, 3.11, or 3.12...

In any case, usually running sanity tests with the latest (or one of the latest) ansible-core release is perfectly fine, this only changed once ansible-core 2.17 dropped support for Python 2.7, and is only a problem while we still support ansible-core versions that still support Python 2.7. Which fortunately ends (at least for main and the latest stable branch) in ~November this year.

(And yes, I've been looking forward to this for years :D )

[felixfontein]

Thanks for your contribution!

[mirabilos]

On Sat, 23 Aug 2025, Felix Fontein wrote: Hmm, the image works fine for me without privileged mode (and I would

Does the integration test work for you without it?

[felixfontein]

Hmm, the image works fine for me without privileged mode (and I would
Does the integration test work for you without it?

No, Networking setup, interface fails with Operation not permitted.

I could imagine that it would work fine in some of the test VMs though that are used in CI (various versions of RHEL, Alpine 3.22, Ubuntu 22.04 and 24.04).

[mirabilos]

24.04 is rather old for this (KEA is, while not brand new, only used by people now that ISC DHCP is EOL, and has had several major changes recently), but if you think it worth, I can try making a regression test for it.

Rest ok then?

[mirabilos]

Or, hm, well, if the “Networking setup, interface” step is the one that fails for you, then I don’t think it will even work there, I need an extra dummy network interface it can temporarily play DHCP server on for this.

[russoz]

hi @mirabilos Thanks for your contribution!!!!

I got some comments myself, please take a look. Not everything is mandatory, feel free to disagree :-)

[ansibullbot]

@mirabilos This PR contains @ mentions in at least one commit message. Those mentions can cause cascading notifications through GitHub and need to be removed. Please squash or amend your commits to remove the mentions.

click here for bot help

[felixfontein]

Or, hm, well, if the “Networking setup, interface” step is the one that fails for you, then I don’t think it will even work there, I need an extra dummy network interface it can temporarily play DHCP server on for this.

That fails for me because it's in a container. Ubuntu 24.04 also runs as a VM, there this is not a problem.

There are packages for KEA for Ubuntu 24.04, Alpine 3.22, and RHEL 10, all which are available as VMs in our CI. You should adjust the tests so they at least run with (at least) one of these VMs.

[mirabilos]

There are packages for KEA for Ubuntu 24.04, Alpine 3.22, and RHEL 10, all which are available as VMs in our CI. You should adjust the tests so they at least run with (at least) one of these VMs.

Hm, okay. If it runs as VM, I can likely set up the service.

I can try to get it working under Ubuntu noble. What magic incantations do I need in the tests/ subdirectory to make it select precisely that VM, and nothing else, to run the integration test?

[felixfontein]

Basically add this to aliases:

azp/posix/2
azp/posix/vm
skip/aix
skip/alpine  # TODO: make this work
skip/docker
skip/fedora  # TODO: make this work (not running in CI right now)
skip/freebsd
skip/macos
skip/osx
skip/rhel  # TODO: make this work
skip/ubuntu22.04
needs/root

The TODOs are so that at some point hopefully someone extends the tests to also run on these systems.

[mirabilos]

@felixfontein seems to pass CI now, with the test actually run on 24.04 VM.

Before I’ll push without the debugging extra steps, is there anything else I need to change before this can be merged?

[felixfontein]

Just some adjustments to the current repo state, I think then it's ok to merge.

[mirabilos]

I’ve added a missing quote, more comments, and also a task to show the dhcp6 server log, for symmetry.

[mirabilos]

… and that ruff thing.

[mirabilos]

On Sun, 2 Nov 2025, Felix Fontein wrote: This is not compatible with how the collection adds new test targets.

How? It’s just a warning.

Please remove it.

Removed, as requested.

[felixfontein]

Thanks for removing it.

This is not compatible with how the collection adds new test targets.
How? It’s just a warning.

When we add a new platform to CI, we try running all existing tests against them (which don't already skip the category of platform, for example because they contain skip/docker or skip/freebsd). If everything passes, the platform is just added. If not, specific tests that don't pass are either adjusted (if it's simple enough) or explicitly disabled (if it needs more work) and marked with a TODO. Having to also take care of a warning for a new platform for a specific test is just more work for maintainers when adding new platforms.

[felixfontein]

If nobody objects, I'll merge this later today/tomorrow.

[mirabilos]

On Sun, 2 Nov 2025, Felix Fontein wrote: > How? It’s just a warning. When we add a new platform to CI, we try running all existing tests against them (which don't already skip the category of platform, for example because they contain `skip/docker` or `skip/freebsd`). If everything passes, the platform is just added. If not, specific tests that don't pass are either adjusted (if it's simple enough) or explicitly disabled (if it needs more work) and marked with a TODO. Having to also take care of a warning for a new platform for a specific test is just more work for maintainers when adding new platforms.

Ah! Okay, makes sense, thanks for the feedback.

[russoz]

Hi @mirabilos

Couple of comments more.

[russoz]

This module is to be added to c.g. 12.0.0, that no longer supports Python 2 (finally! 😄), so this line is now redundant.

[russoz]

If the statement in line 165 below fails to send the command, then the module returns changed=true but nothing happened.

[mirabilos]

On Sun, 2 Nov 2025, Alexei Znamensky wrote: +skip/python2 This module is to be added to c.g. 12.0.0, that no longer supports Python 2 (finally! 😄), so this line is now redundant.

Phew. OK, will remove it.

> + # better safe in case anything fails… + r["changed"] = True If the statement in line 165 below fails to send the command, then the module returns `changed=true` but nothing happened.

That’s correct, I inserted this so that, for all cases where we cannot *guarantee* that nothing changed, it reports changed (plus an error), so that is from the possibly first byte sent.

[russoz]

• better safe in case anything fails… + r["changed"] = True If the statement in line 165 below fails to send the command, then the module returns changed=true but nothing happened.

That’s correct, I inserted this so that, for all cases where we cannot guarantee that nothing changed, it reports changed (plus an error), so that is from the possibly first byte sent.

Users (writing Ansible playbooks) should, of course, handle module failures (or they will pop up in their faces anyway), in which case changed=true might indicate the command was executed, but that is not necessarily the case, and it can cause confusion.

I would say it is probably better not to return changed status at all in this case, since it is unknown - and that should lead to a note in the documentation stating to users that, in case of failure, the state of the service managed by kea is undetermined and the user is responsible for checking it again before making new moves.

Hope that makes sense

[mirabilos]

On Sun, 2 Nov 2025, Alexei Znamensky wrote: in case of failure, the state of the service managed by kea is undetermined and the user is responsible for checking it again before making new moves.

isn’t that already implicit anyway? The setting means changed, not succeeded to change to the desired state, after all ;)

[russoz]

In my gut I still feel this is kinda "wrong" but I cannot find a good objective argument to support that claim. Therefore, I rest my case.

[felixfontein]

Most modules do not return changed at all when failing, even if they already did change something. But I guess there's nothing wrong if modules try to set it correctly (or as correct as possible). Generally failure handling with Ansible is quite non-trivial since there are many different reasons for which a module can fail, and usually there's no good way (except looking for magic string in error messages) to distinguish between different kind of errors.

In any case, I guess it doesn't hurt to try to return changed correctly also in case of errors, even though it's rather unusual. At least one can use this to distinguish an involuntary module failure (like a crash) from an intended module failure (call to module.fail_json)...

[russoz]

One would argue that using one name to mean something other than the wordvs semantics is a good way to get oneself into trouble. Been there, done that.

But anyways, as I wrote before I lack a compelling argument here, so let's go :-)

[mirabilos]

Thanks. I did point this out in the documentation, for what it’s worth:

• Between sending the command and parsing the result status, RV(ignore:changed) will register as V(true) if an error occurs, to err on the safe side.

[felixfontein]

@mirabilos thanks for your contribution!
@russoz thanks for reviewing!