fix(builders/netboot): make "normal" evaluation pass #111

Closed

janik wants to merge 1 commit from builder into main

pull from: builder

merge into: the-distro:main

the-distro:main

the-distro:storage-project

the-distro:block-crawlers

the-distro:fix-buildbot

the-distro:lukegb/rule-94

the-distro:cache-lix

the-distro:build02-lix

the-distro:lukegb/uptime-kuma-root

the-distro:hydra-metrics

the-distro:email-groups

the-distro:cache-ipv4

the-distro:format

the-distro:hexchen/colmena-checker

the-distro:hexy/credentials

the-distro:buildbot

the-distro:ckie/moarr-v4

the-distro:gerrit-metrics

the-distro:moar-builders

the-distro:unplug-epyc

the-distro:hydra-wire-up

the-distro:alertmanager

the-distro:fodwatch

the-distro:cadvisor-containers

the-distro:gerrit-http-clones

the-distro:meta01

Conversation 6 Commits 1 Files changed 2 +12 -3

janik commented 2024-09-21 01:48:57 +00:00

Owner

Copy link

Without this patch running colmena build will run into a few assertion
errors for machines that have config.bagel.baremetal.builders.netboot == true
set. This is due to an assertion check in the initrd module making sure
there is a mount point for /. This can be trivially fixed by just
setting the mount point to the real world value, which is a tmpfs with
64GB assigned.

We also set deployment.targetHost to a domain that will
never resolve in the public internet, to make sure nobody applies these
machines by hand. It would have been nice to throw a error whenever
colmena apply gets executed for one of these hosts, but doing so would
defeat the purpose of this patch, because the colmena build and apply
argument both evaluate the exact same code paths and thus colmena
build would error again.

The motivation behind this was, so we could run colmena build in CI
in the future, and to not scare of new contributors with random build
failures when they first try to build the machines.

The proper solution would be to exclude all the network booted builders
from the regular colmena hive that is exposed to the cli, but this is
too many yaks to shave for now.

Without this patch running `colmena build` will run into a few assertion errors for machines that have `config.bagel.baremetal.builders.netboot == true` set. This is due to an assertion check in the initrd module making sure there is a mount point for `/`. This can be trivially fixed by just setting the mount point to the real world value, which is a tmpfs with 64GB assigned. We also set `deployment.targetHost` to a domain that will never resolve in the public internet, to make sure nobody applies these machines by hand. It would have been nice to throw a error whenever `colmena apply` gets executed for one of these hosts, but doing so would defeat the purpose of this patch, because the colmena `build` and `apply` argument both evaluate the exact same code paths and thus colmena `build` would error again. The motivation behind this was, so we could run `colmena build` in CI in the future, and to not scare of new contributors with random build failures when they first try to build the machines. The proper solution would be to exclude all the network booted builders from the regular colmena hive that is exposed to the cli, but this is too many yaks to shave for now.

janik added 1 commit 2024-09-21 01:49:02 +00:00

fix(builders/netboot): make "normal" evaluation pass ... 9bab95bab7

Without this patch running `colmena build` will run into a few assertion
errors for machines that have `config.bagel.baremetal.builders.netboot == true`
set. This is due to an assertion check in the initrd module making sure
there is a mount point for `/`. This can be trivially fixed by just
setting the mount point to the real world value, which is a tmpfs with
64GB assigned.

We also set `deployment.targetHost` to a domain that will
never resolve in the public internet, to make sure nobody applies these
machines by hand. It would have been nice to throw a error whenever
`colmena apply` gets executed for one of these hosts, but doing so would
defeat the purpose of this patch, because the colmena `build` and `apply`
argument both evaluate the exact same code paths and thus colmena
`build` would error again.

The motivation behind this was, so we could run `colmena build` in CI
in the future, and to not scare of new contributors with random build
failures when they first try to build the machines.

The proper solution would be to exclude all the network booted builders
from the regular colmena hive that is exposed to the cli, but this is
too many yaks to shave for now.

janik requested review from yu-re-ka 2024-09-21 01:49:14 +00:00

kiaragrouwstra commented 2024-09-22 20:39:56 +00:00

Contributor

Copy link

Thanks. This seems to address the error about fileSystems I was getting.

Thanks. This seems to address the error about `fileSystems` I was getting.

janik force-pushed builder from 9bab95bab7 to 464a726664 2024-09-23 21:14:32 +00:00 Compare

raito commented 2024-09-28 20:03:40 +00:00

Owner

Copy link

Alternative solutions would be to do colmena build --on @servers and have @builders be a separate tag?
Or we can adopt a special colmena fork and fix this upstream.

I have patches to make this sort of stuff work ~better.

Alternative solutions would be to do `colmena build --on @servers` and have `@builders` be a separate tag? Or we can adopt a special colmena fork and fix this upstream. I have patches to make this sort of stuff work ~better.

raito commented 2024-09-28 20:04:00 +00:00

Owner

Copy link

(I'm a bit on the fence regarding letting an invalid configuration exist at all, but I understand why it can be nice to fix this in an easier way.)

(I'm a bit on the fence regarding letting an invalid configuration exist at all, but I understand why it can be nice to fix this in an easier way.)

janik commented 2024-09-29 01:42:25 +00:00

Author

Owner

Copy link

Or we can adopt a special colmena fork and fix this upstream.
I have patches to make this sort of stuff work ~better.

Can you like those patches, I would be curious to read them. (I assume it's your custom activation stuff)

Alternative solutions would be to do colmena build --on @servers and have @builders be a separate tag?

That's less clear for people unfamiliar with the code base.

(I'm a bit on the fence regarding letting an invalid configuration exist at all, but I understand why it can be nice to fix this in an easier way.)

Understandable, but the problem already exists without this patch. The pr would merely alleviate some of the associated user pain.

> Or we can adopt a special colmena fork and fix this upstream. > I have patches to make this sort of stuff work ~better. Can you like those patches, I would be curious to read them. (I assume it's your custom activation stuff) > Alternative solutions would be to do colmena build --on @servers and have @builders be a separate tag? That's less clear for people unfamiliar with the code base. > (I'm a bit on the fence regarding letting an invalid configuration exist at all, but I understand why it can be nice to fix this in an easier way.) Understandable, but the problem already exists without this patch. The pr would merely alleviate some of the associated user pain.

janik force-pushed builder from 464a726664 to cef88ec598 2024-09-29 01:44:20 +00:00 Compare

raito commented 2024-09-29 14:23:04 +00:00

Owner

Copy link

Understandable, but the problem already exists without this patch. The pr would merely alleviate some of the associated user pain.

Right now, colmena building these builders will cause the NixOS module fail and make you unable to produce invalid artifacts.

Can you like those patches, I would be curious to read them. (I assume it's your custom activation stuff)

c959d643db
and there must be some others in another tree, I don't find right now.

That's less clear for people unfamiliar with the code base.

But fairly well documented in colmena and we can document it again.

> Understandable, but the problem already exists without this patch. The pr would merely alleviate some of the associated user pain. Right now, colmena building these builders will cause the NixOS module fail and make you unable to produce invalid artifacts. > Can you like those patches, I would be curious to read them. (I assume it's your custom activation stuff) https://git.dgnum.eu/DGNum/colmena/commit/c959d643dbfe7e932c359947f653b1eb49b2be35 and there must be some others in another tree, I don't find right now. > That's less clear for people unfamiliar with the code base. But fairly well documented in colmena and we can document it again.

raito referenced this pull request 2024-10-05 14:37:27 +00:00

feat: tag machine accordingly #124

raito commented 2024-10-06 08:10:09 +00:00

Owner

Copy link

Superseded by #124.

Superseded by #124.

raito closed this pull request 2024-10-06 08:10:09 +00:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

Reviewers

No reviewers

yu-re-ka

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: the-distro/infra#111

No description provided.