Add more cases for API versions

[nishanttotla]

Just making sure we're keeping up.

Ping @dongluochen @allencloud

Signed-off-by: Nishant Totla nishanttotla@gmail.com

[dongluochen]

LGTM

[allencloud]

Just one question, if the serverVersion is 17.03.1~ce, what will return from versions.LessThan(serverVersion, "17.03.1")? I think it will return true, am I right? Here is some code from:

// compare compares two version strings
// returns -1 if v1 < v2, 1 if v1 > v2, 0 otherwise.
func compare(v1, v2 string) int {
	var (
		currTab  = strings.Split(v1, ".")
		otherTab = strings.Split(v2, ".")
	)

	max := len(currTab)
	if len(otherTab) > max {
		max = len(otherTab)
	}
	for i := 0; i < max; i++ {
		var currInt, otherInt int

		if len(currTab) > i {
			currInt, _ = strconv.Atoi(currTab[i])
		}
		if len(otherTab) > i {
			otherInt, _ = strconv.Atoi(otherTab[i])
		}
		if currInt > otherInt {
			return 1
		}
		if otherInt > currInt {
			return -1
		}
	}
	return 0
}

// LessThan checks if a version is less than another
func LessThan(v, other string) bool {
	return compare(v, other) == -1
}

[nishanttotla]

@allencloud good point. I wonder if this is something we need to start checking for everywhere (stripping off the ~ce) at the end if it's going to be present.

WDYT @dongluochen?

[dongluochen]

version is from moby/moby. How is it handled there?

[allencloud]

version is implemented here: https://github.com/moby/moby/blob/master/api/types/versions/compare.go#L10-L42
While I think it is better to strip off the suffix of -ce (maybe -ee as well, I have not experienced that yet).

[nishanttotla]

I'll take a look at how version is handled in moby/moby.

[allencloud]

Any update?
ping @nishanttotla

[nishanttotla]

@allencloud thanks for checking. I haven't had time to get back to this, I will do it in a day or so.

[nishanttotla]

@allencloud I've fixed this PR to take into account the new versions. PTAL.

[allencloud]

Thanks for the improvement. And I hope to get more details of this line of code. What is the usage of this?

[nishanttotla]

My bad, this was for debugging. Removed it. @allencloud

[allencloud]

Thanks for the removal.

[allencloud]

LGTM

Still something to review.

[allencloud]

I think in comment there should be 17.06.x-ce not 17.06-ce, otherwise LGTM.

[nishanttotla]

Thanks @allencloud for the review. It seems like some tests are repeatedly failing, must be something changed with new APIs. I'll investigate.

[nishanttotla]

There's also another issue where the newer docker clients are downgrading version automatically. So actual requests that are sent seem to be using v1.24, for example

time="2017-06-27T18:39:52Z" level=debug msg="HTTP request received" method=GET uri="/v1.24/networks/node-0/bridge" 

[nishanttotla]

Figured out that the issue is #2750

Updating PR to fix #2750

[nishanttotla]

@allencloud I've finally fixed the issue. It was a notable one.

[wsong]

You should probably use ... || versions.Equal(serverVersion, "1.13.1") here.

[nishanttotla]

Done, thanks!

[nishanttotla]

CI seems to have been reliably fixed with this PR 🎉 🎉