search

cpanel / elevate

elevate your cPanel&WHM CentOS 7 server to Almalinux 8
https://cpanel.github.io/elevate/
BSD 2-Clause "Simplified" License
45 stars 29 forks source link

Have bail_out_on_inappropriate_distro() remove Elevate::OS cache #397

Closed cPholloway closed 5 months ago

cPholloway commented 5 months ago

Case RE-249: Elevate did not detect the correct OS on a server that had been elevated and still had the staging file in place. This was due to the cached OS value still being present in the staging file. Due to this, elevate did not bail out as expected on an OS that was not eligible to be elevated.

Changelog: Ensure proper OS detection on servers that have been previously elevated

By submitting pull requests to this repo, I agree to the Contributor License Agreement which can be found at: https://github.com/cpanel/elevate/blob/main/docs/cPanel-CLA.pdf

toddr commented 5 months ago

Is the mistake that we shouldn’t cache it until leapp runs?

cPholloway commented 5 months ago

Is the mistake that we shouldn’t cache it until leapp runs?

No, there are two mistakes here:

  1. There is the general assumption that customers will not use this script again once a server has been elevated. From what I can tell, we make that assumption throughout the script. I suspect that this bug is the tip of the iceberg that we will see regarding the tech debt from that, but it is not something that we necessarily address at the moment on the whole.
  2. I was allowing the cache to be used in Elevate::Blockers::Distros::bail_out_on_inappropriate_distro(). Since that sub is our primary gateway to determine if an OS is eligible to be upgraded, we should never allow a cache to be used there.

I have now reworked this PR to add Elevate::OS::clear_cache() and have bail_out_on_inappropriate_distro() use that instead so that it does not need intimate knowledge of how Elevate::OS works.