Skip to content

Fix guestagent service file #3579

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

Merged
merged 1 commit into from
Jun 4, 2025

Conversation

songponssw
Copy link
Contributor

Fix #3467

I think that the service file is supposed to be created with variables during the boot process.
I’ve fixed the heredoc to print LIMA_CIDATA_* in the boot script.

@jandubois
Copy link
Member

I don't think interpolating the values in the script directly, and having to escape all normal '$' signsis maintenance-friendly. You could define the values first with an interpolated block, and then append the rest of the script verbatim:

cat >/etc/init.d/lima-guestagent <<EOF
#!/sbin/openrc-run
LIMA_CIDATA_GUEST_INSTALL_PREFIX=$LIMA_CIDATA_GUEST_INSTALL_PREFIX
...

EOF

cat >>/etc/init.d/lima-guestagent <<'EOF'
supervisor=supervise-daemon

log_file="${log_file:-/var/log/${RC_SVCNAME}.log}"
...
EOF

But then you can just as well write the values to /etc/conf.d/lima-guestagent.

@songponssw songponssw force-pushed the fix-alpine-guestagent-rc branch 2 times, most recently from dc94184 to 5eb93ff Compare May 26, 2025 02:47
@songponssw
Copy link
Contributor Author

Got it!
Actually, the lima.env file contains the LIMA_CIDATA_*, so I just print it into conf.d/lima-guestagent.

@songponssw songponssw force-pushed the fix-alpine-guestagent-rc branch 2 times, most recently from a65f2fc to af42d2e Compare May 26, 2025 03:22
@jandubois
Copy link
Member

Got it! Actually, the lima.env file contains the LIMA_CIDATA_*, so I just print it into conf.d/lima-guestagent.

There is one problem with that though: lima.env uses the syntax of /etc/environment, but /etc/conf.d files are sourced as shell scripts.

So in lima.env you have LIMA_CIDATA_...=some random text" and everything until either a newline or the #` character will be part of the value.

In shell this can be a syntax error (or even run an unintended command), so the string needs to be quoted using the shell quoting rules.

Now, it looks like we don't have any crazy values in there, that can in turn contain quotes etc, so simply surrounding the value with double-quotes should be good enough.

The only value that I found creating problems was the user comment field (full name):

LIMA_CIDATA_COMMENT=Jan Dubois

And it is just noise, but I still think we should do this properly:

lima-alpine:~# rc-service lima-guestagent stop
/usr/libexec/rc/sh/openrc-run.sh: /etc/init.d/../conf.d/lima-guestagent: line 5: Dubois: not found
 * Stopping lima-guestagent ...                                                                                         [ ok ]
lima-alpine:~# rc-service lima-guestagent start
/usr/libexec/rc/sh/openrc-run.sh: /etc/init.d/../conf.d/lima-guestagent: line 5: Dubois: not found
 * Starting lima-guestagent ...                                                                                         [ ok ]

@songponssw
Copy link
Contributor Author

Appreciate the clarification. I will make the changes.

@songponssw songponssw force-pushed the fix-alpine-guestagent-rc branch from af42d2e to 7c19693 Compare May 26, 2025 06:38
Co-authored-by: Jan Dubois <[email protected]>
Signed-off-by: Songpon Srisawai <[email protected]>
@songponssw songponssw force-pushed the fix-alpine-guestagent-rc branch from d31ceb9 to 5ce662d Compare May 27, 2025 03:28
@songponssw songponssw marked this pull request as ready for review May 28, 2025 13:31
@AkihiroSuda AkihiroSuda requested a review from jandubois June 2, 2025 06:14
Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

@jandubois jandubois merged commit fdc54e0 into lima-vm:master Jun 4, 2025
34 checks passed
@jandubois jandubois added this to the v1.1.2 milestone Jun 4, 2025
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.

/etc/init.d/lima-guestagent uses LIMA_CIDATA_* environment variables
2 participants