Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

Fix "index out of range" error while getting IP #25

Closed
wants to merge 1 commit into from
Closed

Fix "index out of range" error while getting IP #25

wants to merge 1 commit into from

Conversation

carlosedp
Copy link
Contributor

Description

When getting IP for the created container, if the host does not has eth0
as the default network interface, the "index out of range" error is
generated.
This gets the correct interface for the namespace of the container PID.

Motivation and Context

  • I have raised an issue to propose this change this is required

This fixes the error on issue #24 .

How Has This Been Tested?

Tested on a host where the default network interface is eth1 that generated the reported error. With this fix, the IP is returned correctly.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

Commits:

  • I've read the CONTRIBUTION guide
  • My commit message has a body and describe how this was tested and why it is required.
  • I have signed-off my commits with git commit -s for the Developer Certificate of Origin (DCO)

Code:

  • My code follows the code style of this project.
  • I have added tests to cover my changes.

Docs:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@derek derek bot added the no-dco label Jan 13, 2020
@derek
Copy link

derek bot commented Jan 13, 2020

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide.
Tip: if you only have one commit so far then run: git commit --amend --signoff and then git push --force.

@derek derek bot removed the no-dco label Jan 13, 2020
When getting IP for the created container, if the host does not has eth0
as the default network interface, the "index out of range" error is
generated.
This gets the correct interface for the namespace of the container PID.

Signed-off-by: Carlos de Paula <[email protected]>
if config.Sandbox == netns {
for _, ipConfig := range config.IPConfigs {
serviceMap.Add(name, &ipConfig.IP)
log.Printf("%s has IP: %s.\n", name, &ipConfig.IP)
Copy link
Owner

Choose a reason for hiding this comment

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

Wait, why aren't we doing a break out of the loops here?

serviceMap.Add(name, &ipConfig.IP)
log.Printf("%s has IP: %s.\n", name, &ipConfig.IP)
}
}
Copy link
Owner

Choose a reason for hiding this comment

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

Let's discover the IP in the loop, then act on it after the loop, either from breaking out or from finishing without a result.

Copy link
Owner

@alexellis alexellis left a comment

Choose a reason for hiding this comment

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

The network discovery could be improved. Please can you extract it to its own function before merge.

@carlosedp
Copy link
Contributor Author

This has been replaced by the refactor on PR #26

@carlosedp carlosedp closed this Jan 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants