Bug 261446 - freebsd-update should not create boot environment when it's not making changes to the boot environment
Summary: freebsd-update should not create boot environment when it's not making change...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Kyle Evans
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-01-24 21:39 UTC by Xin LI
Modified: 2022-12-14 18:11 UTC (History)
9 users (show)

See Also:
kevans: mfc-stable13+
kevans: mfc-stable12+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Xin LI freebsd_committer freebsd_triage 2022-01-24 21:39:57 UTC
I have noticed that f28f138905416c45ebaa6429f44a0b88a72f54b1 have implemented a feature that would create a new boot environment upon installation.

While the feature itself is very useful, there are some scenarios that we don't want a new boot environment to be created unconditionally.  One of these scenarios would be poudriere, which runs freebsd-update without jailing into the jails.

I think the behavior should be disabled when -b is specified AND when it's not /, because the boot environment is not going to be changed.

I'd like to make the following feature requests:

1) Make the creation of boot environment to be named after both freebsd-version -k and freebsd-version -u results, in other words:

diff --git a/usr.sbin/freebsd-update/freebsd-update.sh b/usr.sbin/freebsd-update/freebsd-update.sh
index 1776115d0776..a6f758813cd8 100644
--- a/usr.sbin/freebsd-update/freebsd-update.sh
+++ b/usr.sbin/freebsd-update/freebsd-update.sh
@@ -911,7 +911,7 @@ install_create_be () {
                esac
                if [ ${CREATEBE} = yes ]; then
                        echo -n "Creating snapshot of existing boot environment... "
-                       VERSION=`freebsd-version -k`
+                       VERSION=`freebsd-version -ku | sort -V | tail -n 1`
                        TIMESTAMP=`date +"%Y-%m-%d_%H%M%S"`
                        bectl create ${VERSION}_${TIMESTAMP}
                        if [ $? -eq 0 ]; then


2) Make freebsd-update to not create boot environment when -b is specified and the base directory is /

3) It's probably reasonable to have a RC script or periodic script to do something like:

(bectl list -H | awk '{print $1;}' | grep -q $(freebsd-version -k) ) || bectl create $(freebsd-version -k)_$(date +%Y-%m-%d_%H%M%S)

4) There should be some way to clean up the automatically created boot environments.  It makes sense to keep them around for e.g. a few weeks, but running out of disk space on / is not really fun.
Comment 1 FiLiS 2022-01-27 14:51:39 UTC
Also ran into this. We update jails with "freebsd-update -b" from the host and after updating a couple of jails from 12.2 to 12.3 we ended up with 2 BEs per jail + 2 for the host.
Comment 2 Julien Cigar 2022-01-28 13:17:59 UTC
Today I was upgrading my jails on a 12.3-RELEASE host with $> freebsd-update -j myjail -r 12.3-RELEASE upgrade and also noticed this: a boot environment is created for every upgraded jail.

The logic behind "CreateBootEnv" should probably not work when using -j (and/or -b)
Comment 3 Kyle Evans freebsd_committer freebsd_triage 2022-01-28 16:20:26 UTC
(In reply to Xin LI from comment #0)

Someone pointed out #2 last night in IRC, and I think that alone is EN worthy. I think we want something like:

diff --git a/usr.sbin/freebsd-update/freebsd-update.sh b/usr.sbin/freebsd-update/freebsd-update.sh
index 1776115d0776..73b5003ee036 100644
--- a/usr.sbin/freebsd-update/freebsd-update.sh
+++ b/usr.sbin/freebsd-update/freebsd-update.sh
@@ -890,7 +890,12 @@ install_check_params () {
 install_create_be () {
        # Figure out if we're running in a jail and return if we are
        if [ `sysctl -n security.jail.jailed` = 1 ]; then
-           return 1
+               return 1
+       fi
+       # We may allow overriding the BASEDIR check at a later time via an
+       # argument, so separate this out.
+       if [ "$BASEDIR" != "/" ]; then
+               return 1
        fi
        # Create a boot environment if enabled
        if [ ${BOOTENV} = yes ]; then
Comment 4 Ed Maste freebsd_committer freebsd_triage 2022-02-03 02:52:52 UTC
(In reply to Kyle Evans from comment #3)
Your patch looks good although I don't quite get the comment -- I think Xin Li's comment that the boot env is not going to be changed is clearer.
Comment 5 Kyle Evans freebsd_committer freebsd_triage 2022-02-03 03:01:05 UTC
(In reply to Ed Maste from comment #4)

The point is that we don't know that it won't change the BE, e.g., if a jail root has been folded in intentionally. It may make sense to provide an override to create it anyways.
Comment 6 commit-hook freebsd_committer freebsd_triage 2022-02-12 21:37:14 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e01e8f911b935eabcc35b4d121951e4e21042ee5

commit e01e8f911b935eabcc35b4d121951e4e21042ee5
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2022-02-12 21:36:24 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2022-02-12 21:36:24 +0000

    freebsd-update: improve BE creation feature

    This addresses one nit and one bug in the BE creation feature of
    freebsd-update:

    The nit addressed is that it currently only names the BEs after the
    userland version, but the kernel version may be higher.  After this
    change, we request both and pass them through sort(1) to choose the
    highest.  This is especially helpful if a freebsd-update patch touched
    one but not the other.

    The bug fixed is that roots updated that are not located at '/', e.g.,
    by using -b or -j, will no longer create boot environments
    automatically.  There's a very low chance these will actually change the
    BE in any meaningful way, anyways.  It could make sense in the future
    to allow an argument-override to create the BE anyways if someone comes
    up with a non-standard setup, e.g., where a jail is an important part of
    their boot environment on an appliance or some such setup.

    Half of this patch is submitted by delphij@, the other half kevans@.

    PR:             261446
    MFC after:      3 days
    Reviewed by:    delphij, emaste, Dave Fullard <dave_fullard.ca>
    Differential Revision:  https://reviews.freebsd.org/D34257

 usr.sbin/freebsd-update/freebsd-update.sh | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
Comment 7 commit-hook freebsd_committer freebsd_triage 2022-02-15 06:10:29 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=47c07814051a86fec9088a1f78d64fdada8abed6

commit 47c07814051a86fec9088a1f78d64fdada8abed6
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2022-02-12 21:36:24 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2022-02-15 06:09:50 +0000

    freebsd-update: improve BE creation feature

    This addresses one nit and one bug in the BE creation feature of
    freebsd-update:

    The nit addressed is that it currently only names the BEs after the
    userland version, but the kernel version may be higher.  After this
    change, we request both and pass them through sort(1) to choose the
    highest.  This is especially helpful if a freebsd-update patch touched
    one but not the other.

    The bug fixed is that roots updated that are not located at '/', e.g.,
    by using -b or -j, will no longer create boot environments
    automatically.  There's a very low chance these will actually change the
    BE in any meaningful way, anyways.  It could make sense in the future
    to allow an argument-override to create the BE anyways if someone comes
    up with a non-standard setup, e.g., where a jail is an important part of
    their boot environment on an appliance or some such setup.

    Half of this patch is submitted by delphij@, the other half kevans@.

    PR:             261446

    (cherry picked from commit e01e8f911b935eabcc35b4d121951e4e21042ee5)

 usr.sbin/freebsd-update/freebsd-update.sh | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
Comment 8 commit-hook freebsd_committer freebsd_triage 2022-02-15 06:11:31 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=35d33d408213d20c63d60c0dfcdf77b2b36f5eee

commit 35d33d408213d20c63d60c0dfcdf77b2b36f5eee
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2022-02-12 21:36:24 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2022-02-15 06:09:41 +0000

    freebsd-update: improve BE creation feature

    This addresses one nit and one bug in the BE creation feature of
    freebsd-update:

    The nit addressed is that it currently only names the BEs after the
    userland version, but the kernel version may be higher.  After this
    change, we request both and pass them through sort(1) to choose the
    highest.  This is especially helpful if a freebsd-update patch touched
    one but not the other.

    The bug fixed is that roots updated that are not located at '/', e.g.,
    by using -b or -j, will no longer create boot environments
    automatically.  There's a very low chance these will actually change the
    BE in any meaningful way, anyways.  It could make sense in the future
    to allow an argument-override to create the BE anyways if someone comes
    up with a non-standard setup, e.g., where a jail is an important part of
    their boot environment on an appliance or some such setup.

    Half of this patch is submitted by delphij@, the other half kevans@.

    PR:             261446

    (cherry picked from commit e01e8f911b935eabcc35b4d121951e4e21042ee5)

 usr.sbin/freebsd-update/freebsd-update.sh | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
Comment 9 commit-hook freebsd_committer freebsd_triage 2022-03-15 18:16:35 UTC
A commit in branch releng/12.3 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=f0df8a13116dca6ac42334d23430160fbeaf6c09

commit f0df8a13116dca6ac42334d23430160fbeaf6c09
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2022-02-12 21:36:24 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2022-03-15 17:46:50 +0000

    freebsd-update: improve BE creation feature

    This addresses one nit and one bug in the BE creation feature of
    freebsd-update:

    The nit addressed is that it currently only names the BEs after the
    userland version, but the kernel version may be higher.  After this
    change, we request both and pass them through sort(1) to choose the
    highest.  This is especially helpful if a freebsd-update patch touched
    one but not the other.

    The bug fixed is that roots updated that are not located at '/', e.g.,
    by using -b or -j, will no longer create boot environments
    automatically.  There's a very low chance these will actually change the
    BE in any meaningful way, anyways.  It could make sense in the future
    to allow an argument-override to create the BE anyways if someone comes
    up with a non-standard setup, e.g., where a jail is an important part of
    their boot environment on an appliance or some such setup.

    Half of this patch is submitted by delphij@, the other half kevans@.

    PR:             261446

    (cherry picked from commit e01e8f911b935eabcc35b4d121951e4e21042ee5)
    (cherry picked from commit 35d33d408213d20c63d60c0dfcdf77b2b36f5eee)

    Approved by:    so
    Security:       FreeBSD-EN-22:09.freebsd-update

 usr.sbin/freebsd-update/freebsd-update.sh | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)