Handle RPM upgrade in %postun script #14379

edwardsb · 2023-10-09T15:46:09Z

This pull request addresses a key aspect of the RPM upgrade process - handling of scripts during upgrades vice pure deletion events.

An RPM upgrade operation consists of both an Install and an Uninstall operation, meaning that during an upgrade, our %postun script is run and previously, it was causing the accidental deletion of binaries needed for the upgrade.

To prevent this unwanted removal during upgrade scenarios, the %postun script now checks for the execution scenario in which it finds itself.

Checklist for submitter

If some of the following don't apply, delete the relevant line.

Changes file added for user-visible changes in changes/ or orbit/changes/.
See Changes files for more information.
Manual QA for all new/changed functionality
- For Orbit and Fleet Desktop changes:
  - Manual QA must be performed in the three main OSs, macOS, Windows and Linux.

edwardsb · 2023-10-09T15:52:05Z

relates to #14380

codecov · 2023-10-09T16:13:10Z

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (ae669e1) 58.87% compared to head (916e2e5) 58.87%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #14379   +/-   ##
=======================================
  Coverage   58.87%   58.87%           
=======================================
  Files         925      925           
  Lines       77319    77333   +14     
  Branches     2222     2222           
=======================================
+ Hits        45521    45531   +10     
- Misses      28186    28190    +4     
  Partials     3612     3612

Flag	Coverage Δ
backend	`59.57% <0.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
orbit/pkg/packaging/linux_shared.go	`0.00% <0.00%> (ø)`

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mostlikelee · 2023-10-09T19:08:42Z

orbit/pkg/packaging/linux_shared.go

+if [ "$1" = 0 ]; then
+	rm -rf /var/lib/orbit /var/log/orbit /usr/local/bin/orbit /etc/default/orbit /usr/lib/systemd/system/orbit.service /opt/orbit
+fi


I'm not too familiar with this part of the code, can you point me to where this script is being called?

@mostlikelee hey this is used as a template file for the RPM/DEB package post run script.

We use https://github.com/goreleaser/nfpm to create RPM/DEB installers of fleetd so this is getting used during fleetctl package -type=rpm

This particular script is executed during the %postun phase of a RPM upgrade/uninstall.

The problem here is that when upgrading, I think the %postun phase is executed and will remove the binary files. This conditional checks to see if we are actually doing a real uninstall before removing the files.

thanks for that! do we need to change where this script is called to pass the new argument, or is that handled natively by the installer?

The system package manager is handling passing the args, yum, rpm, etc.

@mostlikelee I added some details in the issue #14380

lucasmrod · 2023-10-12T21:24:23Z

Have we checked that with deb packages this works the same way? (code is shared for rpm/deb)

lucasmrod · 2023-10-12T21:25:31Z

it was causing the accidental deletion of binaries needed for the upgrade.

What binaries were needed for the upgrade? The new package should have all binaries that it needs. Maybe I'm missing some scenario.

lucasmrod

Left a few questions

edwardsb · 2023-10-12T22:10:17Z

it was causing the accidental deletion of binaries needed for the upgrade.

What binaries were needed for the upgrade? The new package should have all binaries that it needs. Maybe I'm missing some scenario.

All of them. The script is run on upgrade and all of the binaries listed in the script are removed.

edwardsb · 2023-10-12T22:11:01Z

Have we checked that with deb packages this works the same way? (code is shared for rpm/deb)

I tested RPM but still need to test DEB.

lucasmrod · 2023-10-12T22:26:22Z

All of them. The script is run on upgrade and all of the binaries listed in the script are removed.

Gotcha, but why is that an issue?

lucasmrod · 2023-10-12T22:26:50Z

I tested RPM but still need to test DEB.

OK, let me know if you need a hand (I have an Ubuntu VM.)

edwardsb · 2023-10-13T04:57:39Z

All of them. The script is run on upgrade and all of the binaries listed in the script are removed.

Gotcha, but why is that an issue?

Because it runs as the final step of the upgrade, thus removing the files it just got finished upgrading. When the service attempts to start, the files don't exist.

lucasmrod · 2023-10-13T11:31:29Z

All of them. The script is run on upgrade and all of the binaries listed in the script are removed.

Gotcha, but why is that an issue?

Because it runs as the final step of the upgrade, thus removing the files it just got finished upgrading. When the service attempts to start, the files don't exist.

Oh! I missed the fact that this is the "post" part.

Thanks for the clarification.

ksatter · 2023-10-16T18:11:05Z

@edwardsb Do you need a hand getting this tested for deb?

edwardsb · 2023-10-16T18:54:34Z

@edwardsb Do you need a hand getting this tested for deb?

Yes please

lucasmrod

#14380 (comment)

…n during RPM upgrade. The check ensures that files and directories are only removed during a full uninstall ( equals 0), safeguarding necessary files from unintended deletion during an upgrade.

📝 docs(linux_shared.go): add comments to clarify the difference between RPM and Debian uninstallation conditions

edwardsb · 2023-10-27T04:10:49Z

@lucasmrod when you have a moment can you retest on debian?

lucasmrod · 2023-10-27T16:25:06Z

@lucasmrod when you have a moment can you retest on debian?

Works now! (Haven't tested with dnf)

I've tested the following on Ubuntu 22.04:

Installing and uninstalling a deb package via sudo dpkg --install fleet-osquery_1.17.0_amd64.deb and sudo apt remove fleet-osquery -y works as expected.
Installing sudo dpkg --install fleet-osquery_1.16.0_amd64.deb and then installing sudo dpkg --install fleet-osquery_1.17.0_amd64.deb works as expected.

On my CentOS 7:

Installing and uninstalling a rpm package via sudo rpm --install fleet-osquery-1.16.0.x86_64.rpm and sudo yum remove fleet-osquery works as expected.
Installing sudo rpm --install fleet-osquery-1.16.0.x86_64.rpm and then installing sudo dpkg --install fleet-osquery-1.17.0.x86_64.rpm doesn't work, probably not supported by rpm (same behavior on main so nothing to do here).

edwardsb had a problem deploying to Docker Hub October 9, 2023 15:46 — with GitHub Actions Error

edwardsb temporarily deployed to Docker Hub October 9, 2023 15:53 — with GitHub Actions Inactive

rfairburn previously approved these changes Oct 9, 2023

View reviewed changes

edwardsb marked this pull request as ready for review October 9, 2023 15:57

edwardsb requested a review from a team as a code owner October 9, 2023 15:57

mostlikelee reviewed Oct 9, 2023

View reviewed changes

mostlikelee previously approved these changes Oct 9, 2023

View reviewed changes

lucasmrod reviewed Oct 12, 2023

View reviewed changes

lucasmrod requested changes Oct 17, 2023

View reviewed changes

edwardsb dismissed stale reviews from mostlikelee and rfairburn via b946591 October 27, 2023 04:08

edwardsb force-pushed the edwardsb-rpm-upgrades branch from 91a3b13 to b946591 Compare October 27, 2023 04:08

edwardsb had a problem deploying to Docker Hub October 27, 2023 04:08 — with GitHub Actions Error

edwardsb requested a review from lucasmrod October 27, 2023 04:08

edwardsb added 3 commits October 27, 2023 00:08

Add a conditional check in the %postun script to prevent file deletio…

5c88732

…n during RPM upgrade. The check ensures that files and directories are only removed during a full uninstall ( equals 0), safeguarding necessary files from unintended deletion during an upgrade.

add changes file for orbit 14380

4590f86

🐛 fix(linux_shared.go): add condition to handle Debian package removal

916e2e5

📝 docs(linux_shared.go): add comments to clarify the difference between RPM and Debian uninstallation conditions

edwardsb force-pushed the edwardsb-rpm-upgrades branch from b946591 to 916e2e5 Compare October 27, 2023 04:09

edwardsb temporarily deployed to Docker Hub October 27, 2023 04:09 — with GitHub Actions Inactive

lucasmrod approved these changes Oct 27, 2023

View reviewed changes

edwardsb merged commit 71709e5 into main Oct 27, 2023
27 checks passed

edwardsb deleted the edwardsb-rpm-upgrades branch October 27, 2023 16:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle RPM upgrade in %postun script #14379

Handle RPM upgrade in %postun script #14379

edwardsb commented Oct 9, 2023 •

edited

Loading

edwardsb commented Oct 9, 2023

codecov bot commented Oct 9, 2023 •

edited

Loading

mostlikelee Oct 9, 2023

edwardsb Oct 9, 2023

mostlikelee Oct 9, 2023

edwardsb Oct 9, 2023

mostlikelee Oct 9, 2023

edwardsb Oct 9, 2023

lucasmrod commented Oct 12, 2023 •

edited

Loading

lucasmrod commented Oct 12, 2023

lucasmrod left a comment

edwardsb commented Oct 12, 2023

edwardsb commented Oct 12, 2023

lucasmrod commented Oct 12, 2023

lucasmrod commented Oct 12, 2023

edwardsb commented Oct 13, 2023

lucasmrod commented Oct 13, 2023

ksatter commented Oct 16, 2023

edwardsb commented Oct 16, 2023

lucasmrod left a comment

edwardsb commented Oct 27, 2023

lucasmrod commented Oct 27, 2023

Handle RPM upgrade in %postun script #14379

Handle RPM upgrade in %postun script #14379

Conversation

edwardsb commented Oct 9, 2023 • edited Loading

Checklist for submitter

edwardsb commented Oct 9, 2023

codecov bot commented Oct 9, 2023 • edited Loading

Codecov Report

mostlikelee Oct 9, 2023

Choose a reason for hiding this comment

edwardsb Oct 9, 2023

Choose a reason for hiding this comment

mostlikelee Oct 9, 2023

Choose a reason for hiding this comment

edwardsb Oct 9, 2023

Choose a reason for hiding this comment

mostlikelee Oct 9, 2023

Choose a reason for hiding this comment

edwardsb Oct 9, 2023

Choose a reason for hiding this comment

lucasmrod commented Oct 12, 2023 • edited Loading

lucasmrod commented Oct 12, 2023

lucasmrod left a comment

Choose a reason for hiding this comment

edwardsb commented Oct 12, 2023

edwardsb commented Oct 12, 2023

lucasmrod commented Oct 12, 2023

lucasmrod commented Oct 12, 2023

edwardsb commented Oct 13, 2023

lucasmrod commented Oct 13, 2023

ksatter commented Oct 16, 2023

edwardsb commented Oct 16, 2023

lucasmrod left a comment

Choose a reason for hiding this comment

edwardsb commented Oct 27, 2023

lucasmrod commented Oct 27, 2023

edwardsb commented Oct 9, 2023 •

edited

Loading

codecov bot commented Oct 9, 2023 •

edited

Loading

lucasmrod commented Oct 12, 2023 •

edited

Loading