Use controlled execution environment, to avoid failure if PATH is unset (LP: #1959570)#336
Merged
slyon merged 3 commits intocanonical:mainfrom Apr 3, 2023
Merged
Use controlled execution environment, to avoid failure if PATH is unset (LP: #1959570)#336slyon merged 3 commits intocanonical:mainfrom
slyon merged 3 commits intocanonical:mainfrom
Conversation
Contributor
Author
|
This fix seems to be working generally. There's still a brief interruption of the network connectivity, while restarting the "snap.network-manager.networkmanager.service" daemon (obviously), but the SSH connection is recovered after a few seconds. I've built a core22 base snap, using https://launchpad.net/~slyon/+archive/ubuntu/lp1959570 and a UC22 image out of this to run this test (all executed over an SSH connection): |
daniloegea
reviewed
Mar 24, 2023
Contributor
daniloegea
left a comment
daniloegea
left a comment
There was a problem hiding this comment.
Would it be enough settings the environment variables only once via os.environ in the entry point of the python code?
ogayot
reviewed
Mar 29, 2023
ogayot
reviewed
Mar 29, 2023
This is to allow execution in constrained environments, like early boot or Ubuntu Core 20/22 where the PATH environment variable might not be set. E.g. for the 'nmcli' binary: It could be /usr/bin/nmcli for normal NetworkManager or /snap/bin/nmcli -> /snap/bin/network-manager.nmcli for NM snap
Also, consider /usr/bin/nmcli and /snap/bin/nmcli -> /snap/bin/network-manager.nmcli as valid implementations of 'nmcli', so it can be exected in a minimal environment, without the PATH variable being set (e.g. early boot or Ubuntu Core 20/22).
Contributor
Author
|
All tests green. I've re-built a package in the above-mentioned PPA + core22 base snap + UC22 image and run the reproducer from the bug report: Merging. |
daniloegea
pushed a commit
that referenced
this pull request
May 17, 2023
…et (LP: #1959570) (#336) Use a controlled execution environment. This is to allow execution in constrained environments, like early boot or Ubuntu Core 20/22 where the PATH environment variable might not be set Also, handle nmcli's "NetworkManager not running" error gracefully. Considering /usr/bin/nmcli for normal NetworkManager and /snap/bin/nmcli -> /snap/bin/network-manager.nmcli for NetworkManager snap as valid implementations. COMMITS: * cli: use controlled execution environment This is to allow execution in constrained environments, like early boot or Ubuntu Core 20/22 where the PATH environment variable might not be set. E.g. for the 'nmcli' binary: It could be /usr/bin/nmcli for normal NetworkManager or /snap/bin/nmcli -> /snap/bin/network-manager.nmcli for NM snap * cli:status:ip: Introduce utils.nmcli_out() * cli:apply: Handle 'nmcli' NM not running gracefully Also, consider /usr/bin/nmcli and /snap/bin/nmcli -> /snap/bin/network-manager.nmcli as valid implementations of 'nmcli', so it can be exected in a minimal environment, without the PATH variable being set (e.g. early boot or Ubuntu Core 20/22).
5 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Description
Use a controlled execution environment.
This is to allow execution in constrained environments, like early boot or
Ubuntu Core 20/22 where the PATH environment variable might not be set
Also, handle
nmcli's "NetworkManager not running" error gracefully.Considering
/usr/bin/nmclifor normal NetworkManager and/snap/bin/nmcli -> /snap/bin/network-manager.nmclifor NetworkManager snap as valid implementations.Checklist
make checksuccessfully.make check-coverage).