Skip to content

fix no valid addresses caused by lack of ip's version#4

Merged
rakelkar merged 3 commits intorakelkar:windowsCnifrom
carlory:windowsCni
Feb 6, 2018
Merged

fix no valid addresses caused by lack of ip's version#4
rakelkar merged 3 commits intorakelkar:windowsCnifrom
carlory:windowsCni

Conversation

@carlory
Copy link

@carlory carlory commented Feb 5, 2018

I build the win-overlay.exe from the windowsCni branch. but the winoverlay.exe couldn't create the PodSandbox even though it can correctly create hnsendpoints in the vxlan0 overlay network when I
create a deployment resource on my linux master node.

the last step of cmdAdd is printing result. the types.PrintResult finally call this convertTo020 func to get a types020.Result. However this hns.ConstructResult(hnsNetwork, hnsEndpoint) forgot to add the ip's version for current.IPConfig, so we got the err "no valid addresses".

@carlory
Copy link
Author

carlory commented Feb 5, 2018

// Convert to the older 0.2.0 CNI spec Result type
func (r *Result) convertTo020() (*types020.Result, error) {
	oldResult := &types020.Result{
		CNIVersion: types020.ImplementedSpecVersion,
		DNS:        r.DNS,
	}

	for _, ip := range r.IPs {
		// Only convert the first IP address of each version as 0.2.0
		// and earlier cannot handle multiple IP addresses
		if ip.Version == "4" && oldResult.IP4 == nil {
			oldResult.IP4 = &types020.IPConfig{
				IP:      ip.Address,
				Gateway: ip.Gateway,
			}
		} else if ip.Version == "6" && oldResult.IP6 == nil {
			oldResult.IP6 = &types020.IPConfig{
				IP:      ip.Address,
				Gateway: ip.Gateway,
			}
		}

		if oldResult.IP4 != nil && oldResult.IP6 != nil {
			break
		}
	}

	if oldResult.IP4 == nil && oldResult.IP6 == nil {
		return nil, fmt.Errorf("cannot convert: no valid IP addresses")
	}
}

"net"
"strings"

"github.com/Microsoft/hcsshim"
Copy link
Owner

Choose a reason for hiding this comment

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

Did you run gofmt?

Copy link
Author

Choose a reason for hiding this comment

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

@rakelkar

I changed these codes in the vscode editor. It looks like that my editor auto run the gofmt command for me.
should I manually make it recover as before?

@rakelkar
Copy link
Owner

rakelkar commented Feb 5, 2018

@carlory thanks for your fix! can you help me understand when this issue occurs? is this when you work with a new version of CNI?

@rakelkar
Copy link
Owner

rakelkar commented Feb 5, 2018

@madhanrm @dineshgovindasamy change looks reasonable to me - please take a look and comment if you disagree. Thanks

@rakelkar
Copy link
Owner

rakelkar commented Feb 6, 2018 via email

@carlory
Copy link
Author

carlory commented Feb 6, 2018

@rakelkar I updated the description of this issue.

@rakelkar
Copy link
Owner

rakelkar commented Feb 6, 2018

LGTM

@rakelkar rakelkar merged commit 0dbfae4 into rakelkar:windowsCni Feb 6, 2018
dineshgovindasamy pushed a commit that referenced this pull request Feb 12, 2018
* fix no valid addresses caused by lack of  ip's version
* fix nit
* fmt err
madhanrm pushed a commit that referenced this pull request Feb 12, 2018
* fix no valid addresses caused by lack of  ip's version
* fix nit
* fmt err
@carlory carlory deleted the windowsCni branch February 13, 2018 10:05
madhanrm pushed a commit that referenced this pull request Mar 5, 2018
* fix no valid addresses caused by lack of  ip's version
* fix nit
* fmt err
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.

2 participants