Fix tests for the "none" backend#350
Merged
edolstra merged 6 commits intoNixOS:masterfrom Oct 26, 2015
Merged
Conversation
Because the coordinator machine needs to build the test machines, we need to make sure that every package that can't be built without networking is already available. So far, the test has used environment.systemPackages to add those references. However, using pkgs.stdenv won't export a single reference to the store path of stdenv, because it doesn't contain any paths that are in the default environment.pathsToLink. This usually shouldn't be a problem, because stdenv had a nix-support/propagated-user-env-packages file, which made sure that every package listed there was recursively added to the system path as well. Unfortunately, since NixOS/nixpkgs@328f7a6, stdenv doesn't contain propagatedUserEnvPkgs anymore, so these dependencies aren't available to the coordinator machine anymore. Either way, if any of the packages listed in environment.systemPackages do not contain paths that are not in environment.pathsToLink, we don't get any reference to the path and that in turn might lead to unexpected behaviour. One example I can think of is if /lib is removed from pathsToLink, which is already marked as "FIXME: remove" since NixOS/nixpkgs@e636e0a. Signed-off-by: aszlig <aszlig@redmoonstudios.org>
aszlig
added a commit
to aszlig/nixpkgs
that referenced
this pull request
Oct 22, 2015
Clearly it would be the best if we'd directly generate mount units instead of converting /etc/fstab. But in order to do that we need to test it throughly so this approach is for the next stable release. This fix however is intended for inclusion into release-14.12 and release-15.09. Using a simple regular expression unfortunately isn't sufficient for proper mount unit name quoting/escaping and there is a utility in systemd called systemd-escape which does nothing less than that. Of course, using an external program to escape the unit name is way more expensive and causes us to fork for each mount point. But given that we already do quite a lot of forks just for unit starting and stopping, I think it doesn't matter that much. Well, except if you have a whole bunch of mount points. However, if the latter is the case and you have thousands of mount points, you probably have stumbled over this already if your mount point contains a dash. As for my motivation to fix this: I've stumbled on this while trying to fix the "none" backend test for NixOps (see NixOS/nixops#350), where the target machines use /nix/.ro-store and /nix/.rw-store as mount points. The implementation we had so far did improperly escape it so those mount points got the following unit files: * nix-.ro-store.mount * nix-.rw-store.mount The correct names for these units are however: * nix-.ro\x2dstore.mount * nix-.rw\x2dstore.mount So using systemd-escape now properly generates these names. Signed-off-by: aszlig <aszlig@redmoonstudios.org>
aszlig
added a commit
to aszlig/nixpkgs
that referenced
this pull request
Oct 22, 2015
Also related to NixOS/nixops#350, because while switching to the new configuration, depending on /nix/store also propagates to the mount points for /nix/.ro-store and /nix/.rw-store and we don't get an error while trying to unmount them (because nix-daemon needs to be stopped for unmounting these paths). While Nix does have the option to set a different store path, I've found only hardcoded references in nix-daemon.nix, so I'm using a hardcoded reference here as well, because after all customizing the store path will probably only make sense on non-NixOS systems. Signed-off-by: aszlig <aszlig@redmoonstudios.org>
Member
Author
|
Okay, opened NixOS/nixpkgs#10546 for the |
The default load balancing method has been split off into its own module since Apache 2.4, so we need to add that in the config to ensure proper start up of the web server. Here are the details about the module: https://httpd.apache.org/docs/2.4/mod/mod_lbmethod_byrequests.html Signed-off-by: aszlig <aszlig@redmoonstudios.org>
We have virtualisation.writableStore active for the targets that are not yet deployed by NixOps but not within the NixOps network specification. This has worked regardless in the past, because using a writable store was implemented solely using boot.initrd.postMountCommands, so switching to a new configuration didn't change anything and left the store untouched. But after NixOS/nixpkgs@e68b0c7, the mounts for the writable store are now using the fileSystems configuration attribute, which causes the mount points to be changed on switch. Another problem is that the mount units weren't escaped properly. But that isn't the problem here anymore (because we don't change the mounts between activation) and has been (or is going to be) dealt with in NixOS/nixpkgs#10546. Signed-off-by: aszlig <aszlig@redmoonstudios.org>
Just an asthetics change, so we have defined it once rather than twice: Once for each logical specification. So we spare one line, yay! Signed-off-by: aszlig <aszlig@redmoonstudios.org>
This has never been done in this test but it's actually done in <nixpkgs/nixos/lib/build-vms.nix> using assignIPAddresses. The reason this hasn't been failing was that the IP address reconfiguration wasn't done properly back then when this test was happily succeeding. The implementation in <nixpkgs> actually didn't stop the interface at all, which is why this passed unnoticed. Since then, we also have proper networking tests in <nixpkgs>, so we shouldn't assume a broken implementation in <nixpkgs>. However, the implementation is still a bit ugly, because we're now statically setting the IP address instead of reusing assignIPAddresses. We can't reuse the latter, because it requires us to insert a dummy node (coordinator), which we can't remove later on because the node name is referenced in the configuration via getAttrs on the node attribute. Signed-off-by: aszlig <aszlig@redmoonstudios.org>
Member
Author
|
Okay, the test should now be fixed even without the need to merge in NixOS/nixpkgs#10546, tested against current and unmodified |
aszlig
added a commit
to aszlig/nixops
that referenced
this pull request
Oct 23, 2015
Fix tests for the "none" backend
aszlig
added a commit
to openlab-aux/vuizvui
that referenced
this pull request
Oct 23, 2015
Instead of merging all those PRs via the patches attribute, I've now created a branch that has all those PRs merged, which are: * NixOS/nixops#201: Use dedicated SSH keypair for "none" backend * NixOS/nixops#348: Fixup and refactor Hetzner backend tests * NixOS/nixops#349: hetzner: Don't create /root/.ssh/authorized_keys * NixOS/nixops#350: Fix tests for the "none" backend So our version of NixOps now should now correctly cope with users.mutableUsers set to false. Signed-off-by: aszlig <aszlig@redmoonstudios.org>
tests/none-backend.nix
Outdated
Member
There was a problem hiding this comment.
You can use system.extraDependencies for this.
Member
Author
There was a problem hiding this comment.
Ah, good to know, going to fix this for #348 as well.
Member
Author
There was a problem hiding this comment.
Be sure to also cherry-pick 370e928, which does the same for the Hetzner tests.
Didn't notice that there is an option just for that, thanks to @edolstra for the heads-up in: NixOS#350 (comment) Signed-off-by: aszlig <aszlig@redmoonstudios.org>
aszlig
added a commit
to aszlig/nixops
that referenced
this pull request
Oct 26, 2015
Didn't notice that there is an option just for that, thanks to @edolstra for the heads-up in: NixOS#350 (comment) Signed-off-by: aszlig <aszlig@redmoonstudios.org>
edolstra
added a commit
that referenced
this pull request
Oct 26, 2015
Fix tests for the "none" backend
edolstra
pushed a commit
that referenced
this pull request
Oct 26, 2015
Didn't notice that there is an option just for that, thanks to @edolstra for the heads-up in: #350 (comment) Signed-off-by: aszlig <aszlig@redmoonstudios.org>
wkennington
pushed a commit
to triton/triton
that referenced
this pull request
Jul 5, 2016
Clearly it would be the best if we'd directly generate mount units instead of converting /etc/fstab. But in order to do that we need to test it throughly so this approach is for the next stable release. This fix however is intended for inclusion into release-14.12 and release-15.09. Using a simple regular expression unfortunately isn't sufficient for proper mount unit name quoting/escaping and there is a utility in systemd called systemd-escape which does nothing less than that. Of course, using an external program to escape the unit name is way more expensive and causes us to fork for each mount point. But given that we already do quite a lot of forks just for unit starting and stopping, I think it doesn't matter that much. Well, except if you have a whole bunch of mount points. However, if the latter is the case and you have thousands of mount points, you probably have stumbled over this already if your mount point contains a dash. As for my motivation to fix this: I've stumbled on this while trying to fix the "none" backend test for NixOps (see NixOS/nixops#350), where the target machines use /nix/.ro-store and /nix/.rw-store as mount points. The implementation we had so far did improperly escape it so those mount points got the following unit files: * nix-.ro-store.mount * nix-.rw-store.mount The correct names for these units are however: * nix-.ro\x2dstore.mount * nix-.rw\x2dstore.mount So using systemd-escape now properly generates these names. Signed-off-by: aszlig <aszlig@redmoonstudios.org>
johnalotoski
pushed a commit
to input-output-hk/nixops-hetzner
that referenced
this pull request
Jul 13, 2019
Didn't notice that there is an option just for that, thanks to @edolstra for the heads-up in: NixOS/nixops#350 (comment) Signed-off-by: aszlig <aszlig@redmoonstudios.org>
johnalotoski
pushed a commit
to input-output-hk/nixops-hetzner
that referenced
this pull request
Aug 7, 2019
Didn't notice that there is an option just for that, thanks to @edolstra for the heads-up in: NixOS/nixops#350 (comment) Signed-off-by: aszlig <aszlig@redmoonstudios.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While this solves the problem with the unavailable store paths, the activation of the target systems still fail with something like:
We need to look for the origin of this failure and fix it either here or in
<nixpkgs>.