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

Add RPM install/upgrade scriptlet examples to documentation #356

Closed
3 tasks done
alephnull opened this issue Jul 20, 2021 · 16 comments
Closed
3 tasks done

Add RPM install/upgrade scriptlet examples to documentation #356

alephnull opened this issue Jul 20, 2021 · 16 comments
Assignees

Comments

@alephnull
Copy link

alephnull commented Jul 20, 2021

What happened?

When upgrading rpm packages, the scripts generated by nfpm do not do the right thing for upgrades. For instance, look at the pre-install script generated by nfpm, (using rpm -qp --scripts <rpmfile>) it looks like:

preinstall scriptlet (using /bin/sh):
#!/bin/bash
echo "Creating user and group..."
GROUPNAME="tyk"
USERNAME="tyk"

getent group "$GROUPNAME" >/dev/null || groupadd -r "$GROUPNAME"
getent passwd "$USERNAME" >/dev/null || useradd -r -g "$GROUPNAME" -M -s /sbin/nologin -c "Tyk service user" "$USERNAME"

While the package generated by fpm looks like:

preinstall scriptlet (using /bin/sh):
upgrade() {
    :
#!/bin/bash
echo "Removing init scripts..."

SYSTEMD="/lib/systemd/system"
UPSTART="/etc/init"
SYSV1="/etc/init.d"
SYSV2="/etc/rc.d/init.d/"
DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

if [ -f "/lib/systemd/system/tyk-dashboard.service" ]; then
	echo "Found Systemd"
	echo "Stopping the service"
	systemctl stop tyk-dashboard.service
	echo "Removing the service"
	rm /lib/systemd/system/tyk-dashboard.service
	systemctl --system daemon-reload
fi

if [ -f "/etc/init/tyk-dashboard.conf" ]; then
	echo "Found upstart"
	echo "Stopping the service"
	stop tyk-dashboard
	echo "Removing the service"
	rm /etc/init/tyk-dashboard.conf
fi

if [ -f "/etc/init.d/tyk-dashboard" ]; then
	echo "Found Sysv1"
	/etc/init.d/tyk-dashboard stop
	rm /etc/init.d/tyk-dashboard
fi

if [ -f "/etc/rc.d/init.d/tyk-dashboard" ]; then
	echo "Found Sysv2"
	echo "Stopping the service"
	/etc/rc.d/init.d/tyk-dashboard stop
	echo "Removing the service"
	rm /etc/rc.d/init.d/tyk-dashboard
fi

}
_install() {
    :
#!/bin/bash
echo "Creating user and group..."
GROUPNAME="tyk"
USERNAME="tyk"

getent group "$GROUPNAME" >/dev/null || groupadd -r "$GROUPNAME"
getent passwd "$USERNAME" >/dev/null || useradd -r -g "$GROUPNAME" -M -s /sbin/nologin -c "Tyk service user" "$USERNAME"

}
if [ "${1}" -eq 1 ]
then
    # "before install" goes here
    _install
elif [ "${1}" -gt 1 ]
then
    # "before upgrade" goes here
    upgrade
fi

The fpm version does the right thing imho.

How can we reproduce this?

Using https://packagecloud.io/tyk/tyk-gateway-unstable,

# yum install tyk-gateway-3.0.5~rc2
# yum install tyk-gateway-3.0.7~rc23

You will see that the systemd service file is removed at the end of the upgrade as it is in the postuninstall trigger of the old package.

As per the spec, the postuninstall trigger of the old package runs after the postinstall trigger of the new package. This is a little confusing but the fpm generated trigger scripts use $1 to figure out the right situation.

I can't be the first to encounter this problem but I'm not sure if I am doing something wrong.

You can see the goreleaser config at https://github.com/TykTechnologies/tyk/blob/master/.goreleaser.yml#L263-L266

goreleaser version

goreleaser version 0.155.0

GoReleaser Check

  • goreleaser check shows no errors

Search

  • I did search for other open and closed issues before opening this.

Code of Conduct

  • I agree to follow this project's Code of Conduct

Additional context

I have also checked the scripts with GoReleaser version found: v0.169.0 and the scripts are the generated without reference to $1.

We also have a goreleaser license from gumroad, if that makes a difference.

@alephnull
Copy link
Author

For the record, the cp and rm are for systemd service files and it appears that we need to be using some helper macros to configure them.

Is there a way to supply a rpm spec file in the nfpm config?

@caarlos0
Copy link
Member

sorry, your issue is a bit confusing... both examples you said are generated by nfpm, and then "The nfpm version does the right thing imho.", which one?

also, nfpm/goreleaser do not generate any of those scripts... so this is even more confusing for me...

Is there a way to supply a rpm spec file in the nfpm config?

not possible...

@alephnull
Copy link
Author

I've corrected the nfpm/fpm typo in the original comment.

In essence, the fpm generated spec for the preinstall trigger example that I have cited originally,

  • the supplied postremove script is wrapped up in a function called upgrade()
  • the supplied preinstall script is wrapped up in a function called _install()
    And the preinstall script is actually:
if [ "${1}" -eq 1 ]
then
    # "before install" goes here
    _install
elif [ "${1}" -gt 1 ]
then
    # "before upgrade" goes here
    upgrade
fi

Quoting from the docs,

When scriptlets are called, they will be supplied with an argument. This argument, accessed via $1 (for shell scripts) is the number of packages of this name which will be left on the system when the action completes, except for %pretrans and %posttrans which are always run with $1 as 0.

You are correct when you say that the contents of the postremove and preinstall scripts are supplied by me but the wrapping up and the conditional based on $1 was being done by fpm. This way I could use the same scripts to handle both Debian and RPM based distros.

I hope I have done a better job explaining the scenario.

@caarlos0
Copy link
Member

yes, understood it now. Thanks for explaining.

That said, IMHO this should be done by the caller, not by nfpm, otherwise we can't never change the scripts we generate without breaking things...

in any case, since this is more nfpm-specific, will move the issue there and see if anyone has a different opinion.

@caarlos0 caarlos0 transferred this issue from goreleaser/goreleaser Jul 20, 2021
@caarlos0 caarlos0 added the bug Something isn't working label Jul 20, 2021
@caarlos0
Copy link
Member

cc/ @erikgeiser @djgilcrease thoughts?

@alephnull
Copy link
Author

Hey @caarlos0 , I'm not sure what is intended by

That said, IMHO this should be done by the caller, not by nfpm

You mean goreleaser should do the wrapping and pass to nfpm? Or this could be configured in goreleaser.yml somehow?

If so, would you please post a link?

I think perhaps a goreleaser stanza that takes a debian/ dir or rpm spec and will build packages using dpkg-buildpackage or rpmbuild might be needed. Any non trivial packaging will want features provided by native tools that need not be duplicated in nfpm. My 2¢.

@caarlos0
Copy link
Member

I think perhaps a goreleaser stanza that takes a debian/ dir or rpm spec and will build packages using dpkg-buildpackage or rpmbuild might be needed. Any non trivial packaging will want features provided by native tools that need not be duplicated in nfpm. My 2¢.

the while reason we created nfpm is to avoid depending on rpmbuild and dpkg-buildpackage...

You mean goreleaser should do the wrapping and pass to nfpm? Or this could be configured in goreleaser.yml somehow?

I mean nor nfpm/goreleaser should not create any scripts for you, maybe we should have documentation examples for those cases though.

@erikgeiser
Copy link
Member

I think we should make a clear distinction between what nfpm can do or how it can be changed and how goreleaser can be changed in the way it interfaces with nfpm. As this issue is opened here in nfpm, we should look at it with the perspective of a user that directly uses npfm without goreleaser in mind.

Now, I don't understand how this is a bug. The rpm format supports pre/postinstall pre/postuninstall and pre/posttrans. The environment in which the scripts are executed provides the means to distinguish between an installation and an upgrade. In nfpm, we expose this feature of the rpm format directly. On the other side, fpm chooses to split the scripts up further and glues them together internally. For my taste this is way too much magic, especially the init-system detection. If I understand your issue correctly, you would prefer to nfpm to provide the same "magic" that fpm provides, right? If I understand this correctly, we should label this issue as a feature request rather than a bug.

I understand your point and I think many run into similar issues. I would propose to add documentation with examples for rpm scripts similar to the ones fpm uses. Users can then use this as a starting point to create a suitable script that fits their purpose.

@caarlos0
Copy link
Member

for clarification, the issue was opened on goreleaser and I moved it here.

I think it was opened as a bug because @alephnull expected the behavior to be the same as fpm, but, nfpm is not fpm... so yeah, not a bug.

also: even as feature request, I don't think this should be added. as @erikgeiser mentioned: too much magic.

@caarlos0 caarlos0 removed the bug Something isn't working label Jul 20, 2021
@erikgeiser
Copy link
Member

Yes, but to be clear, I understand the problem and I this may be a problem that other users will also run into. So while we won't add this as a feature to nfpm, we should definitely do something about it in the docs before closing the issue.

@erikgeiser erikgeiser changed the title [Bug]: rpm scriptlets Add RPM install/upgrade scriptlet examples to documentation Jul 20, 2021
@alephnull
Copy link
Author

FTR, I am not expecting magic nor compatibility with fpm. I just want upgrades to work. Consider the following,

  • nfpm is the only way to build deb and rpm packages with goreleaser
  • running the postuninstall trigger of the old package after the postinstall trigger of the new package is established rpm behaviour, scriptlets are expected to use the supplied $1 argument to distinguish between the cases
  • nfpm does not support the systemd helper macros
  • thus if the package wants to manage a systemd service, it has to do so in scriptlets for rpm

These facts have led to me shipping a package that will break things when customers upgrade it.

reason we created nfpm is to avoid depending on rpmbuild and dpkg-buildpackage...

These tools have real world relevance that has been built over decades. Ignoring the accumulated experience of rpmbuild and dpkg-buildpackage for whatever reason will come back to bite with the monorepo work in the enterprise version of goreleaser.

I agree that this need not be "fixed" in nfpm. Would you be ok with an issue in goreleaser calling for an alternate to nfpm for rpm and deb packages?

@caarlos0
Copy link
Member

In theory, what you can do with rpmbuild, you should be able to do with nfpm a well - with the only exception being the BuildRequires macros et al.

Maybe it will need some more work (e.g. different scripts for deb and rpm), but it should work, e.g.:

overrides:
  rpm:
    scripts:
      postinstall: ./rpm-postinstall.sh

rpmbuild also does not do anything magical

These tools have real world relevance that has been built over decades. Ignoring the accumulated experience of rpmbuild and dpkg-buildpackage for whatever reason will come back to bite with the monorepo work in the enterprise version of goreleaser.

problem is that different versions of rpmbuild have different behaviors, and then we have to add a lot of if rpmbuildversion == etc guards all over the place.

we are not ignoring the knowledge there, and are more often that not inspecting how they work to mimic in nfpm.

I agree that this need not be "fixed" in nfpm. Would you be ok with an issue in goreleaser calling for an alternate to nfpm for rpm and deb packages?

for the reasons explained above, it's more likely we'll eventually add a "custom packager", like we have the "custom publisher" stuff. Then users can run whatever (e.g. rombuild) and we handle from there.

@djgilcrease
Copy link
Contributor

djgilcrease commented Jul 21, 2021

The tips, hints and other useful information kind of covers this, but not the part about not auto generating scripts

https://nfpm.goreleaser.com/tips/

@erikgeiser
Copy link
Member

@djgilcrease nice, I wasn't aware of that but it looks like it already covers everything related to this issue. So, I guess we can close this issue then, right?

@caarlos0
Copy link
Member

indeed, looks so.

thanks everyone 🙏

@github-actions
Copy link
Contributor

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants