hydra-builder: move operational settings from CLI flags to a TOML config file#1797
Open
Ericson2314 wants to merge 1 commit into
Open
hydra-builder: move operational settings from CLI flags to a TOML config file#1797Ericson2314 wants to merge 1 commit into
Ericson2314 wants to merge 1 commit into
Conversation
…fig file The builder took every tuning knob as a `--flag`, so the NixOS and Darwin modules reconstructed a dozen-argument `ExecStart` and each setting existed twice: once as a module option, once as a CLI flag. The queue runner already solved this with a `--config-path` TOML file; the builder now follows the same shape. `config.rs` splits into three concerns: - `Args` (the clap entry point) carries only `--config-path`, which is meta-config locating the file, plus a flattened `Cli`. - `Cli` keeps the connection/security options that don't belong in a world-readable config file: gateway endpoint, mTLS cert paths, auth token. - `AppConfig` (serde, `camelCase`, `deny_unknown_fields`) holds the operational settings, each defaulting to the old clap default, so a missing file behaves exactly as before. `systems` and `supportedFeatures` stay `Option`: absent means "read from `nix show-config`", an explicit `[]` means "advertise none" — the modules type them as `nullOr (listOf ...)` so that distinction survives into the generated TOML. It was unclear how to take advantage of this `None` for `Some([])` distinction before --- certainly the old NixOS module was not doing it. The modules now generate `/etc/hydra/builder.toml` with `pkgs.formats.toml` and pass only the connection/security flags. This also drops the old `--use-substitutes` always-on bug: the modules guarded it with `useSubstitutes != null`, always true for a non-null bool, so the option never actually took effect.
2bb532e to
9aa5b3b
Compare
Member
Author
|
(@mweinelt mentioned to me the issue with inheriting the default vs truly empty lists. The next step would be to make the NixOS module inherit from the Nix settings.) |
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
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.
The builder took every tuning knob as a
--flag, so the NixOS and Darwin modules reconstructed a dozen-argumentExecStartand each setting existed twice: once as a module option, once as a CLI flag. The queue runner already solved this with a--config-pathTOML file; the builder now follows the same shape.config.rssplits into three concerns:Args(the clap entry point) carries only--config-path, which is meta-config locating the file, plus a flattenedCli.Clikeeps the connection/security options that don't belong in a world-readable config file: gateway endpoint, mTLS cert paths, auth token.AppConfig(serde,camelCase,deny_unknown_fields) holds the operational settings, each defaulting to the old clap default, so a missing file behaves exactly as before.systemsandsupportedFeaturesstayOption: absent means "read fromnix show-config", an explicit[]means "advertise none" — the modules type them asnullOr (listOf ...)so that distinction survives into the generated TOML. It was unclear how to take advantage of thisNoneforSome([])distinction before --- certainly the old NixOS module was not doing it.The modules now generate
/etc/hydra/builder.tomlwithpkgs.formats.tomland pass only the connection/security flags.This also drops the old
--use-substitutesalways-on bug: the modules guarded it withuseSubstitutes != null, always true for a non-null bool, so the option never actually took effect.