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 connectivity issues in Docker Swarm #270

Merged
merged 8 commits into from
Aug 12, 2021

Conversation

vovac12
Copy link
Contributor

@vovac12 vovac12 commented Jul 20, 2021

Closes #249

Add config field to set ip address for node
let mut addr = node.bind().to_socket_addrs().unwrap().next().unwrap();

let mut addr = if let Some(addr) = node.bind_to_ip_address() {
addr
Copy link
Member

Choose a reason for hiding this comment

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

Sanity check is required: if bind_to_ip_address is specified and node address is also specified as ipv4 and they are different, then log error and panic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


let mut addr = if let Some(addr) = node.bind_to_ip_address() {
addr
} else if let Ok(addr) = node.bind().parse() {
Copy link
Member

Choose a reason for hiding this comment

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

Into what type is this string parsed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

std::net::SocketAddr

@ikopylov ikopylov requested a review from piakushin July 21, 2021 15:59
@ikopylov
Copy link
Member

@pyakushin this change is needed ASAP. Please, review it.

@vovac12 vovac12 marked this pull request as ready for review July 29, 2021 13:52
@@ -44,8 +63,15 @@ async fn main() {
.find(|n| n.name() == name)
.unwrap_or_else(|| panic!("cannot find node: '{}' in cluster config", name));
mapper = VirtualMapper::new(&node, &cluster).await;
addr = finded.address().to_socket_addrs().unwrap().next().unwrap();
addr = if let Ok(addr) = finded.address().parse() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: finded -> found

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

}
(Some(addr), _, _) => Some(addr),
(_, Ok(addr), _) => Some(addr),
(_, _, Some(port)) => Some(SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), port)),
Copy link
Contributor

Choose a reason for hiding this comment

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

SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), port) occured multiple times in the code, prob it's better to add some function for it (since it's quite a long line)

Copy link
Member

Choose a reason for hiding this comment

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

Or type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function created

}
(Some(addr), _, _) => Some(addr),
(_, Ok(addr), _) => Some(addr),
(_, _, Some(port)) => Some(SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), port)),
Copy link
Member

Choose a reason for hiding this comment

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

Or type

@@ -39,8 +38,7 @@ pub struct Disk {
impl Node {
pub async fn new(name: String, address: &str, index: u16) -> Self {
error!("address: [{}]", address);
Copy link
Member

Choose a reason for hiding this comment

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

debug code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -39,8 +38,7 @@ pub struct Disk {
impl Node {
pub async fn new(name: String, address: &str, index: u16) -> Self {
error!("address: [{}]", address);
let mut address = lookup_host(address).await.expect("DNS resolution failed");
let address = address.next().expect("address is empty");
let address = address.to_string();
Copy link
Member

Choose a reason for hiding this comment

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

Pass address as String already because you make it owned here anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@piakushin piakushin merged commit 8948e6e into master Aug 12, 2021
@piakushin piakushin deleted the 249-connectivity-issues-in-swarm branch August 12, 2021 09:55
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.

Investigate and fix connectivity issues when Bob is running inside Docker Swarm
4 participants