Register nvws as builder for work profiles#172
Conversation
There was a problem hiding this comment.
Pull request overview
Registers nvws as a remote builder for “work” hosts by splitting builder configuration into personal vs. work builder mixins and wiring them into the common module import graph.
Changes:
- Add
common/_mixins/work-buildersto configure SSH host metadata and registernvwsinnix.buildMachinesfor work profiles. - Add
common/_mixins/personal-buildersto configure SSH host metadata and register personal builders/hosts innix.buildMachinesfor non-work profiles that can use builders. - Update
common/default.niximport selection logic and update README machine description fornvws.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| common/default.nix | Adds canUseWorkBuilders gating and switches builder imports to personal-builders (personal) and work-builders (work). |
| common/_mixins/work-builders/default.nix | New mixin: adds SSH config for nvws and registers it as an ssh-ng build machine for work hosts. |
| common/_mixins/personal-builders/default.nix | New mixin: adds SSH config for personal hosts/builders and registers multiple ssh-ng build machines for non-work hosts. |
| README.md | Documents nvws as also serving as a remote builder for work machines. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
In work env, all dns queries are funneled through corp dns server.
Some policy seems to block nixpkgs ssh binary from accessing local resources via .local sigh.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -29,7 +30,10 @@ in | |||
| ] | |||
| ++ lib.optionals canUseBuilders [ | |||
| ./_mixins/community-builders | |||
| ./_mixins/remote-builders | |||
| ./_mixins/personal-builders | |||
| ] | |||
| ++ lib.optionals canUseWorkBuilders [ | |||
| ./_mixins/work-builders | |||
| ]; | |||
There was a problem hiding this comment.
canUseWorkBuilders already excludes hostname == "nvws", and _mixins/work-builders also guards nix.buildMachines with (hostname != "nvws"). This duplicates the same policy in two places and risks divergence later. Prefer to keep the exclusion in only one place (either gate the import or gate the nix.buildMachines entry).
| nix.buildMachines = lib.optional (hostname != "nvws") { | ||
| hostName = "nvws.local"; | ||
| system = "x86_64-linux"; | ||
| protocol = "ssh-ng"; | ||
| maxJobs = 4; | ||
| speedFactor = 100; | ||
| supportedFeatures = [ | ||
| "nixos-test" | ||
| "benchmark" | ||
| "big-parallel" | ||
| "kvm" | ||
| ]; | ||
| }; |
There was a problem hiding this comment.
nix.buildMachines is wrapped in lib.optional (hostname != "nvws"), but this module is already only imported when hostname != "nvws" (via canUseWorkBuilders in common/default.nix). Consider removing this extra guard to avoid having the same condition in two places.
| nix.buildMachines = lib.optional (hostname != "nvws") { | |
| hostName = "nvws.local"; | |
| system = "x86_64-linux"; | |
| protocol = "ssh-ng"; | |
| maxJobs = 4; | |
| speedFactor = 100; | |
| supportedFeatures = [ | |
| "nixos-test" | |
| "benchmark" | |
| "big-parallel" | |
| "kvm" | |
| ]; | |
| }; | |
| nix.buildMachines = [ | |
| { | |
| hostName = "nvws.local"; | |
| system = "x86_64-linux"; | |
| protocol = "ssh-ng"; | |
| maxJobs = 4; | |
| speedFactor = 100; | |
| supportedFeatures = [ | |
| "nixos-test" | |
| "benchmark" | |
| "big-parallel" | |
| "kvm" | |
| ]; | |
| } | |
| ]; |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | `pi5` | NixOS (Raspberry Pi) | DHCP and network services for the lab. | [nixos/pi5/default.nix](nixos/pi5/default.nix) | [common](common), [nixos](nixos) | | ||
| | `beast` | NixOS (x86_64-linux) | NAS storage + Jellyfin/Jellarr server. | [nixos/beast/default.nix](nixos/beast/default.nix) | [common](common), [nixos](nixos) | | ||
| | `nvws` | Proxmox host | Work Proxmox node configuration. Single node. | [nixos/nvws/default.nix](nixos/nvws/default.nix) | [common](common), [nixos](nixos) | | ||
| | `nvws` | Proxmox host | Work Proxmox node configuration. Single node. Also serves as a remote x86_64-linux builder for work machines. | [nixos/nvws/default.nix](nixos/nvws/default.nix) | [common](common), [nixos](nixos) | |
There was a problem hiding this comment.
The README description states nvws serves as "a remote x86_64-linux builder", but the work-builders configuration in common/_mixins/work-builders/default.nix shows it supports both "x86_64-linux" and "aarch64-linux" systems. The description should be updated to accurately reflect that it supports both architectures, for example: "Also serves as a remote x86_64-linux and aarch64-linux builder for work machines."
| | `nvws` | Proxmox host | Work Proxmox node configuration. Single node. Also serves as a remote x86_64-linux builder for work machines. | [nixos/nvws/default.nix](nixos/nvws/default.nix) | [common](common), [nixos](nixos) | | |
| | `nvws` | Proxmox host | Work Proxmox node configuration. Single node. Also serves as a remote x86_64-linux and aarch64-linux builder for work machines. | [nixos/nvws/default.nix](nixos/nvws/default.nix) | [common](common), [nixos](nixos) | |
Emulation won't serve my needs - being able to build aarch64 images. Some syscalls are not properly emulated. :( This reverts commit 7a31e79.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| lib, | ||
| config, | ||
| username, | ||
| hostname, | ||
| ... | ||
| }: |
There was a problem hiding this comment.
The work-builders module is missing the pkgs import in the function arguments and does not include openssh_gssapi in environment.systemPackages. The personal-builders module includes both (lines 4 and 56), and the community-builders module also includes openssh_gssapi. This package is likely needed for SSH connections to work properly with the builder. Consider adding pkgs to the function arguments and including environment.systemPackages = [ pkgs.openssh_gssapi ] to match the pattern in other builder configurations.
No description provided.