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

fix: improve socket error handling #414

Merged
merged 4 commits into from
Jul 3, 2024
Merged

Conversation

KMJ-007
Copy link
Contributor

@KMJ-007 KMJ-007 commented Jun 29, 2024

I am new to rust, I was building docker tui, and came across the same issue as #383, i tried to fix this,

I don't know what other things I need to fix or add, this is going to be my 1st PR for rust, so if any issues need to be addressed please let me know,

closes #383

had this question:

  • should i modify existing examples or create new one for this?
  • is there a need for a test for this?

thankyou for creating this lib, it is helping me a lot!!

@fussybeaver
Copy link
Owner

Thanks for this... could you also check if the socket is readable? I recall that some linux distributions require group access to the socket.

@KMJ-007
Copy link
Contributor Author

KMJ-007 commented Jun 29, 2024

Thanks for this... could you also check if the socket is readable? I recall that some linux distributions require group access to the socket.

i have mac, in that it is working, not asking for any permission, don't have linux machine at the moment

image

@KMJ-007
Copy link
Contributor Author

KMJ-007 commented Jul 1, 2024

@fussybeaver , i got my friend's linux pc, and tested it

it is working, not asking for any permission

image

@KMJ-007
Copy link
Contributor Author

KMJ-007 commented Jul 1, 2024

it is running pop os

@fussybeaver , i got my friend's linux pc, and tested it

it is working, not asking for any permission

image

@KMJ-007
Copy link
Contributor Author

KMJ-007 commented Jul 1, 2024

let me know what is show stopper for this PR, and what i can improve?

src/docker.rs Outdated
@@ -4,6 +4,7 @@ use std::fs;
use std::future::Future;
#[cfg(feature = "ssl")]
use std::io;
use std::path::Path;
#[cfg(feature = "ssl")]
use std::path::{Path, PathBuf};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI is failing because the ssl feature conflicts with this changeset

@KMJ-007
Copy link
Contributor Author

KMJ-007 commented Jul 2, 2024

All ci/cd's are green except docs one, i tried to check the logs but didn't understand why it is falling

@fussybeaver
Copy link
Owner

It looks like a doctest is failing, try adding a ,no_run pragma to this line

@KMJ-007
Copy link
Contributor Author

KMJ-007 commented Jul 2, 2024

now something new is failing

@KMJ-007
Copy link
Contributor Author

KMJ-007 commented Jul 3, 2024

@fussybeaver All Green

@fussybeaver fussybeaver merged commit 0c38ac2 into fussybeaver:master Jul 3, 2024
13 checks passed
@mominul
Copy link

mominul commented Jul 9, 2024

@fussybeaver Hi, can you kindly release a patch version of this crate with this change? I am facing this HyperLegacyError.

called `Result::unwrap()` on an `Err` value: Client(CreateContainer(HyperLegacyError { err: Error { kind: Connect, source: Some(Os { code: 2, kind: NotFound, message: "No such file or directory" }) } }))

I am using testcontainers_modules crate which in turn uses bollard on macOS with Docker version 26.1.1, build 4cf5afa. Although docker is running I am getting this error. I got referenced to #399 from a google search. It might help me to troubleshoot this error if this change is available in the new release.

@mominul
Copy link

mominul commented Jul 9, 2024

After patching the bollard dependency in Cargo.toml

[patch.crates-io]
bollard = { git = "https://github.com/fussybeaver/bollard.git" }

The error I get is:

Failed to start container: failed to initialize a docker client: Socket not found: /var/run/docker.sock

After enabling the Allow privileged port mapping option, now bollard is able to connect with docker successfully. 🎉

image

I solved this by following the lando/lando#3533 thread. But following the testcontainers/testcontainers-java#6045 and lando/lando#3533 threads, it seems that in macOS docker desktop opens the docker.sock at ~/.docker/run/docker.sock and /var/run/docker.sock is a symlink to it:

$ ll /var/run/docker.sock 
lrwxr-xr-x@ 1 root  daemon    38B Jul  9 23:55 /var/run/docker.sock@ -> /Users/mominul/.docker/run/docker.sock

So on macOS, ~/.docker/run/docker.sock should be a fallback option, otherwise the users would need to face issues like I have faced.

Thank you for this crate! ❤️

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 this pull request may close these issues.

Add Docker Socket availability check during connection creation
3 participants