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.

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

create_disk: Make rootfs size small even for qemu/IaaS #924

Merged
merged 1 commit into from
Nov 18, 2019
Merged

create_disk: Make rootfs size small even for qemu/IaaS #924

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
38 changes: 25 additions & 13 deletions src/cmd-buildextend-metal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,19 +140,30 @@ fi
img=${name}-${build}-${image_type}.${basearch}.${image_format}
path=${PWD}/${img}

# For bare metal images, we estimate the disk size. For qemu, we get it from
# image.yaml.
if [[ $image_type == metal || $image_type == dasd ]]; then
echo "Estimating disk size..."
/usr/lib/coreos-assembler/estimate-commit-disk-size --repo "$ostree_repo" "$ref" --add-percent 20 > "$PWD/tmp/ostree-size.json"
size="$(jq '."estimate-mb".final' "$PWD/tmp/ostree-size.json")"
# extra size is the non-ostree partitions, see create_disk.sh
size="$(( size + 513 ))M"
echo "Disk size estimated to $size"
ignition_platform_id="metal"
ignition_platform_id="${image_type}"
# dasd is a different disk format, but it's still metal. Just like
# if in the future we introduce a 4k sector size x86_64 image type (metal-4k).
if [ "${image_type}" = dasd ]; then
ignition_platform_id=metal
fi

echo "Estimating disk size..."
# The additional 35% here is obviously a hack, but we can't easily completely fill the filesystem,
# and doing so has apparently negative performance implications.
/usr/lib/coreos-assembler/estimate-commit-disk-size --repo "$ostree_repo" "$ref" --add-percent 35 > "$PWD/tmp/ostree-size.json"
rootfs_size="$(jq '."estimate-mb".final' "$PWD/tmp/ostree-size.json")"
# extra size is the non-ostree partitions, see create_disk.sh
image_size="$(( rootfs_size + 513 ))M"
echo "Disk size estimated to ${image_size}"

# For bare metal images, we use the estimated image size. For IaaS/virt, we get it from
# image.yaml because we want a "default" disk size that has some free space.
if [ "${image_type}" = metal ]; then
# Unset the root size, which will inherit from the image size
rootfs_size=0
else
size="$(python3 -c 'import sys, yaml; print(yaml.safe_load(sys.stdin)["size"])' < "$configdir/image.yaml")G"
ignition_platform_id="$image_type"
image_size="$(python3 -c 'import sys, yaml; print(yaml.safe_load(sys.stdin)["size"])' < "$configdir/image.yaml")G"
rootfs_size="${rootfs_size}M"
fi

kargs="$(python3 -c 'import sys, yaml; args = yaml.safe_load(sys.stdin).get("extra-kargs", []); print(" ".join(args))' < "$configdir/image.yaml")"
Expand All @@ -167,7 +178,7 @@ ostree_remote="$(python3 -c 'import sys, yaml; print(yaml.safe_load(sys.stdin).g
save_var_subdirs="$(python3 -c 'import sys, yaml; print(yaml.safe_load(sys.stdin).get("save-var-subdirs-for-selabel-workaround", "NONE"))' < "$configdir/image.yaml")"
luks_flag="$(python3 -c 'import sys, yaml; lf=yaml.safe_load(sys.stdin).get("luks_rootfs", ""); print("--luks-rootfs" if lf.lower() in ("yes", "true") else "")' < "$configdir/image.yaml")"

qemu-img create -f ${image_format} "${path}.tmp" "$size"
qemu-img create -f ${image_format} "${path}.tmp" "${image_size}"
# We support deploying a commit directly instead of a ref
ref_arg=${ref}
if [ -n "${ref_is_temp}" ]; then
Expand All @@ -191,6 +202,7 @@ runvm "${target_drive[@]}" -- \
--ostree-remote "${ostree_remote}" \
--ostree-repo "${ostree_repo}" \
--save-var-subdirs "${save_var_subdirs}" \
--rootfs-size "${rootfs_size}" \
"${luks_flag}"
mv "${path}.tmp" "$path"
echo "{}" > tmp/vm-iso-checksum.json
Expand Down
15 changes: 11 additions & 4 deletions src/create_disk.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ Options:
--ostree-remote: the ostree remote
--ostree-repo: location of the ostree repo
--save-var-subdirs: "yes" to workaround selabel issue for RHCOS
--rootfs-size: Create the root filesystem with specified size
--luks-rootfs: place rootfs in a LUKS container

You probably don't want to run this script by hand. This script is
run as part of 'coreos-assembler build'.
EOC
}

rootfs_size="0"
luks_rootfs=""
extrakargs=""

Expand All @@ -52,6 +54,7 @@ do
--ostree-remote) remote_name="${1}"; shift;;
--ostree-repo) ostree="${1}"; shift;;
--save-var-subdirs) save_var_subdirs="${1}"; shift;;
--rootfs-size) rootfs_size="${1}"; shift;;
--luks-rootfs) luks_rootfs=1;;
*) echo "${flag} is not understood."; usage; exit 10;;
--) break;
Expand Down Expand Up @@ -83,14 +86,18 @@ set -x
# Pin /boot and / to the partition number 1 and 4 respectivelly
BOOTPN=1
ROOTPN=4
# Make the size relative
if [ "${rootfs_size}" != "0" ]; then
rootfs_size="+${rootfs_size}"
fi
case "$arch" in
x86_64)
sgdisk -Z $disk \
-U 00000000-0000-4000-a000-000000000001 \
-n ${BOOTPN}:0:+384M -c ${BOOTPN}:boot \
-n 2:0:+127M -c 2:EFI-SYSTEM -t 2:C12A7328-F81F-11D2-BA4B-00A0C93EC93B \
-n 3:0:+1M -c 3:BIOS-BOOT -t 3:21686148-6449-6E6F-744E-656564454649 \
-n ${ROOTPN}:0:0 -c ${ROOTPN}:root -t ${ROOTPN}:0FC63DAF-8483-4772-8E79-3D69D8477DE4
-n ${ROOTPN}:0:${rootfs_size} -c ${ROOTPN}:root -t ${ROOTPN}:0FC63DAF-8483-4772-8E79-3D69D8477DE4
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, should these be prepended by a +? Otherwise from a scan of sgdisk(8), it seems like it interprets it as an offset from the start of the disk (so the partition size would actually be rootfs_size - (384 + 127 + 1)).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, very nice catch! I had that in the other PR but lost it when reworking it here.
Fixed ⬇️

sgdisk -p "$disk"
EFIPN=2
BIOSPN=3
Expand All @@ -100,15 +107,15 @@ case "$arch" in
-U 00000000-0000-4000-a000-000000000001 \
-n ${BOOTPN}:0:+384M -c ${BOOTPN}:boot \
-n 2:0:+127M -c 2:EFI-SYSTEM -t 2:C12A7328-F81F-11D2-BA4B-00A0C93EC93B \
-n ${ROOTPN}:0:0 -c ${ROOTPN}:root -t ${ROOTPN}:0FC63DAF-8483-4772-8E79-3D69D8477DE4
-n ${ROOTPN}:0:${rootfs_size} -c ${ROOTPN}:root -t ${ROOTPN}:0FC63DAF-8483-4772-8E79-3D69D8477DE4
sgdisk -p "$disk"
EFIPN=2
;;
s390x)
sgdisk -Z $disk \
-U 00000000-0000-4000-a000-000000000001 \
-n ${BOOTPN}:0:+384M -c ${BOOTPN}:boot \
-n ${ROOTPN}:0:0 -c ${ROOTPN}:root -t ${ROOTPN}:0FC63DAF-8483-4772-8E79-3D69D8477DE4
-n ${ROOTPN}:0:${rootfs_size} -c ${ROOTPN}:root -t ${ROOTPN}:0FC63DAF-8483-4772-8E79-3D69D8477DE4
sgdisk -p "$disk"
;;
ppc64le)
Expand All @@ -117,7 +124,7 @@ case "$arch" in
-U 00000000-0000-4000-a000-000000000001 \
-n 2:0:+4M -c 2:PowerPC-PReP-boot -t 2:9E1A2D38-C612-4316-AA26-8B49521E5A8B \
-n ${BOOTPN}:0:+384M -c ${BOOTPN}:boot \
-n ${ROOTPN}:0:0 -c ${ROOTPN}:root -t ${ROOTPN}:0FC63DAF-8483-4772-8E79-3D69D8477DE4
-n ${ROOTPN}:0:${rootfs_size} -c ${ROOTPN}:root -t ${ROOTPN}:0FC63DAF-8483-4772-8E79-3D69D8477DE4
sgdisk -p "$disk"
PREPPN=2
;;
Expand Down