fix: support systemd user-specific config #118
Conversation
WalkthroughThis pull request introduces modifications to systemd service management and Linux service templates, focusing on user-specific service handling and workflow configuration. The changes primarily involve updating the GitHub Actions integration test workflow to remove Changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
service/systemd.go (2)
17-18: Consider making service paths configurable
Currently, these const paths are hard-coded; offering configuration might help accommodate non-standard setups.
28-32: Avoid swallowing the error fromuser.Current()
Logging or returning the error could help diagnose unexpected runtime environments. For example:func NewSystemd(commandName CommandName) *Systemd { currentUser, err := user.Current() if err != nil { + fmt.Fprintf(os.Stderr, "Warning: unable to retrieve current user: %v\n", err) return &Systemd{commandName: commandName} } return &Systemd{commandName: commandName, user: currentUser, userMode: currentUser.Uid != "0"} }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/integration-tests.yml(1 hunks)service/systemd.go(4 hunks)service/template.go(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run Integration Tests on Ubuntu
🔇 Additional comments (28)
service/systemd.go (21)
23-24: Struct additions look good
Storing user details and mode flag is clear and beneficial for user-specific logic.
47-78: Prerequisite checks appear thorough
Enabling lingering and setting environment variables is appropriate for user-mode systemd usage. Consider adding debug logs if enabling lingering fails frequently in certain environments.
Line range hint
80-104: Initialization logic seems correct
App home resolution and binary path selection for each command type is consistent.
105-127: User-level vs. system-level service logic is well-handled
This approach cleanly separates user-mode from root-mode.
137-138: Prerequisite call ensures environment is set
Carrying out a final check before proceeding is a robust defensive measure.
148-148: Explicit daemon reload
A straightforward systemctl call is appropriate after creating or modifying the unit file.
157-158: Enable service
Straightforward approach to enable the service upon creation.
160-168: Centralizedsystemctlhelper
This method nicely reduces duplication, handling user-wide vs. system-wide calls in one place.
170-175: Dynamic service directory selection
Returning either the user or system location is clear.
179-181: Double-check environment prerequisites before fetching logs
Ensures journald commands have the right environment.
187-188: Log tailing
Adequate solution for streaming logs in real time.
190-196: Flexible journalctl helper
Mirrors thesystemctlapproach for user/system.
203-205: Prerequisite call before log pruning
Consistent usage helps ensure correct environment.
210-210: Vacuum logs
Prunes older logs effectively for the targeted unit.
214-216: Check environment before service start
Ensures the user environment is prepped.
221-221: Starting service
Simple call; logic is straightforward.
225-227: Check environment before stopping
Maintains consistency with other lifecycle methods.
232-232: Stop command
Appropriate direct call to systemctl.
236-238: Check environment before restarting
Keeps usage consistent across commands.
243-243: Service restart
In line with the overall pattern established here.
252-252: Returning computed service file path
Utilizes the user or system path logic effectively.service/template.go (6)
242-242: Clarified parameter order in comment
Documenting the new injection order helps maintain clarity.
250-252: User field injection for upgradable Cosmovisor
Appending%[5]sfor user-level control is a neat approach.
263-275: Removed user specification for non-upgradable Cosmovisor
Simplifies the template and matches the user-mode approach.
284-294: Binary template adjustments
The home directory and LD_LIBRARY_PATH usage align with the newly outlined arguments.
301-311: OPinitBot template
Parameter order updates and usage of[3]sfor service name is consistent with the new approach.
318-326: Relayer template
Removal of explicit user references aligns well with user-mode logic..github/workflows/integration-tests.yml (1)
30-30: No more sudo
Droppingsudoaligns with the new user-mode handling in systemd.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
service/template.go(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run Integration Tests on Ubuntu
🔇 Additional comments (4)
service/template.go (4)
252-256: Verify environment variable paths.The environment variable paths have been updated to use
%[4]s(appHome) instead of the previous path. Ensure this change is consistent with the systemd user configuration requirements.Consider adding a comment explaining why these paths are using appHome and how they relate to user-specific configuration.
284-285: LGTM! Clear and descriptive template name.The rename from
LinuxRunBinaryTemplatetoLinuxRunMinitiaBinaryTemplateimproves clarity about the template's purpose.
338-338: LGTM! Consistent template mapping.The template map has been correctly updated to use the renamed
LinuxRunMinitiaBinaryTemplate.
242-242: Verify the UserField parameter usage.The template comment indicates
UserFieldas the 5th parameter, which is used to prefix theExecStartcommand. This is likely for user-specific systemd configuration, but the usage needs to be verified.Run the following script to check how UserField is constructed and used:
Also applies to: 250-250
✅ Verification successful
UserField Usage in LinuxRunUpgradableCosmovisorTemplate Verified
- The template comment at line 242 indicates that the argument order is
[binaryName, binaryPath, serviceName, appHome, UserField]- The placeholder
%[5]sis present in the ExecStart definition (line 250) to prefix the command with the provided UserField value.- Searches confirm that no conflicting usage exists in the file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check how UserField is constructed and used in the codebase # Search for UserField construction rg -A 5 "UserField.*=.*" # Search for template usage with UserField ast-grep --pattern 'fmt.Sprintf($_, LinuxRunUpgradableCosmovisorTemplate, $$$)'Length of output: 108
Script:
#!/bin/bash # Description: Verify the usage of the UserField parameter and its associated %5$s usage in LinuxRunUpgradableCosmovisorTemplate within service/template.go echo "Searching for 'UserField' in service/template.go:" rg -n "UserField" service/template.go echo -e "\nSearching for '%[5]s' in service/template.go:" rg -n "%\[5\]s" service/template.go echo -e "\nSearching for 'LinuxRunUpgradableCosmovisorTemplate' in service/template.go:" rg -n "LinuxRunUpgradableCosmovisorTemplate" service/template.goLength of output: 1666
Description
WIP
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.
I have...
!in the type prefix if API or client breaking changeReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes