-
Notifications
You must be signed in to change notification settings - Fork 706
autostart: Ensure instance is started/stopped by launchd
or systemctl
#4139
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
base: master
Are you sure you want to change the base?
Conversation
783c0f2
to
428fcb1
Compare
How to test this PR? |
Also please explain what issue is being solved by this PR |
opened #4141 |
ac9db28
to
31d7007
Compare
89c500c
to
7082f48
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be *_linux.go
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code that is platform-dependent and can only be built on Linux is already isolated in systemd_linux.go
. systemd.go
has code that can be built on any platform, only the operation is platform-dependent.
If we put the code itself included in systemd.go
into systemd_linux.go
, I think the number of dummy codes required for systemd_others.go
will increase.
|
||
[Service] | ||
ExecStart={{.Binary}} start %i --foreground | ||
ExecStart={{.Binary}} start %i --foreground --progress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be *_darwin.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code included in launchd.go
is based on the darwin platform, but the build is not based on the darwin platform.
The runtime guard should be enough.
<string>start</string> | ||
<string>{{ .Instance }}</string> | ||
<string>--foreground</string> | ||
<string>--progress</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there is no way to arbitrarily pass the --progress
option from the limactl start
process that the user starts to the limactl hostagent
that is started via launchctl
, It is to be handed over at the time of registration.
limactl hostagent
collects progress logs regardless of whether there is a process to observe progress.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is --progress
needed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To pass --progress
to limactl hostagent
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User -> limactl start --progress
-> launchctl
-> launchd
-> limactl start --progress
-> limactl hostagent --progress
.
There is no api to passing --progress
from user initiated limactl start
to limactl hostagent
via launchctl
.
- Hide creating/deleting an autostart entry file into `Register`/`Unregister` as the implementation details. - Separate the `launchd` and `systemd`-specific code into packages. Signed-off-by: Norio Nomura <[email protected]>
…ctl` If the instance isn’t launched by `launchd`, it won’t be stopped on log out. This changes to use `launchctl` or `systemctl` to start/stop the instance if it’s registered to autostart. ## Affected sub commands: - `limactl start` - `limactl stop` - `limactl restart` - `limactl edit` - `limactl shell` ## API changes ### `pkg/autostart` - Introduced `AutoStartedIdentifier()`: - If not empty, it indicates whether the instance was started by `launchd` or `systemd`. - Added `RequestStart()`: - Delegates the operation to `launchd` or `systemd`. - Added `RequestStop()`: - Delegates the operation to `launchd` or `systemd`. ### `pkg/autostart/launchd` - Added `AutoStartedServiceName()`: - Uses the XPC_SERVICE_NAME environment variable as the service name. - Added `RequestStart()`: - Uses `launchctl enable service-target` to avoid failing `bootstrap`. - Uses `launchctl bootstrap domain-target plist-path`. - Added `RequestStop()`: - Uses `launchctl bootout service-target` if the instance is launched by `launchd`. - Added `--progress` to the `limactl` option in `io.lima-vm.autostart.INSTANCE.plist`: - Required to support `limactl start --progress`. ### `pkg/autostart/systemd` - Added `AutoStartedServiceName()`: - Uses `CurrentUnitName()` by `github.com/coreos/go-systemd/v22/util` as the service identifier. - Added `RequestStart()`: - Uses `systemctl --user start unit-name`. - Added `RequestStop()`: - Uses `systemctl --user stop unit-name` if the instance is launched by `systemd`. - Added `--progress` to the `limactl` option in `[email protected]`: - Required to support `limactl start --progress`. ### `pkg/hostagent` - Add `AutoStartedIdentifier` to `Info`. - If not empty, it indicates whether the instance was started by `launchd` or `systemd`. ### `pkg/instance` - `StartWithPaths()`: - Use `autostart.IsRegistered()` to check if the instance is registered to autostart. - If `launchHostAgentForeground` is true, ignore autostart registration. - If the instance is registered to autostart, use `autostart.RequestStart()` instead of launching HostAgent. - `StopGracefully()`: - Use `autostart.RequestStop()`. - `Restart()`, `RestartForcibly()`: - Use `autostart.IsRegistered()` to skip `networks.Reconcile()` if the instance is registered to autostart. Signed-off-by: Norio Nomura <[email protected]> bump `coreos/go-systemd` to v22.6.0 Signed-off-by: Norio Nomura <[email protected]>
…hctl bootstrap` Because instances may be stopped without unloading the plist file. e.g. `limactl stop -f` or `limactl factory-reset`. If the plist file is not unloaded, `launchctl bootstrap` will fail. Signed-off-by: Norio Nomura <[email protected]>
7082f48
to
994efee
Compare
I was researching how to test and took a detour to fix |
This PR is formed with two parts:
autoStartManager
to prepare for the following commitlaunchd
orsystemctl
Refactor to add
autoStartManager
Register
/Unregister
as the implementation details.launchd
andsystemd
-specific code into packages.Use
launchctl
orsystemctl
to start/stop the instance if it’s registered to autostart.If the instance isn’t launched by
launchd
, it won’t be stopped on log out.Affected sub commands:
limactl start
limactl stop
limactl restart
limactl edit
limactl shell
API changes
pkg/autostart
AutoStartedIdentifier()
:launchd
orsystemd
.RequestStart()
:launchd
orsystemd
.RequestStop()
:launchd
orsystemd
.pkg/autostart/launchd
AutoStartedServiceName()
:RequestStart()
:launchctl enable service-target
to avoid failingbootstrap
.launchctl bootstrap domain-target plist-path
.RequestStop()
:launchctl bootout service-target
if the instance is launched bylaunchd
.--progress
to thelimactl
option inio.lima-vm.autostart.INSTANCE.plist
:limactl start --progress
.pkg/autostart/systemd
AutoStartedServiceName()
:CurrentUnitName()
bygitproxy.zycloud.tk/coreos/go-systemd/v22/util
as the service identifier.RequestStart()
:systemctl --user start unit-name
.RequestStop()
:systemctl --user stop unit-name
if the instance is launched bysystemd
.--progress
to thelimactl
option in[email protected]
:limactl start --progress
.pkg/hostagent
AutoStartedIdentifier
toInfo
.launchd
orsystemd
.pkg/instance
StartWithPaths()
:autostart.IsRegistered()
to check if the instance is registered to autostart.launchHostAgentForeground
is true, ignore autostart registration.autostart.RequestStart()
instead of launching HostAgent.StopGracefully()
:autostart.RequestStop()
.Restart()
,RestartForcibly()
:autostart.IsRegistered()
to skipnetworks.Reconcile()
if the instance is registered to autostart.