internal/libhive: support simulator build configurations via --config - #1607
danceratopz wants to merge 3 commits into
Conversation
spencer-tb
left a comment
There was a problem hiding this comment.
Thanks for adding this support! 2 potential paths before approving on my side :)
Let me know what you prefer, I'm happy with both!
1) Keep the current approach, and align the spelling.
Drop the --sim-file alias (hive.go:95) so --sim.file is the only form, and add --client.file:
./hive --sim.file simulators.yaml --client.file clients.yaml2) Or switch to one file and one flag for both. Both parsers exist, so this is mostly wiring, i.e just use a single --configs flag:
# devnet8.yaml
clients:
- client: go-ethereum
build_args: { baseimage: docker.io/ethereum/client-go, tag: latest }
simulators:
- simulator: ethereum/eels/consume-engine
build_args: { tag: glamsterdam-devnet-8 }
- simulator: ethereum/eels/consume-rlp
dockerfile: git
build_args: { branch: devnets/glamsterdam/8, fixtures: tests-glamsterdam-devnet@v8.1.4 }./hive --config devnet8.yaml--client and --sim stay as optional filters over the file, --client-file stays or is removed for --config.
Client and simulator build configurations now share one YAML format: a flat list whose entries are identified by their `client` or `simulator` key. The new --config flag loads such a file, and --client-file becomes an alias of it, so existing client files keep working unchanged and can gain simulator entries in place. The --sim.file and --sim-file flags are removed. Selection over the file is unchanged: --client filters the client entries when set explicitly, and --sim filters the simulator entries. A file without simulator entries falls back to matching --sim against the inventory, and a file without client entries falls back to the --client list.
Thanks, good suggestions! I think it's cleaner to only require specifying a single file.
There's a small tweak to the file format, so that we can alias - client: go-ethereum
build_args:
baseimage: docker.io/ethereum/client-go
tag: latest
- simulator: ethereum/eels/consume-engine
build_args:
fixtures: stable@latest
branch: ""
- simulator: ethereum/eels/consume-rlp
build_args:
fixtures: stable@latest
branch: ""Then: ./hive --config devnet.yamlSelection works as before: ./hive --config devnet.yaml --client go-ethereum --sim consume-engineThe only potential disadvantage is that we insist on a single file, so you can't mix and match client and simulator config files. But I don't think there's a real use case for mixing them, since our images are tied to mainnet or specific devnet configuration anyway. I just pushed this change, but don't mind doing another round to get this right if need be! |
spencer-tb
left a comment
There was a problem hiding this comment.
Awesome! I like that --client-file still works :)
Approved from my side! :)
Adds
--configto load client and simulator build configurations from a single YAML file. The file is a flat list in which each entry is either a client (clientkey) or a simulator (simulatorkey) with an optional Dockerfile extension and build arguments. Client entries use the existing--client-fileformat, so--client-filebecomes an alias of--config: existing client files keep working unchanged and can gain simulator entries in place. This enables per-simulator configuration and selection of alternate Dockerfiles when available. The second commit replaces the initial--sim.fileapproach following review;--sim.fileand--sim-fileare removed.Run with
./hive --config devnet.yaml. Entries run in file order;--clientfilters the client entries when set explicitly,--simfilters the simulator entries using its existing regexp semantics, and--sim.buildargoverrides matching file arguments. A file without simulator entries matches--simagainst the inventory as before, and a file without client entries uses the--clientlist, so client-only and simulator-only files are both valid. Unknown fields, entries with both or neither key,nametagon a simulator, unknown or duplicate simulators, missing alternate Dockerfiles, and empty files are rejected. Passing both--configand--client-fileis an error. The file is optional. Without it,--client,--sim,--sim.buildargand the defaultDockerfilebehave exactly as before, includinghive_context.txtbuild contexts.docs/commandline.mddocuments the file format and currently supported EELS examples. The documentation is intentionally minimal: no simulator ships an alternate Dockerfile yet, so thedockerfilefield is described generically and the examples use only build arguments the current EELS simulators accept. The change adds configuration plumbing; simulator Dockerfiles and image publishing remain separate work.Validation:
gofmt,go test -race . ./internal/... ./hivesim/...,go vet . ./internal/... ./hivesim/...,go build, andgit diff --checkpassed on the branch head. Regression tests cover parsing of client-only, simulator-only and mixed files, selection, argument precedence and isolation, and Docker build requests for default and alternate Dockerfiles with overridden contexts. CLI smoke checks verified both flag spellings, the alias conflict error, and error reporting for missing or invalid files. No live Docker simulation was run for this change.Motivation
We'd like to start publishing images for EELS simulators and this mechanism is required to maintain support of building an arbitrary branch/ref from source (each eels
Dockerfilewould move toDockerfile.gitand be replaced with a minimal Dockerfile that pulls in a pre-built image).