Skip to content

cmd-buildextend-metal: dont use anaconda on x86_64 #563

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.

Sign up for GitHub

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

Merged
jlebon merged 1 commit into from
Jun 27, 2019
Merged

cmd-buildextend-metal: dont use anaconda on x86_64 #563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
81 changes: 47 additions & 34 deletions src/cmd-buildextend-metal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,16 @@ dn=$(dirname "$0")
print_help() {
cat 1>&2 <<'EOF'
Usage: coreos-assembler buildextend-metal --help
coreos-assembler buildextend-metal [--build ID] [--bios] [--uefi]
coreos-assembler buildextend-metal [--build ID]

Build raw metal images for bios or metal. If neither switches are provided,
both targets are built.
Build a bare metal image.
EOF
}

# Parse options
BIOS=
UEFI=
rc=0
build=
options=$(getopt --options h --longoptions help,build:,bios,uefi -- "$@") || rc=$?
options=$(getopt --options h --longoptions help,build: -- "$@") || rc=$?
[ $rc -eq 0 ] || {
print_help
exit 1
Expand All @@ -32,12 +29,6 @@ while true; do
print_help
exit 0
;;
--bios)
BIOS=1
;;
--uefi)
UEFI=1
;;
--build)
build=$2
shift
Expand All @@ -62,11 +53,15 @@ if [ $# -ne 0 ]; then
fatal "Too many arguments passed"
fi

# default to both of them
if [ -z "${BIOS}" ] && [ -z "${UEFI}" ]; then
BIOS=1
UEFI=1
fi
case "$arch" in
"x86_64"|"aarch64")
## fall through to the rest of the file
;;
*)
echo "$arch is not supported for this command"
exit 1
;;
esac

export LIBGUESTFS_BACKEND=direct

Expand Down Expand Up @@ -118,26 +113,45 @@ if [ "${rev_parsed}" != "${commit}" ]; then
fi # otherwise, the repo already has a ref, so no need to create
fi


itype=metal
img=${name}-${build}-${itype}.raw
if [ -f "${builddir}/${img}" ]; then
echo "Bare metal image already exists"
exit
fi

path=${PWD}/${img}
# for anaconda logs
mkdir -p tmp/anaconda

for itype in ${BIOS:+metal-bios} ${UEFI:+metal-uefi}; do
img=${name}-${build}-${itype}.raw
if [ -f "${builddir}/${img}" ]; then
echo "Image $itype already exists"
continue
fi

path=${PWD}/${img}
run_virtinstall "${ostree_repo}" "${ref}" "${path}.tmp" --variant="${itype}"
if [ "$arch" == "aarch64" ]; then
run_virtinstall "${ostree_repo}" "${ref}" "${path}.tmp" --variant="${itype}-uefi"
Copy link
Member

@jlebon jlebon Jun 27, 2019

Choose a reason for hiding this comment

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

Hmm, how come we don't have to estimate the disk size in this path?

Copy link
Contributor Author

@ajeddeloh ajeddeloh Jun 27, 2019

Choose a reason for hiding this comment

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

We do, it's in run_virtinstall

/usr/lib/coreos-assembler/gf-platformid "${path}"{.tmp,} metal
rm -f "${path}".tmp
else
if [ ! -e "$PWD/tmp/ostree-size.json" ]; then
echo "Estimating disk size..."
/usr/lib/coreos-assembler/estimate-commit-disk-size --repo "$ostree_repo" "$ref" > "$PWD/tmp/ostree-size.json"
Copy link
Member

@jlebon jlebon Jun 27, 2019

Choose a reason for hiding this comment

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

An echo "Estimating disk size..." here would be useful, since it does take some time.

fi
size="$(jq '."estimate-mb".final' "$PWD/tmp/ostree-size.json")"
# 384M is the non-ostree partitions
Copy link
Member

@jlebon jlebon Jun 27, 2019

Choose a reason for hiding this comment

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

Random thought related to this: I remember we recently discussed the fact that because we don't actually expand until the real root, it technically limits how much Ignition can write to the disk. Or I forgot, did we say we want to switch that auto-expand logic to a base config so ignition-disks does it?

Copy link
Contributor Author

@ajeddeloh ajeddeloh Jun 27, 2019

Choose a reason for hiding this comment

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

I don't remember where we stand on that either, but it's somewhat disconnected from this PR. Ignition disks notably can't extend filesystems, only partitions (fixing that is... complicated). We might think about doing something between disks and files though (effectively moving coreos-growpart or something like it to the initramfs).

This currently matches the existing behavior, lets separate any changes there to another PR?

Copy link
Member

@jlebon jlebon Jun 27, 2019

Choose a reason for hiding this comment

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

Yup, WFM!

size="$(( size + 384 ))M"
echo "Disk size estimated to $size"
kargs="$(python3 -c 'import sys, yaml; args = yaml.load(sys.stdin)["extra-kargs"]; print(" ".join(args))' < "$configdir/image.yaml")"
kargs="$kargs console=tty0 console=${VM_TERMINAL},115200n8 ignition.platform.id=metal"

qemu-img create -f qcow2 "${path}.qcow2" "$size"
runvm_with_disk "${path}.qcow2" /usr/lib/coreos-assembler/create_disk.sh /dev/vda "$ostree_repo" "${ref-:${commit}}" /usr/lib/coreos-assembler/grub.cfg "$name" "\"$kargs\""
qemu-img convert "${path}.qcow2" "$path"
rm "${path}.qcow2"
fi

# flush it out before going to the next one so we don't have to redo both if
# the next one fails
# flush it out before going to the next one so we don't have to redo both if
# the next one fails

# there's probably a jq one-liner for this...
python3 -c "
# there's probably a jq one-liner for this...
python3 -c "
import sys, json
j = json.load(sys.stdin)
j['images']['$itype'] = {
Expand All @@ -148,7 +162,6 @@ j['images']['$itype'] = {
json.dump(j, sys.stdout, indent=4)
" < "${builddir}/meta.json" > meta.json.new

# and now the crucial bit
mv -T meta.json.new "${builddir}/meta.json"
mv -T "${img}" "${builddir}/${img}"
done
# and now the crucial bit
mv -T meta.json.new "${builddir}/meta.json"
mv -T "${img}" "${builddir}/${img}"
2 changes: 1 addition & 1 deletion src/gf-platformid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ coreos_gf_run_mount "${tmp_dest}"
# * grub config
# * BLS config (for subsequent config regeneration)
# First, the grub config.
if [ "$arch" != "x86_64" ] || [ "$platformid" == "metal" ]; then
if [ "$arch" != "x86_64" ]; then
if [ "$(coreos_gf exists '/boot/efi')" == 'true' ]; then
grubcfg_path=$(coreos_gf glob-expand /boot/efi/EFI/*/grub.cfg)
else
Expand Down