Skip to content

Add Boot Environment [Snapshots] Integration - #7749

Merged
AdSchellevis merged 17 commits into
masterfrom
be
Aug 13, 2024
Merged

AdSchellevis merged 17 commits into
masterfrom
be

Conversation

@AdSchellevis

Copy link
Copy Markdown
Member

This PR supersedes #7650 and implements the new Snapshot feature.

As @sheridans nicely noted in the original PR:

This integration provides an intuitive and seamless way for users to create, manage, and switch between boot environments, enhancing system management and recovery options.

Notable changes on top of the initial PR:

  • Cleaned up codebase
  • Moved to Core package for consistency (firmware and backups live there too)
  • Renamed the menu item to Snapshots in an attempt to explain the feature to non-fbsd users

TODO:

[ ] Documentation

sheridans and others added 9 commits August 8, 2024 20:06
This pull request introduces a new feature to the OPNsense web interface, allowing users to manage FreeBSD boot environments directly within OPNsense. This integration provides an intuitive and seamless way for users to create, manage, and switch between boot environments, enhancing system management and recovery options.
…his into core at some point.

* remove model as it wasn't actually used
* simplified api actions and left some comments for work still in need to be done
* removed some unused code in the volt template

ref: #7650
* move "not supported" to endpoint so we can use it in the same template
* add some php doc to the endpoints
* basic validations for name validity and duplication
* cleanup the volt template to its bare minimum, which is more inline with how other components work at the moment

ref: #7650
* cleanup python scripts, use one simple wrapper around bectl while keeping output roughly the same (errors could be simplified, but may be a next step)
* make some columns sortable in the grid, including created time

ref: #7650
Final bits and pieces to bring bectl into OPNsense.
* Move BootEnvironments into Core package as this is likely a more logical place (and api endpoints look nicer, /api/core/boot_environment/)
* Rename the menu item to Snapshots, when possible we prefer single clause names and snapshots might need less explanation for non-fbsd users (eventually, we are creating snapshots here)
* Change zfs check message to a "alert alert-warning" dialog and change the text a bit
* Remove used includes [use] in php files
* Merge Menu and ACL with Core model, same as firmware

ref  #7650
@AdSchellevis AdSchellevis added the feature Adding new functionality label Aug 8, 2024
@AdSchellevis
AdSchellevis requested a review from fichtner August 8, 2024 18:13
@AdSchellevis AdSchellevis self-assigned this Aug 8, 2024
@AdSchellevis

Copy link
Copy Markdown
Member Author

@sheridans if you have time to test and review, that would be highly appreciated

@sheridans

Copy link
Copy Markdown
Contributor

@AdSchellevis thank you for the time and effort you put into this, in such a short time frame, I'll run some tests. Excited to see this introduced. Thank you for helping getting this over the line!

private function find($fieldname, $value)
{
if (empty($this->environments)) {
$this->environments = json_decode(trim((new Backend())->configdRun('bootenvironment list')), true) ?? [];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we do have a "zfs" file for this, rather not clutter?


$msg = null;
if ($this->findByName($name)) {
$msg = gettext('A boot environment already exists by this name');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we may want to consider to rename all the "boot environment" strings to "snapshot" too. Similar to "BE" which is highly ambiguous in our scope.

Comment on lines +656 to +659
<name>System: Boot Environments</name>
<patterns>
<pattern>ui/bootenvironments/*</pattern>
<pattern>api/bootenvironments/*</pattern>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same here to be honest

@@ -0,0 +1,41 @@
[list]
command:/usr/local/opnsense/scripts/bootenvironments/bectl.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would also prefer this in system directory.

@fichtner fichtner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we want to fix these things post merge? Up front would be cleaner I guess.

@AdSchellevis

Copy link
Copy Markdown
Member Author

@fichtner yes, it looks like I missed a spot there indeed.

* rename all "boot environments" to "snapshots" for consistency
* merge zfs configd command with zfs template
@AdSchellevis

Copy link
Copy Markdown
Member Author

@fichtner going to hand this over to you for a final check, this fb90fca should contain the last bits and pieces as discussed.

Comment thread src/opnsense/mvc/app/views/OPNsense/Core/snapshot.volt Outdated
Comment thread src/opnsense/mvc/app/controllers/OPNsense/Core/Api/SnapshotsController.php Outdated
Comment thread src/opnsense/mvc/app/controllers/OPNsense/Core/Api/SnapshotsController.php Outdated
Comment thread src/opnsense/mvc/app/models/OPNsense/Core/ACL/ACL.xml Outdated
Comment thread src/opnsense/mvc/app/models/OPNsense/Core/Menu/Menu.xml Outdated
Comment thread src/opnsense/mvc/app/models/OPNsense/Core/Menu/Menu.xml Outdated
Comment thread src/opnsense/mvc/app/views/OPNsense/Core/snapshot.volt Outdated

@fichtner fichtner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perfect, let's go!

@AdSchellevis
AdSchellevis merged commit 8322848 into master Aug 13, 2024
fichtner pushed a commit that referenced this pull request Aug 13, 2024
This pull request introduces a new feature to the OPNsense web interface,
allowing users to manage FreeBSD boot environments directly within OPNsense.

This integration provides an intuitive and seamless way for users to create,
manage, and switch between boot environments, enhancing system management
and recovery options.

Renamed the menu item to "Snapshots" in an attempt to explain the feature
to non-FreeBSD users.
@fichtner

Copy link
Copy Markdown
Member

@sheridans @AdSchellevis thank you both 🎉

@Vexz00

Vexz00 commented Aug 19, 2024

Copy link
Copy Markdown

An option to automatically create a new snapshot before OPNsense performs an update would be perfect. This way you could skip the manual creation of a snapshot before you update. Is this included (yet)?

@fichtner

Copy link
Copy Markdown
Member

Due to the fact that this quickly and sometimes inexplicably fills the disk it is not included at the moment.

@Vexz00

Vexz00 commented Aug 19, 2024

Copy link
Copy Markdown

Due to the fact that this quickly and sometimes inexplicably fills the disk it is not included at the moment.

Could be solved with an additional option like "keep the latest X snapshots".

@meyergru

meyergru commented Aug 19, 2024

Copy link
Copy Markdown
Contributor

No, it could not. A ZFS snapshot grows bigger with any subsequent change, such as logging. Thus, limiting the number of snapshots is essentially useless to limit the space being used. Even more, since the snapshots are built on top of each other, having many of them does not increase the space used all that much.

The only viable option I can see is a checkbox for the user to decide if he wishes a snapshot before the upgrade.

@sheridans

sheridans commented Aug 19, 2024

Copy link
Copy Markdown
Contributor

One option I was toying with is to take a snapshot mark it as previous, mark current as current (zfs properties) perform upgrade. On successful boot, remove the one marked as previous.

Purely for use in an auto recovery s scenario, should something go pear-shaped during the upgrade

On failure, current as failed, reboot into previous.

@doktornotor

doktornotor commented Aug 19, 2024

Copy link
Copy Markdown
Contributor

There apparently appears to be some severe misunderstanding of how this thing works.... The below is an example from XigmaNAS:

# bectl list
BE                        Active Mountpoint Space Created
upgrade-2023-10-05-160627 -      -          1.34G 2023-10-05 16:06
upgrade-2024-04-17-145606 -      -          1.06G 2024-04-17 14:56
upgrade-2024-06-30-114819 -      -          1.38G 2024-06-30 11:48
upgrade-2024-07-05-100351 NR     /          7.31G 2024-07-05 10:03

# zpool list zroot
NAME    SIZE  ALLOC   FREE  CKPOINT  EXPANDSZ   FRAG    CAP  DEDUP    HEALTH  ALTROOT
zroot  15.9G  7.38G  8.49G        -         -    21%    46%  1.00x    ONLINE  -

# bectl destroy upgrade-2023-10-05-160627
# bectl list
BE                        Active Mountpoint Space Created
upgrade-2024-04-17-145606 -      -          1.26G 2024-04-17 14:56
upgrade-2024-06-30-114819 -      -          1.38G 2024-06-30 11:48
upgrade-2024-07-05-100351 NR     /          5.98G 2024-07-05 10:03

# zpool list zroot
NAME    SIZE  ALLOC   FREE  CKPOINT  EXPANDSZ   FRAG    CAP  DEDUP    HEALTH  ALTROOT
zroot  15.9G  6.04G  9.83G        -         -    20%    38%  1.00x    ONLINE  -

@sheridans

Copy link
Copy Markdown
Contributor

I missing something? The only snapshots I proposed removed, are the last succesful, iff successful. Llatest would be marked bad if failed, allowing user to investigate or delete and retry.

Specifically I was thinking create snapshot, mount it, perform upgrade on mounted, activate reboot, if successful remove previous, if failed mark bad, reboot into previous allowing user to debug or delete and retry.

fichtner pushed a commit that referenced this pull request Aug 29, 2024
This pull request introduces a new feature to the OPNsense web interface,
allowing users to manage FreeBSD boot environments directly within OPNsense.

This integration provides an intuitive and seamless way for users to create,
manage, and switch between boot environments, enhancing system management
and recovery options.

Renamed the menu item to "Snapshots" in an attempt to explain the feature
to non-FreeBSD users.

(cherry picked from commit 7118a82)
(cherry picked from commit c3af228)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Adding new functionality

Development

Successfully merging this pull request may close these issues.

6 participants