Introduce test for deploying all services via FediPanel #361
No reviewers
Labels
No labels
0 points
0.5 points
1 point
13 points
2 points
21 points
3 points
34 points
5 points
55 points
8 points
api service
blocked
component: fediversity panel
component: nixops4
documentation
estimation high: >3d
estimation low: <2h
estimation mid: <8h
infinite points
productisation
project-management
question
role: application developer
role: application operator
role: hosting provider
role: maintainer
security
technical debt
testing
type unclear
type: bug
type: deliverable
type: key result
type: objective
type: task
type: user story
user experience
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: fediversity/fediversity#361
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "Niols/Fediversity:check-deployment-panel"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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! 🎉
CI failure of
check-panelis to be expected considering my hacks to the panel.CI failures of
check-deployment-cliandcheck-deployment-panelare more concerning. Both these tests pass locally for me. In CI, they fail with[Errno 32] Broken pipeand[Errno 9] Bad file descriptorwhich does not sound good. We might be hitting limits on the runner?002b646214to5ff6269b80The 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.
5ff6269b80to63f8bb6bda63f8bb6bdato051199f19a051199f19ato7d1d5c3002nix develop#3757d1d5c3002toda4df14b13After Fediversity/Fediversity#375 and Fediversity/Fediversity#376, this PR does not contain any non-mergeable hacks, imo. We're getting pretty close to merging!
All hacks resolved it seems?
da4df14b13tod3c19b344dWIP: Introduce test for deploying all services via FediPanelto Introduce test for deploying all services via FediPanelAll green!
@ -0,0 +1 @@## This is a placeholder file. It will be overwritten by the test.why do we need the placeholder?
in retrospect found the other file does make mention of the placeholder
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_keysin 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.#385 has been merged, this PR rebased on top, and
deployer.pubhas been removed in 74ae7e10d06519cee55154e4f380ab6b06630b74.@ -0,0 +31,4 @@../common/nixosTest.nix./nixosTest.nix];_module.args.inputs = inputs;why this over
module.specialArgs?🤷 no preference!
@ -0,0 +40,4 @@flakeIgnore = ["E302""E303" # too many blank lines"E305"maybe clarify the other two as well 🙈
Done in 7181dc1808d02be5a26017d7ea8127dcb572c5ff!
@ -0,0 +150,4 @@production = true;domain = "deployer";secrets = {SECRET_KEY = dummyFile; # REVIEW: what is this secret for?good point, i can't find it either - i opened a PR to remove it at #381
Alright! Should I remove the comment, then? Or wait for #381 to be merged and rebase this PR?
following Fediversity/Fediversity#381 (comment), you can have a comment in
settings.pyinstead if you like?Moved comment in e3b3d2e02038cb35416b4590cbdaa04f2d2a04a0.
@ -0,0 +169,4 @@peertubepeertube.inputDerivationgixygixy.inputDerivationdo 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
gixy is an nginx configuration tester - it must be needed to check the configuration coming from all the
services.nginx.*.maybe add a comment on this? 😶
Done in 87fde40306f1fa7b8485e57b63d43a7ec946729b.
@ -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")maybe make them in one go with
mkdir -p?mkdir -phas a much loser semantics and will succeed silently in many more cases (eg. if/rundoesn't exist or/run/fedipaneldoes exist). But if you think it improves readability I'm happy to go for it.ah, i'm satisfied with that reasoning
@ -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.filed as #383
@ -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. — Niolsfiled as #384
@ -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. — Niolsfiled as #382
attribute 'lib' missing#328deployer.pub#385e3b3d2e020to74ae7e10d0@ -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.
Sad.
Nice, so as a final touch it'd be cool to convert the
REVIEWcomments toTODOand 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!Done! Let's wait for green CI and squash this beast.
beb80bcee0tof917b62de9(thunderclap)