Skip to content
Open
26 changes: 26 additions & 0 deletions .chloggen/rpm_dont_start_service.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: "breaking"

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: "packaging"

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Prevent service being started/enabled upon install for rpms"
Copy link
Contributor

Choose a reason for hiding this comment

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

this changelog doesn't reflect the changes. You seem to be checking for systemd presence in the system.
Please clarify this changelog.

Copy link
Author

Choose a reason for hiding this comment

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

I believe this is an accurate description of the change in behaviour for end users - please also refer to #1334 for more expansion on the motivation here.

All of the logic in the scripts that I am adding already exists in the base postinstall.sh scripts - my change merely creates specific versions with slightly different outcomes for rpms.

For example, for the plain otelcol distribution, the diff between the existing script and my rpm specific override:

--- a/distributions/otelcol/postinstall.sh
+++ b/distributions/otelcol/postinstall-rpm.sh
@@ -7,10 +7,9 @@ if command -v systemctl >/dev/null 2>&1; then
     if [ -d /run/systemd/system ]; then
         systemctl daemon-reload
     fi
-    systemctl enable otelcol.service
     if [ -f /etc/otelcol/config.yaml ]; then
         if [ -d /run/systemd/system ]; then
-            systemctl restart otelcol.service
+            systemctl try-restart otelcol.service
         fi
     fi
 fi

If you think there's a better approach then I'm happy to adjust.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK I see. There is boilerplate on that script you can strip away imo ; remove the check for the config file presence. Is checking for /run/systemd/system presence even necessary?

Copy link
Author

Choose a reason for hiding this comment

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

I agree, have removed.


# One or more tracking issues or pull requests related to the change
issues: [1334]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: "This brings behaviour inline with usual expectations for rpm packages which usually leave this up to the system administrator."

# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []

3 changes: 3 additions & 0 deletions cmd/goreleaser/internal/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,9 @@ func (b *distributionBuilder) newNfpms(dist string) []config.NFPM {
Overrides: map[string]config.NFPMOverridables{
"rpm": {
Dependencies: []string{"/bin/sh"},
Scripts: config.NFPMScripts{
PostInstall: "postinstall-rpm.sh",
},
},
},
NFPMOverridables: config.NFPMOverridables{
Expand Down
5 changes: 5 additions & 0 deletions cmd/goreleaser/internal/distro_opamp.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ var (
content.Source = path.Join("cmd", d.Name, content.Source)
d.Nfpms[0].Contents[i] = content
}

rpm_overrides := d.Nfpms[0].Overrides["rpm"]
rpm_overrides.Scripts.PostInstall = path.Join("cmd", d.Name, d.Nfpms[0].Overrides["rpm"].Scripts.PostInstall)
d.Nfpms[0].Overrides["rpm"] = rpm_overrides

d.Nfpms[0].Scripts.PreInstall = path.Join("cmd", d.Name, d.Nfpms[0].Scripts.PreInstall)
d.Nfpms[0].Scripts.PostInstall = path.Join("cmd", d.Name, d.Nfpms[0].Scripts.PostInstall)
d.Nfpms[0].Scripts.PreRemove = path.Join("cmd", d.Name, d.Nfpms[0].Scripts.PreRemove)
Expand Down
2 changes: 2 additions & 0 deletions cmd/opampsupervisor/.goreleaser.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ nfpms:
rpm:
dependencies:
- /bin/sh
scripts:
postinstall: cmd/opampsupervisor/postinstall-rpm.sh
id: opampsupervisor
ids:
- opampsupervisor-linux
Expand Down
15 changes: 15 additions & 0 deletions cmd/opampsupervisor/postinstall-rpm.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/bin/sh

# Copyright The OpenTelemetry Authors
# SPDX-License-Identifier: Apache-2.0

if command -v systemctl >/dev/null 2>&1; then
if [ -d /run/systemd/system ]; then
systemctl daemon-reload
fi
if [ -f /etc/opampsupervisor/config.yaml ]; then
if [ -d /run/systemd/system ]; then
systemctl try-restart opampsupervisor.service
fi
fi
fi
2 changes: 2 additions & 0 deletions distributions/otelcol-contrib/.goreleaser.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ nfpms:
rpm:
dependencies:
- /bin/sh
scripts:
postinstall: postinstall-rpm.sh
id: otelcol-contrib
ids:
- otelcol-contrib-linux
Expand Down
15 changes: 15 additions & 0 deletions distributions/otelcol-contrib/postinstall-rpm.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/bin/sh

# Copyright The OpenTelemetry Authors
# SPDX-License-Identifier: Apache-2.0

if command -v systemctl >/dev/null 2>&1; then
if [ -d /run/systemd/system ]; then
systemctl daemon-reload
fi
if [ -f /etc/otelcol-contrib/config.yaml ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you checking for this file presence?

Copy link
Author

Choose a reason for hiding this comment

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

Like the others, this was present in the original file that my rpm-specific version is derived from. However in this case as we're not attempting to start the service first time I agree that it's entirely useless, so I have removed it.

if [ -d /run/systemd/system ]; then
systemctl try-restart otelcol-contrib.service
fi
fi
fi
2 changes: 2 additions & 0 deletions distributions/otelcol-otlp/.goreleaser.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ nfpms:
rpm:
dependencies:
- /bin/sh
scripts:
postinstall: postinstall-rpm.sh
id: otelcol-otlp
ids:
- otelcol-otlp-linux
Expand Down
17 changes: 17 additions & 0 deletions distributions/otelcol-otlp/postinstall-rpm.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#!/bin/sh

# Copyright The OpenTelemetry Authors
# SPDX-License-Identifier: Apache-2.0

if command -v systemctl >/dev/null 2>&1; then
if [ -d /run/systemd/system ]; then
systemctl daemon-reload
fi
if [ -f /etc/otelcol-otlp/config.yaml ]; then
if [ -d /run/systemd/system ]; then
systemctl try-restart otelcol-otlp.service
fi
else
echo "Make sure to configure otelcol-otlp by creating /etc/otelcol-otlp/config.yaml"
fi
fi
2 changes: 2 additions & 0 deletions distributions/otelcol/.goreleaser.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ nfpms:
rpm:
dependencies:
- /bin/sh
scripts:
postinstall: postinstall-rpm.sh
id: otelcol
ids:
- otelcol-linux
Expand Down
15 changes: 15 additions & 0 deletions distributions/otelcol/postinstall-rpm.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/bin/sh

# Copyright The OpenTelemetry Authors
# SPDX-License-Identifier: Apache-2.0

if command -v systemctl >/dev/null 2>&1; then
if [ -d /run/systemd/system ]; then
systemctl daemon-reload
fi
if [ -f /etc/otelcol/config.yaml ]; then
if [ -d /run/systemd/system ]; then
systemctl try-restart otelcol.service
fi
fi
fi
17 changes: 17 additions & 0 deletions scripts/package-tests/package-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,23 @@ podman run --name "$container_name" --privileged -v /sys/fs/cgroup:/sys/fs/cgrou

install_pkg "$container_name" "$PKG_PATH"

if [[ "$pkg_type" == "rpm" ]]; then
echo "Checking $SERVICE_NAME service state after install ..."
if $container_exec systemctl is-active "$SERVICE_NAME"; then
echo "$SERVICE_NAME service running after rpm install" >&2
exit 1
fi
echo "$SERVICE_NAME service correctly not running after rpm install"

if $container_exec systemctl is-enabled "$SERVICE_NAME"; then
echo "$SERVICE_NAME service enabled after rpm install" >&2
exit 1
fi
echo "$SERVICE_NAME service correctly not enabled after rpm install"

$container_exec systemctl start "$SERVICE_NAME"
fi

# If we got to this point, we might need to check the logs of the systemd service
# when it's not properly active. This is added as a trap because the check
# for service status below will return an error exitcode if the service is
Expand Down