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

Mac PW Pool: Install libkrun (krunkit) #208

Merged
merged 1 commit into from
Jul 31, 2024
Merged
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
49 changes: 44 additions & 5 deletions mac_pw_pool/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
#
# This script should be called with a single argument string,
# of the label YAML to configure. For example "purpose: prod"
#
# N/B: Under special circumstances, this script (possibly with modifications)
# can be executed more than once. All operations which modify state/config.
# must be wrapped in conditional checks.

set -eo pipefail

Expand Down Expand Up @@ -74,10 +78,43 @@ grep -q homebrew /etc/paths || \
# environment isn't loaded automatically.
. /etc/profile

msg "Installing podman-machine, testing, and CI deps. (~2m install time)"
msg "Installing podman-machine, testing, and CI deps. (~5-10m install time)"
Copy link
Member

Choose a reason for hiding this comment

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

Is this bump due to multiple brew commands? If so, would it be worth assembling a string (with comments on each one, as you're doing here) then running it all in one command?

Copy link
Member Author

Choose a reason for hiding this comment

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

Bump was based on observation w/ addition of 'krunkit' and breaking up the install command. Brew produces quite a bit of output during each install, and install order is significant. Like DNF repos., download times can also vary quite a bit due to many factors.

I chose to break up the installs in this PR mainly due to the slp/homebrew-krunkit problem I ran into. This appears to have been resolved now, so I could switch to a package array w/ line comments and a single install command.

I'll give it one try, but if it fails this will need to be done as a followup. The testing-feature is needed somewhat urgently and I won't be able to monkey around with the 3-hour DH spinup/spindown delays.

Copy link
Member Author

Choose a reason for hiding this comment

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

FWIW, this is the setup.log from running the script in 783ae1e

setup.log

if [[ ! -x /usr/local/bin/gvproxy ]]; then
brew tap cfergeau/crc
brew install go go-md2man coreutils pkg-config pstree gpgme vfkit cirruslabs/cli/cirrus
declare -a brew_taps
declare -a brew_formulas

brew_taps=(
# Required to use upstream vfkit
cfergeau/crc

# Required to use upstream krunkit
slp/krunkit
)

brew_formulas=(
# Necessary for worker-pool participation + task execution
cirruslabs/cli/cirrus

# Necessary for building podman|buildah|skopeo
go go-md2man coreutils pkg-config pstree gpgme

# Necessary for testing podman-machine
vfkit

# Necessary for podman-machine libkrun CI testing
krunkit
)

# msg() includes a ##### prefix, ensure this text is simply
# associated with the prior msg() output.
echo " Adding taps[] ${brew_taps[*]}"
echo " before installing formulas[] ${brew_formulas[*]}"

for brew_tap in "${brew_taps[@]}"; do
brew tap $brew_tap
done

brew install "${brew_formulas[@]}"

# Normally gvproxy is installed along with "podman" brew. CI Tasks
# on this instance will be running from source builds, so gvproxy must
Expand Down Expand Up @@ -158,7 +195,9 @@ if ! arch -arch x86_64 /usr/bin/uname -m; then
fi

msg "Restricting appstore/software install to admin-only"
if [[ -x /usr/sbin/softwareupdate ]]; then
# Abuse the symlink existance as a condition for running `sudo defaults write ...`
# since checking the state of those values is complex.
if [[ ! -L /usr/local/bin/softwareupdate ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Taking a step back, is it really worth preserving the if? You're doing ln -sf (force) below, so even if this runs twice it should be ok. And it would really simplify life for a maintainer: no need to think about the conditional.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I can change this easy enough. It's such a toungue-in-cheek "fix" anyway, best keep it as simple as possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Investigated doing this, it's not worth it. The setup script needs to be able to run more than once on a mac. This symlink's existence "protecting" execution of a few apple-specific config. commands where retrieving their current state is complex. I could use a separate marker-file, but didn't think there was much point.

I'll add a comment detailing this for future maintainers.

# Ref: https://developer.apple.com/documentation/devicemanagement/softwareupdate
sudo defaults write com.apple.SoftwareUpdate restrict-software-update-require-admin-to-install -bool true
sudo defaults write com.apple.appstore restrict-store-require-admin-to-install -bool true
Expand All @@ -168,7 +207,7 @@ if [[ -x /usr/sbin/softwareupdate ]]; then
# also desireable to limit use of the utility in a CI environment generally.
# Since /usr/sbin is read-only, but /usr/local is read-write and appears first
# in $PATH, deploy a really fragile hack as an imperfect workaround.
sudo ln -s /usr/bin/false /usr/local/bin/softwareupdate
sudo ln -sf /usr/bin/false /usr/local/bin/softwareupdate
fi

# FIXME: Semi-secret POOLTOKEN value should not be in this file.
Expand Down