Skip to content
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

(PA-5786) Stop using posttrans for post-install steps #819

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tvpartytonight
Copy link
Contributor

Before this commit, vanagon used the %posttrans section of an rpm spec for #add_postinstall_action. Specifying items in the %posttrans section means that rpm considers the transaction done, and rpm continues to excecute irregardless of the result of the %posttrans scriptlet. This behavior breaks puppet-agent 8 fips upgrades because certain configuration files are created in a #add_postinstall_action, and those configuration files were not yet ready before the agent daemon is killed and then started as part of the upgrade.

This change eliminates the %posttrans section entirely in favor of moving all #add_postinstall_action code to the %post section.

Please add all notable changes to the "Unreleased" section of the CHANGELOG.

Before this commit, vanagon used the `%posttrans` section of an
rpm spec for #add_postinstall_action. Specifying items in the
`%posttrans` section means that rpm considers the transaction
done, and rpm continues to excecute irregardless of the result
of the `%posttrans` scriptlet. This behavior breaks puppet-agent
8 fips upgrades because certain configuration files are created
in a #add_postinstall_action, and those configuration files were
not yet ready before the agent daemon is killed and then started
as part of the upgrade.

This change eliminates the `%posttrans` section entirely in favor
of moving all #add_postinstall_action code to the `%post` section.
@tvpartytonight tvpartytonight requested review from a team as code owners October 18, 2023 22:29
@tvpartytonight
Copy link
Contributor Author

I was able to build a puppet-agent package with this modified version of vanagon and I was able to upgrade redhatfips-8_x86_64 without any problems loading SSL.

Copy link
Contributor

@e-gris e-gris left a comment

Choose a reason for hiding this comment

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

Overall, I think this is right by removing unnecessary hand-waving. However, I was curious why we added %posttrans in the first place:

commit 9a8930c8f62da177d7ca32320853e232b8e4e8ca
Author: Melissa Stone <[email protected]>
Date:   Thu Oct 22 11:22:27 2015 -0700

(CPR-234) Teach Vanagon how to add packaging scriptlets to rpm packaging

Prior to this commit, rpm packaging only new how to handle pre and post
install scripts, and had no ability to handle special cases, like if we
wanted to only run a script on install and not upgrade. This commit adds
the ability to handle pre and post removal scripts, and adds the ability
to handle special instructions, even in the %posttrans section.

Unfortunately, I think we've lost CPR-234 to the Jira migration, but I will spend a little time to see if its archived somewhere. I'm curious what prompted it.

Further, I had a look at the corresponding deb commit: 10b157a just in case some parallel action was needed. From my reading, it isn't.

Also: please add an entry in CHANGELOG.md

@e-gris
Copy link
Contributor

e-gris commented Oct 19, 2023

Oh, cool, we do have the original ticket: https://tickets-archive.ops.puppetlabs.net/browse/CPR-234

My summary: "we need this for razor" so, I'm doubling my approval here.

Copy link
Contributor

@joshcooper joshcooper left a comment

Choose a reason for hiding this comment

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

There are some other vanagon projects where add_postinstall_action is called: https://github.com/search?q=org%3Apuppetlabs+add_postinstall_action+Language%3ARuby&type=code

@nmburgan thoughts about whether this will break ace-vanagon, pe-bolt-vanagon, puppet-enterprise-vanagon, etc?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants