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

Correctly start rhc-canonical-facts.timer and service #157

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

subpop
Copy link
Collaborator

@subpop subpop commented Oct 15, 2024

A collection of small changes to the rhc-canonical-facts service and timer. These changes are intended to ensure the timer and services run appropriately. Instead of trying to start rhc-canonical-facts.service as a requirement of yggdrasil.service, this PR changes the behavior of roc connect so that first the rhc-canonical-facts.timer is started, followed by the yggdrasil.service.

Card ID: CCT-291

@subpop subpop force-pushed the canonical-facts-service branch from 07514cd to 26fb5d6 Compare October 15, 2024 16:31
@subpop
Copy link
Collaborator Author

subpop commented Oct 16, 2024

/packit rebuild-failed

@subpop subpop force-pushed the canonical-facts-service branch from b3d8f54 to ee54746 Compare October 24, 2024 15:50
@subpop subpop force-pushed the canonical-facts-service branch 3 times, most recently from d7ace59 to 60c40f6 Compare November 12, 2024 14:45
@subpop subpop force-pushed the canonical-facts-service branch 7 times, most recently from ec67f69 to 9a1081d Compare November 27, 2024 04:56
@subpop subpop marked this pull request as ready for review November 27, 2024 14:44
@subpop subpop requested a review from jirihnidek November 27, 2024 14:44
@subpop subpop marked this pull request as draft December 2, 2024 17:36
@subpop subpop removed the request for review from jirihnidek December 2, 2024 17:36
@subpop subpop force-pushed the canonical-facts-service branch 5 times, most recently from 8ca6042 to 72f554e Compare December 5, 2024 03:37
@subpop subpop marked this pull request as ready for review December 5, 2024 03:56
@subpop subpop requested a review from jirihnidek December 5, 2024 03:56
@subpop subpop changed the title Require yggdrasil.service Correctly start rhc-canonical-facts.timer and service Dec 5, 2024
Copy link
Contributor

@jirihnidek jirihnidek left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Overall it looks good, and it seems that it works in typical scenario. I have some questions, comments and suggestions.

When starting yggdrasil service failed (due to fact that mosquitto was not running), then nothing is reported, but when I tried to run rhc disconnect and yggdrasil service was not running, then following error was printed:

root@localhost:/# rhc disconnect
Disconnecting thinkpad-p1 from Red Hat.
This might take a few seconds.

● Cannot deactivate yggdrasil service: cannot disable yggdrasil.service: cannot stop unit yggdrasil.service: timed out waiting for state 'inactive': timed out waiting 1s for unit state 'inactive'
● Disconnected from Red Hat Insights
● Disconnected from Red Hat Subscription Management

Manage your connected systems: https://red.ht/connector

The following errors were encountered during disconnect:

TYPE   STEP       ERROR  
ERROR  yggdrasil  Cannot deactivate yggdrasil service: cannot disable yggdrasil.service: cannot stop unit yggdrasil.service: timed out waiting for state 'inactive': timed out waiting 1s for unit state 'inactive'

When I run rhc disconnect and then I run systemct status yggdrasil.service, then I got following warning message:

Warning: The unit file, source configuration file or drop-ins of yggdrasil.service changed on disk. Run 'systemctl daemon-reload' to reload units.

... The reason is (probably) in one of my comment.

data/systemd/rhc-canonical-facts.service Show resolved Hide resolved
data/systemd/80-rhc.preset Outdated Show resolved Hide resolved
dist/srpm/rhc.spec.in Show resolved Hide resolved
internal/systemd/systemd.go Show resolved Hide resolved
internal/systemd/systemd.go Show resolved Hide resolved
internal/systemd/systemd.go Show resolved Hide resolved
internal/systemd/systemd.go Outdated Show resolved Hide resolved
internal/systemd/systemd.go Outdated Show resolved Hide resolved
activate.go Show resolved Hide resolved
activate.go Show resolved Hide resolved
To ensure yggd can read the facts file, create it and change ownership
to yggdrasil.
The timer should only be started when `rhc connect` is run, therefore it is no longer necessary to enable the timer by default.
This package implements a basic wrapper around the systemd D-Bus API.
Running 'rhc connect' will explicitly activate and start the
rhc-canonical-facts.timer unit. Likewise, running 'rhc disconnect' will
explicitly deactivate and stop the rhc-canonical-facts.timer unit.
@subpop subpop force-pushed the canonical-facts-service branch from 72f554e to 013b766 Compare December 6, 2024 16:16
@subpop
Copy link
Collaborator Author

subpop commented Dec 6, 2024

Cannot deactivate yggdrasil service: cannot disable yggdrasil.service: cannot stop unit yggdrasil.service: timed out waiting for state 'inactive': timed out waiting 1s for unit state 'inactive'

This error message seems acceptable to me. Would you prefer something else?

@subpop subpop requested a review from jirihnidek December 6, 2024 16:18
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