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

Fix CI on CentOS #114

Merged
merged 1 commit into from
Jul 3, 2021
Merged

Fix CI on CentOS #114

merged 1 commit into from
Jul 3, 2021

Conversation

smortex
Copy link
Member

@smortex smortex commented Jun 30, 2021

Pull Request (PR) description

We are experiencing CI failures on CentOS because the service cannot start because it cannot create it's PID file.

Running the very same test locally works as expected, so the root cause of the different behavior on the CI is unfortunately unknown. Yet, ensuring the directory for the PID file exist seems to fix the CI issue.

This Pull Request (PR) fixes the following issues

n/a

@smortex smortex added the invalid This doesn't seem right label Jun 30, 2021
@smortex
Copy link
Member Author

smortex commented Jun 30, 2021

Cannot reproduce locally with:

BEAKER_PUPPET_COLLECTION=puppet6 BEAKER_setfile='centos7-64{hostname=centos7-64.example.com,image=centos:7.6.1810}' bundle exec rake beaker

@smortex smortex force-pushed the ci branch 3 times, most recently from 58b1bf5 to 5b54b5f Compare July 1, 2021 02:40
@smortex smortex changed the title DO NOT MERGE Explicitly manage /var/run/chrony Jul 1, 2021
@smortex smortex removed the invalid This doesn't seem right label Jul 1, 2021
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

An alternative is to only do this in CI. https://github.com/voxpupuli/voxpupuli-acceptance/#per-host-configuration describes that you can create spec/setup_acceptance_node.pp. Perhaps that's a better place for a workaround?

Also, it only appears to show up on CentOS 7 (at least now). Perhaps scope it to that while you're at it?

@oranenj
Copy link

oranenj commented Jul 2, 2021

I don't think this is at all correct; I just looked at a CentOS 7 system using chrony, and /var/run is a symlink to /run and its contents get lost on boot, so managing files in there is not correct. Something else seems to be the problem.

@oranenj
Copy link

oranenj commented Jul 2, 2021

chrony seems to create the directory by itself when it's started; it starts as root, so unless the test container is restricting writes to /run somehow I don't see why it would fail.

@alexjfisher
Copy link
Member

I don't think this is at all correct; I just looked at a CentOS 7 system using chrony, and /var/run is a symlink to /run and its contents get lost on boot, so managing files in there is not correct. Something else seems to be the problem.

indeed, it's tmpfs. I think @ekohl 's suggestion is the best to only apply the workaround in CI.

@smortex
Copy link
Member Author

smortex commented Jul 2, 2021

💯

I added commits to test this and stumbled on voxpupuli/voxpupuli-acceptance#27. When fixed, I can remove the last commit and merge all other commits in a single one.

@smortex smortex changed the title Explicitly manage /var/run/chrony Fix CI on CentOS Jul 2, 2021
@alexjfisher
Copy link
Member

We are experiencing CI failures on CentOS because the service cannot
start because it cannot create it's PID file.

Running the very same test locally works as expected, so the root cause
of the different behavior on the CI is unfortunately unknown.  Yet,
ensuring the directory for the PID file exist seems to fix the CI issue.

So manage explicitely this directory when running on CentOS in CI.
@bastelfreak bastelfreak added the bug Something isn't working label Jul 3, 2021
@smortex smortex merged commit c0a3dba into master Jul 3, 2021
@smortex smortex deleted the ci branch July 3, 2021 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants