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

Disabling Rockons service breaks Rockons UI page post rocknet config #2239

Closed
phillxnet opened this issue Jan 3, 2021 · 5 comments · Fixed by #2252
Closed

Disabling Rockons service breaks Rockons UI page post rocknet config #2239

phillxnet opened this issue Jan 3, 2021 · 5 comments · Fixed by #2252

Comments

@phillxnet
Copy link
Member

Post merge of pr "Implement docker networks. Fixes #1982." #2207 (4.0.5-0) we have a failure to check if docker is first enabled prior to attempting to query docker for rocknets purposes. This leads to the following error within the Web-UI on some pages (Rock-ons currently known):

['Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?', '']

Work around is to re-enable the Rock-ons service via the services page.

Thanks to @FroggyFlox for helping to narrow down this issue thus far.

Currently this issue is only associated with Rockstor version 4.0.5 (Release candidate 6).

@FroggyFlox
Copy link
Member

Thanks a lot for clearly laying out this issue, @phillxnet.
I'll try to get to it as soon as I can.

@FroggyFlox
Copy link
Member

FroggyFlox commented Jan 3, 2021

Problem has been identified:

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

This shouldn't include the running_filters. We should thus remove these but I will need to make sure this doesn't break the other uses for this function. That also highlights the lack of corresponding unit tests, @phillxnet ... I'll try to see if I can remedy to that as well, but that is likely to take a bit more time, unfortunately.

EDIT: I forgot to mention maybe the most important point:
One shouldn't be able to customize Networking settings for a rock-on using host networking. The logic in place to detect such rock-ons has unfortunately been broken somewhere during my rebase and I failed to catch it; my apologies.
I believe this was the issue that led to the error you described in your review of #2207 : #2207 (comment)

@phillxnet
Copy link
Member Author

phillxnet commented Jan 3, 2021

@FroggyFlox

One shouldn't be able to customize Networking settings for a rock-on using host networking

Oh yes, I completely forgot about that one. Do you fancy making an additional issue for that?

@phillxnet
Copy link
Member Author

unfortunately been broken somewhere during my rebase

Yes, that my fault I'm afraid; in taking so long to tend to the rocknets pull request I appreciate that you had to rebase a number of times.
But at least it's in now :).

@FroggyFlox
Copy link
Member

@phillxnet , sorry for the lack of focus on my previous comment... I've created a dedicated issue for that.

With regards to the current issue, there actually are a few points that are either broken (the one you described), or for which the error surfaced to the user could be more explicit when the Rock-on service is not running. I'll list them below and update as I find more (if any):

  1. Creating a new rocknet from System > Network: errors out with the docker error surfaced to the user:
CommandException: Error running a command. cmd = /usr/bin/docker network list --format {{.Name}}. rc = 1. stdout = ['']. stderr = ['Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?', '']

This could be further improved to specifically tell the user to check whether the Rock-on service is turned on.

  1. The same error as in point 1 above occurs when trying to edit an existing rocknet. Here, we might want to disable the "Edit" pencil icon directly (with a tooltip explaining why), rather than surfacing an error to the user. This would also greatly simplify the back-end logic.

  2. Visiting the System > Network page throws the old unknown ctype: bridge error in the logs even though we know deal with these connections. This is because the logic behind that is expecting the Rock-on service to be ON. We could simply change the level in the logs, though, from logger.error() to logger.info() as this isn't really an error strictly speaking.

FroggyFlox added a commit to FroggyFlox/rockstor-core that referenced this issue Jan 4, 2021
FroggyFlox added a commit to FroggyFlox/rockstor-core that referenced this issue Jan 4, 2021
FroggyFlox added a commit to FroggyFlox/rockstor-core that referenced this issue Jan 6, 2021
…status

 to display/hide the edit and delete connection buttons accordingly (rockstor#2239)
FroggyFlox added a commit to FroggyFlox/rockstor-core that referenced this issue Jan 8, 2021
FroggyFlox added a commit to FroggyFlox/rockstor-core that referenced this issue Jan 8, 2021
Verify that docker daemon is running before probing containers.
Minor refactoring and comments on JS logic to detect rock-on service status
 to display/hide the edit and delete connection buttons accordingly (rockstor#2239)
Raise explicit error when attempting to create a rocknet while Rock-on service
is OFF (rockstor#2239).
Change log level of unknown ctype: bridge connection error to "debug".
FroggyFlox added a commit to FroggyFlox/rockstor-core that referenced this issue Jan 8, 2021
Verify that docker daemon is running before probing containers.
Minor refactoring and comments on JS logic to detect rock-on service status
 to display/hide the edit and delete connection buttons accordingly (rockstor#2239)
Raise explicit error when attempting to create a rocknet while Rock-on service
is OFF (rockstor#2239).
Change log level of unknown ctype: bridge connection error to "debug".
phillxnet added a commit that referenced this issue Jan 18, 2021
…k_fetch

Improve handling of rock-on-related functions when Rock-ons service is OFF (#2239)
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