cmd: add detection for containers and pid1 #248
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@cjdcordeiro @hpidcock Could I kindly use your expertise to scrutinise the proposed change to the boot id code for containers, as well as the container detection logic? |
flotter
force-pushed
the
container-detect
branch
from
35aaad6
to
9494855
Compare
hpidcock
requested changes
Jun 27, 2023
|
|
||
| // ResetContainerInit forces the container runtime check | ||
| // to retry with globals reset | ||
| func ResetContainerInit() { |
There was a problem hiding this comment.
We don't need this if we seperate out the logic from the variable/sync
|
|
||
| // MockPid2ProcPath assigns a temporary path to where the PID2 | ||
| // status can be found. | ||
| func MockPid2ProcPath(path string) (restore func()) { |
There was a problem hiding this comment.
Same here, if we allow the implementation to be tested directly, we don't need this.
cjdcordeiro
reviewed
Jun 27, 2023
woky
reviewed
Jun 27, 2023
The system manager currently makes some assumptions about the environment
it is running in.
For example, it assumes that a shutdown program is available in userspace
and accessible with PATH configured appropriately.
internals/daemon/daemon.go:
:
cmd := exec.Command("shutdown", "-r", ...
:
This patch adds two detection mechanisms that will allow code to make
environment specific decisions in the future (not part of this patch):
- cmd.IsConfined() returns true if running inside a container runtime
- cmd.IsInit() returns true if the system manager was started as PID 1
In addition, the overlord code currently disables reboot failure detection
if the system manager is running as PID 1. However, this change is only
required for container runtimes, and not generically.
- Update the boot id workaround code to only apply for container runtimes.
flotter
force-pushed
the
container-detect
branch
from
baba647
to
9c11f29
Compare
|
Sorry people for talking your time for reviews on this, but it was agreed the scope at which I was trying to solve my own little problem is too inflated here, so I am going to have to close this for now. Perhaps this will be needed again. The next person to find this closed PR with a hope to detect container runtimes / VMs - have a look here: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The system manager currently makes some assumptions about the environment it is running in.
For example, it assumes that a shutdown program is available in userspace and accessible with PATH configured appropriately.
internals/daemon/daemon.go:
:
cmd := exec.Command("shutdown", "-r", ...
:
This patch adds two detection mechanisms that will allow code to make environment specific decisions in the future (not part of this patch):
cmd.IsConfined() returns true if running inside a container runtime
cmd.IsInit() returns true if the system manager was started as PID 1
In addition, the overlord code currently disables reboot failure detection if the system manager is running as PID 1. However, this change is only required for container runtimes, and not generically.