Introduce test for deploying all services via FediPanel #361

Merged
fricklerhandwerk merged 6 commits from Niols/Fediversity:check-deployment-panel into main 2025-06-18 12:37:49 +02:00
Owner

Closes #277

Same as #329 but where we run the FediPanel and interact with it via a browser
instead of running NixOps4 directly. Achieving this test led me to make quite a
few changes to the rest of the codebase. There are three main types of changes:

  • Clean changes to the rest of the codebase. Those are all in independent commit
    referring to a small independent pull request. They can be debatted there and
    merged. They should be merged before this PR.

  • Dirty changes to the rest of the codebase. These typically change a hardcoded
    value into another one, that type of things. They are also each in a different
    commit. We should discuss the best way to actually provide what I need, and
    implement it cleanly in another PR. Such PRs should be merged before this PR.

  • The introduction of the test. This is the main and last commit on this branch,
    and really the only one that should remain once all the other changes have
    been integrated to main. It still contains a few things that are slightly
    hackish, but at least they are contained to the test. They could be fixed
    before we merged, but I would say we should merge and only later come back to
    clean things up.

Finally, here we are, after weeks of work on this! 🎉

Closes #277 Same as #329 but where we run the FediPanel and interact with it via a browser instead of running NixOps4 directly. Achieving this test led me to make quite a few changes to the rest of the codebase. There are three main types of changes: - Clean changes to the rest of the codebase. Those are all in independent commit referring to a small independent pull request. They can be debatted there and merged. They should be merged before this PR. - Dirty changes to the rest of the codebase. These typically change a hardcoded value into another one, that type of things. They are also each in a different commit. We should discuss the best way to actually provide what I need, and implement it cleanly in another PR. Such PRs should be merged before this PR. - The introduction of the test. This is the main and last commit on this branch, and really the only one that should remain once all the other changes have been integrated to main. It still contains a few things that are slightly hackish, but at least they are contained to the test. They could be fixed before we merged, but I would say we should merge and only later come back to clean things up. Finally, here we are, after weeks of work on this! :tada:
Author
Owner

CI failure of check-panel is to be expected considering my hacks to the panel.

CI failures of check-deployment-cli and check-deployment-panel are more concerning. Both these tests pass locally for me. In CI, they fail with [Errno 32] Broken pipe and [Errno 9] Bad file descriptor which does not sound good. We might be hitting limits on the runner?

CI failure of `check-panel` is to be expected considering my hacks to the panel. CI failures of `check-deployment-cli` and `check-deployment-panel` are more concerning. Both these tests pass locally for me. In CI, they fail with `[Errno 32] Broken pipe` and `[Errno 9] Bad file descriptor` which does not sound good. We might be hitting limits on the runner?
Niols force-pushed check-deployment-panel from 002b646214 to 5ff6269b80 2025-06-04 19:39:25 +02:00 Compare
Author
Owner

The fact that they now succeed suggests that this was in fact the usual Fediversity/Fediversity#362. Also, the new test takes nearly an hour, where it hits a timeout, so depending on the load on the runner, it can fail for this reason. All good, now, though.

The fact that they now succeed suggests that this was in fact the usual https://git.fediversity.eu/Fediversity/Fediversity/issues/362. Also, the new test takes nearly an hour, where it hits a timeout, so depending on the load on the runner, it can fail for this reason. All good, now, though.
Niols force-pushed check-deployment-panel from 5ff6269b80 to 63f8bb6bda 2025-06-11 10:01:06 +02:00 Compare
Niols force-pushed check-deployment-panel from 63f8bb6bda to 051199f19a 2025-06-12 11:36:22 +02:00 Compare
Niols force-pushed check-deployment-panel from 051199f19a to 7d1d5c3002 2025-06-12 13:08:16 +02:00 Compare
Niols force-pushed check-deployment-panel from 7d1d5c3002 to da4df14b13 2025-06-13 22:23:09 +02:00 Compare
Author
Owner

After Fediversity/Fediversity#375 and Fediversity/Fediversity#376, this PR does not contain any non-mergeable hacks, imo. We're getting pretty close to merging!

After https://git.fediversity.eu/Fediversity/Fediversity/pulls/375 and https://git.fediversity.eu/Fediversity/Fediversity/pulls/376, this PR does not contain any non-mergeable hacks, imo. We're getting pretty close to merging!

All hacks resolved it seems?

All hacks resolved it seems?
Niols force-pushed check-deployment-panel from da4df14b13 to d3c19b344d 2025-06-16 10:49:11 +02:00 Compare
Niols changed title from WIP: Introduce test for deploying all services via FediPanel to Introduce test for deploying all services via FediPanel 2025-06-16 11:10:57 +02:00
Author
Owner

All green!

All green!
@ -0,0 +1 @@
## This is a placeholder file. It will be overwritten by the test.
Owner

why do we need the placeholder?

why do we need the placeholder?
Owner

in retrospect found the other file does make mention of the placeholder

in retrospect found the other file does make mention of the placeholder
Author
Owner

While tracking where we use it to show you, I found that... we weren't using it at all! This is cool but also a bit of a bug, because that means that it only works because we manually put the public key of the deployer in /root/.ssh/authorized_keys in the targets, but not thanks to the configuration that we push. Still, for the sake of simplicity, maybe it is nicer to get rid of those deployers? We can move the conversation to Fediversity/Fediversity#385.

While tracking where we use it to show you, I found that... we weren't using it at all! This is cool but also a bit of a bug, because that means that it only works because we manually put the public key of the deployer in `/root/.ssh/authorized_keys` in the targets, but not thanks to the configuration that we push. Still, for the sake of simplicity, maybe it is nicer to get rid of those deployers? We can move the conversation to https://git.fediversity.eu/Fediversity/Fediversity/pulls/385.
Author
Owner

#385 has been merged, this PR rebased on top, and deployer.pub has been removed in 74ae7e10d06519cee55154e4f380ab6b06630b74.

#385 has been merged, this PR rebased on top, and `deployer.pub` has been removed in 74ae7e10d06519cee55154e4f380ab6b06630b74.
kiara marked this conversation as resolved
kiara approved these changes 2025-06-16 14:19:28 +02:00
@ -0,0 +31,4 @@
../common/nixosTest.nix
./nixosTest.nix
];
_module.args.inputs = inputs;
Owner

why this over module.specialArgs?

why this over `module.specialArgs`?
Author
Owner

🤷 no preference!

🤷 no preference!
kiara marked this conversation as resolved
@ -0,0 +40,4 @@
flakeIgnore = [
"E302"
"E303" # too many blank lines
"E305"
Owner

maybe clarify the other two as well 🙈

maybe clarify the other two as well 🙈
Author
Owner

Done in 7181dc1808d02be5a26017d7ea8127dcb572c5ff!

Done in 7181dc1808d02be5a26017d7ea8127dcb572c5ff!
kiara marked this conversation as resolved
@ -0,0 +150,4 @@
production = true;
domain = "deployer";
secrets = {
SECRET_KEY = dummyFile; # REVIEW: what is this secret for?
Owner

good point, i can't find it either - i opened a PR to remove it at #381

good point, i can't find it either - i opened a PR to remove it at #381
Author
Owner

Alright! Should I remove the comment, then? Or wait for #381 to be merged and rebase this PR?

Alright! Should I remove the comment, then? Or wait for #381 to be merged and rebase this PR?
Owner

following Fediversity/Fediversity#381 (comment), you can have a comment in settings.py instead if you like?

following https://git.fediversity.eu/Fediversity/Fediversity/pulls/381#issuecomment-7893, you can have a comment in `settings.py` instead if you like?
Author
Owner

Moved comment in e3b3d2e02038cb35416b4590cbdaa04f2d2a04a0.

Moved comment in e3b3d2e02038cb35416b4590cbdaa04f2d2a04a0.
kiara marked this conversation as resolved
@ -0,0 +169,4 @@
peertube
peertube.inputDerivation
gixy
gixy.inputDerivation
Owner

do we know why we need gixy anyway? maybe if we can clarify that we could at least know when we may or may not need it in the event of future refactors

do we know why we need gixy anyway? maybe if we can clarify that we could at least know when we may or may not need it in the event of future refactors
Author
Owner

gixy is an nginx configuration tester - it must be needed to check the configuration coming from all the services.nginx.*.

gixy is an nginx configuration tester - it must be needed to check the configuration coming from all the `services.nginx.*`.
Owner

maybe add a comment on this? 😶

maybe add a comment on this? 😶
Author
Owner

Done in 87fde40306f1fa7b8485e57b63d43a7ec946729b.

Done in 87fde40306f1fa7b8485e57b63d43a7ec946729b.
kiara marked this conversation as resolved
@ -0,0 +252,4 @@
## someone wanted to change the code of the flake.
##
with subtest("Give the panel access to the flake"):
deployer.succeed("mkdir /run/fedipanel /run/fedipanel/flake >&2")
Owner

maybe make them in one go with mkdir -p?

maybe make them in one go with `mkdir -p`?
Author
Owner

mkdir -p has a much loser semantics and will succeed silently in many more cases (eg. if /run doesn't exist or /run/fedipanel does exist). But if you think it improves readability I'm happy to go for it.

`mkdir -p` has a much loser semantics and will succeed silently in many more cases (eg. if `/run` doesn't exist or `/run/fedipanel` does exist). But if you think it improves readability I'm happy to go for it.
Owner

ah, i'm satisfied with that reasoning

ah, i'm satisfied with that reasoning
kiara marked this conversation as resolved
@ -0,0 +259,4 @@
## REVIEW: I want a programmatic way to provide an SSH key to the panel (and
## therefore NixOps4). This should happen either in the Python code, but
## maybe it is fair that that one picks up on the user's key? It could
## also be in the Nix packaging.
Owner

filed as #383

filed as #383
kiara marked this conversation as resolved
@ -0,0 +276,4 @@
## generate _usable_ certificates in NixOS tests. I suggest we merge this,
## and track the task to set it up in a cleaner way. I would tackle this in
## a subsequent PR, and hopefully even contribute this BetterWay(tm) to
## nixpkgs. — Niols
Owner

filed as #384

filed as #384
kiara marked this conversation as resolved
@ -0,0 +293,4 @@
""")
## REVIEW: I would hope for a more declarative way to add users. This should
## be handled by the Nix packaging of the FediPanel. — Niols
Owner

filed as #382

filed as #382
kiara marked this conversation as resolved
Niols force-pushed check-deployment-panel from e3b3d2e020 to 74ae7e10d0 2025-06-17 16:39:26 +02:00 Compare
@ -42,6 +42,7 @@ def get_secret(name: str, encoding: str = "utf-8") -> str:
return secret
# SECURITY WARNING: keep the secret key used in production secret!
# REVIEW: This seems to be used nowhere; can we get rid of it?

This is a requirement by Django, it needs to be there.

This is a requirement by Django, it needs to be there.
Author
Owner

Sad.

Sad.
fricklerhandwerk left a comment
Owner

Nice, so as a final touch it'd be cool to convert the REVIEW comments to TODO and leave them there to rot until they're annyoing enough to actually fix. Other than that it won't get any better and we can continue developing the use cases. Big yay!

Nice, so as a final touch it'd be cool to convert the `REVIEW` comments to `TODO` and leave them there to rot until they're annyoing enough to actually fix. Other than that it won't get any better and we can continue developing the use cases. Big yay!
Author
Owner

Done! Let's wait for green CI and squash this beast.

Done! Let's wait for green CI and squash this beast.
Niols force-pushed check-deployment-panel from beb80bcee0 to f917b62de9 2025-06-17 17:31:55 +02:00 Compare
fricklerhandwerk deleted branch check-deployment-panel 2025-06-18 12:37:50 +02:00

(thunderclap)

(thunderclap)
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: fediversity/fediversity#361
No description provided.