feat(scripts): add linux deb & rpm package support to install script #1219
Conversation
| checkSystemdConfigNotCreated, | ||
| checkDeprecatedSystemdConfigNotCreated, | ||
| } | ||
|
|
||
| for _, spec := range []testSpec{ |
There was a problem hiding this comment.
Is there a simple way to avoid all this duplication? For example, take the testspec slice, run all the tests as they are before this change, and then run them again manually enabling native packaging?
There was a problem hiding this comment.
The main issue is that the PreActions, PreChecks, PostChecks, etc. differ between binary & package installs in most if not all of the specs. Reducing duplication may not be worth the time investment as we're planning on removing binary installation support from the install script.
There was a problem hiding this comment.
Intuitively, the install script should give the same results whether using native packaging or not, at least with systemd enabled. What is the source of the differences?
There was a problem hiding this comment.
The majority of the difference is in the pre-actions & pre-checks.
I avoided using much of the config mocking used in the tests to prevent package installation conflicts around existing config files. This isn't a problem when installing under an interactive tty as the installer can prompt how to handle conflicts. To work around this issue I wrote a helper to install a previous version of the package (defined by the const previousVersion) in some of the preActions of specs. This should work with binary installations but versions differ between binaries and packages to allow for out-of-band changes to packaging code.
The binary install uses the wrong path for the systemd unit file which has been fixed in the packages. I had assumed, perhaps incorrectly, that we wouldn't want to change paths of any files for binary installations. We can change the path to match the packages. We'll need logic to handle the removal or migration of the unit file at the old path.
There was a problem hiding this comment.
Allright, I'm fine with that. Can we then move the native test cases to a separate test? There's a lot of them and I'd like to introduce some basic organization here, to make changes easier in the future.
pkg/scripts_test/check.go
Outdated
| ownerUser = rootUser | ||
| ownerGroup = rootGroup |
There was a problem hiding this comment.
Why is this different? Do the native packages not set the same owner as the install script?
There was a problem hiding this comment.
When SYSTEMD_DISABLED=true, the install script doesn't add the otelcol-sumo user and thus the files are owned by root. The packages only support systemd and thus will always make use of the otelcol-sumo user.
The docker-compose config is a way to run both the tests against both deb & rpm packages on platforms that don't support it (e.g. macOS). It was quite helpful developing against it. I plan to add a job to test RPMs in CI as well. Systemd requires PID 1 which doesn't easily work with containers. The Python scripts are a hack to simulate systemd & journalctl for the purpose of testing. I agree that the Dockerfiles, config & Python scripts don't belong in this PR. I'll open a separate PR for that work and add some documentation. |
amdprophet
force-pushed
the
feature/linux-pkg-install-script
branch
from
ac0a916
to
763f1e2
Compare
github-actions
bot
added
the
github_actions
amdprophet
force-pushed
the
feature/linux-pkg-install-script
branch
2 times, most recently
from
6be0421
to
a829a6a
Compare
|
I'm currently refactoring the specs & checks to remove a lot of the duplication. In doing so I discovered that I broke FIPS installation, hostmetrics installation with Linux packages only, and a few other install paths. The ownership of config files differs between, not only binary and package installs but, non-systemd binary and systemd binary installs. This is why some of the specs check for |
|
@amdprophet once we're "all-in" with native packages, we can look to deprecate/remove non-service managed installations. |
amdprophet
force-pushed
the
feature/linux-pkg-install-script
branch
14 times, most recently
from
214564f
to
b958b0c
Compare
Signed-off-by: Justin Kolberg <[email protected]>
amdprophet
force-pushed
the
feature/linux-pkg-install-script
branch
from
b958b0c
to
6ad8440
Compare
github-actions
bot
added
the
documentation
|
@amdprophet What is the status of this change? |
|
@amdprophet can this be closed, since we are moving to the packaging repo? |
WIP: This is still a WIP but is nearly complete. There are a few test failures for deb packages that should be fixed shortly.
Adds support for Linux deb & rpm packages to the install script. A new
--use-native-packagingflag has been added to use package installation instead of the current binary installation method.SumoLogic/sumologic-otel-collector-packaging#28 contains fixes for packages that are required for the package installation via
install.shto function. A new package release is required after that PR is merged.