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 Debian Bookworm to integration-tests #902

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

dklimpel
Copy link
Contributor

@dklimpel dklimpel commented May 7, 2024

Checklist
  • make test-all (UNIX) passes. CI will also test this
  • unit and/or integration tests are included (if applicable)
  • documentation is changed or added (if applicable)

Description of change

The current used images / Dockerfiles are EoL. Starting add a supported Debian Bookworm image.

I did a few tests. Since Debian Jessy the check for run level does not work anymore.

bookworm:
  services:
    apache2: []

@dklimpel
Copy link
Contributor Author

dklimpel commented May 8, 2024

A few results for the ticket.

The new container image runs in podman without any problems. In docker the container terminates immediately.
I have the same result as the CI on my local machine.
-> edit: fixed with docker run ... --privileged

The old cent os image does not run on my local machine as well. It behaves in the same way as in the github pipeline (my tests). The container freezes without starting the services it contains (webserver/proxy).

The images all contain more or less systemd/init. This is not really an architecture or a use case for containers. Starting multiple processes could also be done with the help of e.g. supervisord.

The real reason why init/systemd is included is probably to check services via goss. This would certainly not be possible in standard containers.

@dklimpel dklimpel marked this pull request as ready for review May 10, 2024 13:58
@dklimpel dklimpel requested a review from aelsabbahy as a code owner May 10, 2024 13:58
@aelsabbahy
Copy link
Member

Ok, so I think we need to be careful with upgrading different Linux versions. The purpose of the integration tests is to test against real world examples of services/package managers that goss supports.

goss/system/system.go

Lines 162 to 179 in aed5633

func DetectService() string {
if HasCommand("systemctl") {
if isLegacySystemd() {
return "systemdlegacy"
}
return "systemd"
}
// Centos Docker container doesn't run systemd, so we detect it or use init.
switch DetectDistro() {
case "ubuntu":
return "upstart"
case "alpine":
return "alpineinit"
case "arch":
return "systemd"
}
return "init"
}

So if upgrading a distro will move it from say sysV init to systemd then the integration tests now have a gap in coverage. Also, it probably means they are potentially redundant (e.g. Debian may no longer be providing any value over ubuntu tests).

@dklimpel
Copy link
Contributor Author

I understand the arguments. And that is also correct.

On the other hand "test against real world examples" is not possible with the current versions that are EOL.

version EOL
centos7 30 Jun 2024
wheezy 31 May 2018
trusty 02 Apr 2019

@aelsabbahy
Copy link
Member

sure, I wonder if that part of the codebase should be deprecated or if I need to see if any non-EOL linux distributions still use sysV init.

@aelsabbahy
Copy link
Member

sure, I wonder if that part of the codebase should be deprecated or if I need to see if any non-EOL linux distributions still use sysV init.

Hmm.. doesn't seem like sysv init is around much anymore.

Let me think through the best approach for CI, it may need to be rethought a bit. Currently the docker tests allow for testing different package managers and init styles. Perhaps it's time to start deprecating support for some of them and removing some of the tests.

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.

2 participants