Fix detection of rock-on host networking (#2242) #2261
Merged
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
out to be unnecessary and contradicting the intended logic (rockstor#2242) Accordingly, rename probe_running_containers() to probe_containers() as we don't always probe for "running" containers.
|
@FroggyFlox Re:
Confirmed: |
|
@FroggyFlox
As well as the blocking mechanism for this non host networking rock-on while the rock-on was enable. This all looks thus far to be as tested/presented, I'll get this merged to avoid any further hold up. Thanks a bunch for seeing to this niggle, much appreciated. Nice fix and the associated simplification is most welcome. |
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.
Fixes #2242
@phillxnet, ready for review
As detailed in #2242, we are facing a limitation in the changes included in #2207 as the latter was supposed to correctly detect rock-ons that are using containers with host networking. This is the result of an improperly configured setting in
system.docker.probe_running_containers(). This pull-request thus proposes to simply correct that mistake to properly detect rock-on using host networking.Description
As mentioned above, we currently fail to properly detect host networking because we are currently running the following docker command (edited for simplicity):
docker ps -a --filter network="host" [running_filters]In the above, the
[running_filters]flag contradicts and nullifies the-aflag, which is thus responsible for our failure to detect host networking correctly.It is important to note that the
[running_filters]were originally introduced early in #2207 development as a way to ensure the containers were running before connecting them to docker networks. This turns out to be unnecessary and over-complicating the overall back-end logic as docker can connect a container to a network even if it's not currently running ("created" is the minimum). As a result, we can explore doing without these[running_filters].In this context, here are the current uses of
probe_running_containers():Here, we can see that none of these uses do actually depend on the related container to be in a
runningstate, so we can deprecate these[running_filters]safely. This is thus what this PR is proposing to do.Note that it also renames
probe_running_containers()toprobe_containers()to reflect this change.Demonstration
Reproducing the steps in #2242 (in its linked comment: #2239 (comment)):
Notably, other uses of the
probe_containers()function have been verified to be unaffected by:Testing
Leap 15.2 (ISO install):
Full Testing Outputs
Leap15.2 (ISO install)