-
Notifications
You must be signed in to change notification settings - Fork 169
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
init: add ability to use existing ISO/sha256sum #210
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM overall, so far I've generally been doing cp -al /srv/fcos/installer /srv/silverblue/installer
then coreos-assembler init --force
.
That said I lean a bit towards baking in the installer to the image, but in the end we're going to move away from Anaconda anyways.
src/cmd-init
Outdated
if [ "${INSTALLER_DIR}" != "null" ] && [ "${FORCE}" != "1" ]; then | ||
if (cd "${INSTALLER_DIR}" && sha256sum -c "${checksums_bn}"); then | ||
(cd installer | ||
cp "${INSTALLER_DIR}"/"${installer_bn}" . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cp --reflink=auto
is a general good practice.
src/cmd-init
Outdated
FORCE=0 | ||
INSTALLER_DIR="null" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm...I'd say use the empty string ""
as an "unset" value.
src/cmd-init
Outdated
mkdir -p installer | ||
|
||
if [ "${INSTALLER_DIR}" != "null" ] && [ "${FORCE}" != "1" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then [ -n "${INSTALLER_DIR}" ] && ...
I found myself doing a lot of `init` operations when testing changes related to #190 and wanted a way to skip the download of the ISO/sha256sum. This change introduces a new flag (`--installerdir`) that will instruct the `init` command to look in a provided directory for the ISO and sha256sum. If the required files are found, they'll be copied to the `installer` directory for use later on. If the `--force` option is provided, it downloads the ISO/sha256sum regardless if the `--installerdir` flag is provided.
Thanks for the feedback! Updated ⬆️ |
👍 |
if (cd "${INSTALLER_DIR}" && sha256sum -c "${checksums_bn}"); then | ||
(cd installer | ||
cp --reflink=auto "${INSTALLER_DIR}"/"${installer_bn}" . | ||
cp --reflink=auto "${INSTALLER_DIR}"/"${checksums_bn}" . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though why even copy it? We could just ln -s
here, right? There's precedence at least doing this with how we handle local paths to checkouts of fedora-coreos-config
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think really we want ln
and then fall back to cp --reflink=auto
, like what ostree does.
But in the end we're going to be dropping Anaconda hopefully soon anyways...
I found myself doing a lot of
init
operations when testing changesrelated to #190 and wanted a way to skip the download of the
ISO/sha256sum.
This change introduces a new flag (
--installerdir
) that willinstruct the
init
command to look in a provided directory for theISO and sha256sum. If the required files are found, they'll be copied
to the
installer
directory for use later on.If the
--force
option is provided, it downloads the ISO/sha256sumregardless if the
--installerdir
flag is provided.