Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add scripts for setting up hoeve #620

Merged
merged 8 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions hitl-setup/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<!--
SPDX-FileCopyrightText: 2024 Google LLC

SPDX-License-Identifier: Apache-2.0
-->
This folder contains all information and scripts to bootstrap the HITL setup. Setting up should be done manually by running `setup.sh`, CI should only check if the setup is correct.

The setup script does the following:
* Create a bridge device
* Connect all interfaces defined in the `fpganet` file to the bridge
* Set up the bridge device with a static IP address defined in `fpganet`
* Set up DHCP server for the bridge device
10 changes: 10 additions & 0 deletions hitl-setup/dhcpd.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# SPDX-FileCopyrightText: 2024 Google LLC
#
# SPDX-License-Identifier: CC0-1.0

default-lease-time 600;
max-lease-time 7200;
ddns-update-style none;
subnet 10.0.0.0 netmask 255.255.255.0 {
range 10.0.0.2 10.0.0.254;
}
leonschoorl marked this conversation as resolved.
Show resolved Hide resolved
7 changes: 7 additions & 0 deletions hitl-setup/fpganet
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# SPDX-FileCopyrightText: 2024 Google LLC
#
# SPDX-License-Identifier: CC0-1.0

BRIDGE_NAME=eth-fpga
INTERFACES="enp3s0 enp4s0 enp5s0 enp6s0 enp13s0 enp14s0 enp15s0 enp16s0"
INTERFACE_IP=10.0.0.1/24
27 changes: 27 additions & 0 deletions hitl-setup/fpganet.service
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# SPDX-FileCopyrightText: 2024 Google LLC
#
# SPDX-License-Identifier: CC0-1.0

[Unit]
Description=Set up bridge network to combine multiple NICs into one interface
DefaultDependencies=no
Wants=network.target systemd-udev-settle.service
After=local-fs.target network-pre.target systemd-sysctl.service systemd-modules-load.service systemd-udev-settle.service
Before=network.target shutdown.target network-online.target
Conflicts=shutdown.target

[Service]
Type=oneshot
RemainAfterExit=true
EnvironmentFile=/etc/fpganet
ExecStart=/usr/sbin/ip link add name $BRIDGE_NAME type bridge
ExecStart=/usr/sbin/ip link set dev $BRIDGE_NAME up
ExecStart=/usr/sbin/ip address add $INTERFACE_IP dev $BRIDGE_NAME
ExecStart=/usr/bin/sh -c 'for INTERFACE in $INTERFACES; do /usr/sbin/ip link set dev $INTERFACE up; done'
ExecStart=/usr/bin/sh -c 'for INTERFACE in $INTERFACES; do /usr/sbin/ip link set dev $INTERFACE master $BRIDGE_NAME; done'

ExecStop=/usr/sbin/ip link delete name $BRIDGE_NAME type bridge

[Install]
WantedBy=multi-user.target
WantedBy=network-online.target
Comment on lines +26 to +27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
WantedBy=multi-user.target
WantedBy=network-online.target
WantedBy=multi-user.target
WantedBy=network-online.target
WantedBy=$whatever-the-github-actions-runner-is

?

Copy link
Contributor

@DigitalBrains1 DigitalBrains1 Sep 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to prevent the runner from running if this script failed? Because Wanted is not strong enough for that.

If it's just about ordering, that's already taken care of by Before=network.target.

[edit]
Silly me. Wanted doesn't do ordering in the first place. Sorry, please explain what you're trying to accomplish, so we can work out what to do.
[/edit]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to prevent the runner from running if this script failed?

Yes.

How would you specify this?

Copy link
Contributor

@DigitalBrains1 DigitalBrains1 Sep 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add this to the runner:

[Unit]
Requires=fpganet.service
After=fpganet.service

but if you want the runner to be able to run if you systemctl disable fpganet.service, you could add this to fpganet.service:

[Unit]
Before=actions.runner.bittide-bittide-hardware.hoeve-1.service

[Install]
RequiredBy=actions.runner.bittide-bittide-hardware.hoeve-1.service

But this seems weird. Why would it suddenly no longer require the service just because you systemctl disabled it? On the other hand, if you do that, you might have good reason to? I dunno :-).

[edit]
Fixed dumb copy-paste error in latter config snippet.
[/edit]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under the (possibly naive) impression that just doing:

systemctl restart fpganet.service
systemctl enable fpganet.service
[...]
systemctl add-wants isc-dhcp-server.service fpganet.service
systemctl restart isc-dhcp-server.service
systemctl enable isc-dhcp-server.service

would be good enough to make sure that first the fpganet.service gets started, and then isc-dhcp-server.service, somewhere during boot up of the machine.

If you want to do it "really properly" you should probably add a dependency between the runner and the dhcp server.
Just depending on the setting up of the bridge isn't strictly enough.

Copy link
Contributor

@DigitalBrains1 DigitalBrains1 Sep 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand what you mean, but I don't see any ordering in the commands you give. Those commands will certainly not start the fpganet.service before isc-dhcp-server.service.

Note that requirement dependencies do not influence the order in which services are started or stopped. This has to be configured independently with the After= or Before= options. If unit foo.service pulls in unit bar.service as configured with Wants= and no ordering is configured with After= or Before=, then both units will be started simultaneously and without any delay between them if foo.service is activated.

[edit]
Well, it depends on what is taken as a given. As written currently, clearly fpganet.service will run before isc-dhcp-server.service because fpganet.service is Before=network-online.target and isc-dhcp-server.service is After=network-online.target and Wants=network-online.target. But I meant that your commands did not create this ordering; it is a pre-existing ordering, your commands do not establish it.
[/edit]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to do it "really properly" you should probably add a dependency between the runner and the dhcp server.

Doing that (if I understand what you mean correctly) means that during bootup, it will wait for the DHCP server to be started before launching the runners. But the DHCP server doesn't need to be available until well into a CI run, not for the runner to be able to start. So I think this is overkill and not really the systemd "we want boot to be quick and ordering dependencies to be few" philosophy. I think we needed fpganet.service to be well and truly before the runner because otherwise Docker would not be able to bind the bridge interface into its runtime; in other words, the runner truly does depend on fpganet.service to be available when it is starting.

But I may be misinterpreting, there are so many ways to interpret "add a dependency" in systemd land.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we needed fpganet.service to be well and truly before the runner because otherwise Docker would not be able to bind the bridge interface into its runtime;

This was an earlier experiment, for the current version, the runner can start fine without the bridge. Obviously CI jobs that need the bridge will fail without it, but the runner starts fine and other jobs run fine.

48 changes: 48 additions & 0 deletions hitl-setup/setup.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
#!/bin/bash
# SPDX-FileCopyrightText: 2024 Google LLC
#
# SPDX-License-Identifier: Apache-2.0

FORCE="$1"
set -xuo pipefail
HERE=$(realpath "$(dirname "$0")")
source "$HERE/fpganet" || exit $?

echo Checking that all INTERFACES exist
for INTERFACE in $INTERFACES; do
ip addr show $INTERFACE > /dev/null || exit $?
done
echo All INTERFACES exist

FILES="/etc/systemd/system/fpganet.service /etc/fpganet /etc/dhcp/dhcpd.conf /etc/default/isc-dhcp-server"
EXISTS=
for FILE in $FILES; do
if [ -e $FILE ]; then
EXISTS="$EXISTS $FILE"
fi
done
if [ -n "$EXISTS" -a "$FORCE" != -f ]; then
set +x
echo "Running this script with -f will overwrite:"
echo "$EXISTS"
echo "But now it will stop, because -f wasn't supplied"
exit 1
fi

set -e
apt-get install -y isc-dhcp-server iproute2

cp "$HERE/fpganet.service" /etc/systemd/system
cp "$HERE/fpganet" /etc

cp "$HERE/dhcpd.conf" /etc/dhcp/
echo "INTERFACESv4=\"$BRIDGE_NAME\"" > /etc/default/isc-dhcp-server

systemctl daemon-reload

systemctl enable fpganet.service
systemctl restart fpganet.service

systemctl disable isc-dhcp-server6.service
systemctl enable isc-dhcp-server.service
systemctl restart isc-dhcp-server.service
Loading