nit: we can trim the space here, and use strings.Join() instead of the loop in L246-L248.

Thanks. Done.

Probably this can be moved to UT, but I think it is ok to keep this IT at the moment.

Thanks @AkihiroSuda. In #29619 the issue was only triggered by docker build --label A=A --label B=B ... through command line. LABEL inside Dockerfile directly will not trigger the issue. So I think it probably make sense to have a IT for it.

I agree this should really be a unit test. Every bug is technically invoked from either the CLI or API, but I don't think that necessarily warrants an integration test. Integration tests are great for catching regressions where the bug was because of an integration between two components.

In this case the bug seems to be in a single component, so it could be tested directly.

A rule I like to follow is that if a function is hard to unit test, it's a good sign the function should be refactored in a way that makes it easier to test. Functions that are hard to test are often hard to re-use as well. Since this function is 100 lines long, it is difficult to test the behaviour in isolation. It might not be a bad idea to split it up, making it easier to test.

you may want to put the labels in out-of-order to test sording actually works. e.g. org.c, org.a, org.d, ...

For clarity, yes I agree.

In practice, it won't make a difference, because Go always randomizes map order; see https://nathanleclaire.com/blog/2014/04/27/a-surprising-feature-of-golang-that-colored-me-impressed/

@briceburg The map is always randomized:

ubuntu@ubuntu:~/tmp$ cat main.go 
package main

import (
	"fmt"
)

func main() {
	labels := map[string]string{
		"org.a": "cli-a",
		"org.b": "cli-b",
		"org.c": "cli-c",
		"org.d": "cli-d",
		"org.e": "cli-e",
	}
	for k, v := range labels {
		fmt.Println(k, v)
	}
}

ubuntu@ubuntu:~/tmp$ 
ubuntu@ubuntu:~/tmp$ go run main.go 
org.a cli-a
org.b cli-b
org.c cli-c
org.d cli-d
org.e cli-e
ubuntu@ubuntu:~/tmp$ go run main.go 
org.e cli-e
org.a cli-a
org.b cli-b
org.c cli-c
org.d cli-d
ubuntu@ubuntu:~/tmp$ go run main.go 
org.d cli-d
org.e cli-e
org.a cli-a
org.b cli-b
org.c cli-c
ubuntu@ubuntu:~/tmp$ go run main.go 
org.b cli-b
org.c cli-c
org.d cli-d
org.e cli-e
org.a cli-a
ubuntu@ubuntu:~/tmp$

I updated the PR to shuffle the order though to make it clear though.

Yes, totally unnecessary but perhaps more clear to a gomature like myself. boy do I have a lot to learn! 😄

Many thanks for sharing, and for jumping on the label issue so quickly! It's inspiring to witness this project work.