-
Notifications
You must be signed in to change notification settings - Fork 163
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
Verify TORELEASE before performing jail upgrade. #509
Conversation
…eBSD dists. (comment update)
…a release branch. (verify it even before trying to mount the jail prevents accidents, and "exit 1" ensures no unnecessary calls and traffic to FreeBSD's update servers will be performed)
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
Host system running Poudriere with proposed patch:
Creating one jail using FreeBSD dists:
Upgrading previously created jail:
Refusing to upgrade the jail to a snapshot branch (using 'ftp' method; freebsd-update):
|
I'll look today. |
Can you rebase this without b1469c4? |
src/share/poudriere/jail.sh
Outdated
@@ -198,6 +198,16 @@ update_jail() { | |||
msg "Upgrading using ${METHOD}" | |||
case ${METHOD} in | |||
ftp|http|ftp-archive) | |||
# In case we use FreeBSD dists and TORELEASE is present, check if it's a release branch. | |||
if [ ! -z ${TORELEASE} ]; 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.
Just use -n
here rather than ! -z
and please quote ${TORELEASE}: -n "${TORELEASE}"
src/share/poudriere/jail.sh
Outdated
if [ ! -z ${TORELEASE} ]; then | ||
case ${TORELEASE} in | ||
*-ALPHA*|*-CURRENT|*-PRERELEASE|*-STABLE) | ||
msg_error "Only release branches are supported by freebsd-update(8)." |
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.
The use of freebsd-update(8) by poudriere is an implementation detail that is not documented and may change in the future; the user only knows they are using -m ftp
(or http/ftp-archive) here, so we should not reference freebsd-update(8).
Please change freebsd-update(8) here to reference ${METHOD} such as Only release branches are supported with method ${METHOD}"
src/share/poudriere/jail.sh
Outdated
*-ALPHA*|*-CURRENT|*-PRERELEASE|*-STABLE) | ||
msg_error "Only release branches are supported by freebsd-update(8)." | ||
msg_error "Please try to upgrade to a new BETA, RC or RELEASE version." | ||
exit 1 ;; |
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.
Please put ;;
on next line to follow other style for multi-line case statements.
src/share/poudriere/jail.sh
Outdated
@@ -218,15 +228,14 @@ update_jail() { | |||
chmod +x ${JAILMNT}/usr/sbin/freebsd-update.fixed | |||
if [ -z "${TORELEASE}" ]; then | |||
# We're running inside the jail so basedir is /. | |||
# If we start using -b this needs to match it. |
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.
Please rebase out this commit, it was handled separately from this PR.
…e change requests to the way TORELEASE was checked, remove mention to freebsd-update(8) and use ';;' in a new line.
@bdrewery, thank you for the feedback/review. I just applied your requests, and rebased the branch. Do you mind to see if it's okay now? Please let me know. |
…eBSD dists. (comment update) Issue #509
In case we use FreeBSD dists and TORELEASE is present, check if it's a release branch. (verify it even before trying to mount the jail prevents accidents, and "exit 1" ensures no unnecessary calls and traffic to FreeBSD's update servers will be performed) add *-STABLE to the list of possible snapshot versions. Issue #509
In case we use FreeBSD dists to try performing an upgrade on a jail, TORELEASE must be specified; although it must be checked to ensure if we are able to use it with freebsd-update(8). Only release branches like "BETA", "RC", and "RELEASE" are supported.
Check it even before trying to mount the jail prevents accidents (mounting jail rootfs, an additional fs), and "exit 1" ensures no unnecessary calls and traffic to FreeBSD's update servers will be performed.