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

populate bridgeName if inspect cmd is run with --all or --name #433

Merged
merged 2 commits into from
May 28, 2021

Conversation

karimra
Copy link
Member

@karimra karimra commented May 28, 2021

@steiler
Copy link
Collaborator

steiler commented May 28, 2021

Karim did you consider the network mode "host"?
Quickly scanning the code I unfortunately can't tell.

switch node.NetworkMode {
case "host":
containerHostConfig.NetworkMode = container.NetworkMode("host")
default:
containerHostConfig.NetworkMode = container.NetworkMode(c.Mgmt.Network)
containerNetworkingConfig.EndpointsConfig = map[string]*network.EndpointSettings{
c.Mgmt.Network: {
IPAMConfig: &network.EndpointIPAMConfig{
IPv4Address: node.MgmtIPv4Address,
IPv6Address: node.MgmtIPv6Address,
},
},
}
}

@karimra
Copy link
Member Author

karimra commented May 28, 2021

what would be printed in case of host mode ?

@hellt
Copy link
Member

hellt commented May 28, 2021

I think for now it is ok if it prints nothing (output from the tip of this branch)

image

the potential improvements are to print host maybe?

@karimra
Copy link
Member Author

karimra commented May 28, 2021

yes, there is basically nothing to print, network name is host and was not created by clab --> skip

Instead of writing host under an IP address column, maybe add a column Network Mode or Bridge Name?

@steiler
Copy link
Collaborator

steiler commented May 28, 2021

Printing host sounds good to me.
Noting is also fine for now, just thought that this case might cause errors, not sure if this would be covered by a test so far.

@steiler
Copy link
Collaborator

steiler commented May 28, 2021

@karimra to basically normalize it ... thats also a great idea!

@hellt
Copy link
Member

hellt commented May 28, 2021

I think a column named Network with the potential values makes sense, wdyt?

  • docker network name if it is a bridged network
  • host-mode string if it is a host mode networ

@hellt hellt merged commit 6c85a8f into master May 28, 2021
@steiler steiler mentioned this pull request Jun 17, 2021
@hellt hellt deleted the fix-inspect-all branch October 22, 2021 17:44
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.

3 participants