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

CFE-3982: Random process killing #5362

Merged
merged 5 commits into from
Nov 8, 2023

Conversation

vpodzime
Copy link
Contributor

@vpodzime vpodzime commented Nov 7, 2023

The `cfengine3.service` has `Wants` on all our services which
ensures they are started when the `cfengine3.service` starts. If
an individual service is enabled with `systemctl enable`, it
should only be added to the respective systemd target in which it
should start.
@vpodzime
Copy link
Contributor Author

vpodzime commented Nov 7, 2023

@cf-bottom jenkins, please

@vpodzime vpodzime force-pushed the master-blind_shooting branch from a329134 to 26eb4af Compare November 7, 2023 15:13
@cfengine cfengine deleted a comment from cf-bottom Nov 7, 2023
@vpodzime vpodzime force-pushed the master-blind_shooting branch from 26eb4af to a04d5ba Compare November 7, 2023 15:21
@cfengine cfengine deleted a comment from cf-bottom Nov 7, 2023
@cf-bottom
Copy link

@nickanderson
Copy link
Member

@vpodzime We have systemd units mirrored in Masterfiles: cfengine/masterfiles#2760

@vpodzime
Copy link
Contributor Author

vpodzime commented Nov 7, 2023

@vpodzime We have systemd units mirrored in Masterfiles: cfengine/masterfiles#2760

Thanks!

larsewi
larsewi previously approved these changes Nov 7, 2023
Copy link
Contributor

@larsewi larsewi left a comment

Choose a reason for hiding this comment

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

Nice 🚀

libpromises/locks.c Outdated Show resolved Hide resolved
libpromises/locks.c Show resolved Hide resolved
libpromises/locks.c Outdated Show resolved Hide resolved
libpromises/locks.c Show resolved Hide resolved
olehermanse
olehermanse previously approved these changes Nov 7, 2023
Copy link
Member

@olehermanse olehermanse left a comment

Choose a reason for hiding this comment

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

Looks good to me, just some smaller comments, thanks for working on this 🚀

cf-check/dump.c Outdated Show resolved Hide resolved
libpromises/locks.c Outdated Show resolved Hide resolved
libpromises/locks.c Outdated Show resolved Hide resolved
libpromises/locks.c Outdated Show resolved Hide resolved
libpromises/locks.c Outdated Show resolved Hide resolved
libpromises/locks.c Outdated Show resolved Hide resolved
nickanderson added a commit to nickanderson/masterfiles that referenced this pull request Nov 7, 2023
WantedBy=cfengine3.service was removed from systemd service templates
for individual components. It was un-necessary as cfengine3.service already
wants the individual services.

cfengine/core#5362

Ticket: CFE-3982
Changelog: commit
vpodzime and others added 4 commits November 8, 2023 10:24
Instead of requiring that the file name ends with
e.g. "cf_lock.lmdb", just check if the file name contains the
string. This ensures that files like `cf_lock.lmdb.backup` match
as well. And if someone renames their `cf_lastseen.lmdb` to
`cf_lock.lmdb_cf_lastseen.lmdb` or something similar, it's not
our fault they get wrong output.
This function totally doesn't do what its name says. It only
checks if the DB was modified in the current boot and if not, it
restores it from the latest backup. Which is done and the end of
every agent run. So this function effectively reverts the locks
DB to the state of the last agent run on every boot dropping
significant data like daemon locks.

Ticket: CFE-3982
Changelog: cf_lock.lmdb is no longer restored from backup on
           every boot
It's no longer being used in locks.c, but it can potentially be
useful for being called explicitly. After all, it's a
complementary function to BackupLockDatabase() which is also
non-static.

Ticket: CFE-3982
Changelog: None
If a non CFEngine process has a matching PID and start time, we
shouldn't try to kill it because it's practically impossible that
it is a real holder of one of our locks in cf_lock.lmdb. Most
likely it's an unfortunate process that ended up with the
matching PID and start time after a reboot.

Ticket: CFE-3982
Changelog: Only CFEngine processes are now killed as expired lock owners

Co-authored-by: Benoît Peccatte <[email protected]>
@vpodzime vpodzime dismissed stale reviews from olehermanse and larsewi via 6234c8c November 8, 2023 09:24
@vpodzime vpodzime force-pushed the master-blind_shooting branch from a04d5ba to 6234c8c Compare November 8, 2023 09:24
@vpodzime
Copy link
Contributor Author

vpodzime commented Nov 8, 2023

@cf-bottom jenkins, please

@cf-bottom
Copy link

@vpodzime vpodzime merged commit e03a762 into cfengine:master Nov 8, 2023
12 checks passed
vpodzime pushed a commit to vpodzime/masterfiles that referenced this pull request Nov 8, 2023
WantedBy=cfengine3.service was removed from systemd service templates
for individual components. It was un-necessary as cfengine3.service already
wants the individual services.

cfengine/core#5362

Ticket: CFE-3982
Changelog: commit
(cherry picked from commit 4e490e7)
vpodzime pushed a commit to vpodzime/masterfiles that referenced this pull request Nov 8, 2023
WantedBy=cfengine3.service was removed from systemd service templates
for individual components. It was un-necessary as cfengine3.service already
wants the individual services.

cfengine/core#5362

Ticket: CFE-3982
Changelog: commit
(cherry picked from commit 4e490e7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants