Skip to content
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

[Rock-ons] Failure to detect rock-ons using host networking #2242

Closed
FroggyFlox opened this issue Jan 4, 2021 · 0 comments · Fixed by #2261
Closed

[Rock-ons] Failure to detect rock-ons using host networking #2242

FroggyFlox opened this issue Jan 4, 2021 · 0 comments · Fixed by #2261
Assignees

Comments

@FroggyFlox
Copy link
Member

FroggyFlox commented Jan 4, 2021

One of the features intended in #2207 was to detect the rock-ons with container(s) using host networking so that we disable the Networking customization options accordingly. Unfortunately, the underlying logic was lost during #2207's development and we now fail to accurately detect such rock-ons.

As mentioned in #2239 (comment), the culprit is the following line...

if container:
cmd.extend((running_filters + ["--filter", "name={}".format(container),]))

... in which the running_filters shouldn't be applied there. Indeed, these are intended to detect running rock-ons, which shouldn't be used here and is in contradiction with the all=True flag that we need instead (and use).

It has also become evident we need to establish unit test coverage of system/docker.py to catch such problems/regressions.

@phillxnet phillxnet added this to the First 4 Stable (ISO) milestone Jan 23, 2021
FroggyFlox added a commit to FroggyFlox/rockstor-core that referenced this issue Jan 24, 2021
out to be unnecessary and contradicting the intended logic (rockstor#2242)
FroggyFlox added a commit to FroggyFlox/rockstor-core that referenced this issue Jan 24, 2021
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.
phillxnet added a commit that referenced this issue Jan 25, 2021
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 a pull request may close this issue.

2 participants