Remove duplicate keys in labels of docker info

This fix tries to address the issue raised in 24392 where
labels with duplicate keys exist in `docker info`, which
contradicts with the specifications in the docs.

The reason for duplicate keys is that labels are stored as
slice of strings in the format of `A=B` (and the input/output).

This fix tries to address this issue by checking conflict
labels when daemon started, and remove duplicate labels (K-V).

The existing `/info` API has not been changed.

An additional integration test has been added to cover the
changes in this fix.

This fix fixes 24392.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

cli/command/system/info.go

@@ -223,6 +223,21 @@ func prettyPrintInfo(dockerCli *command.DockerCli, info types.Info) error {
 		for _, attribute := range info.Labels {
 			fmt.Fprintf(dockerCli.Out(), " %s\n", attribute)
 		}
+		// TODO: Engine labels with duplicate keys has been deprecated in 1.13 and will be error out
+		// after 3 release cycles (1.16). For now, a WARNING will be generated. The following will
+		// be removed eventually.
+		labelMap := map[string]string{}
+		for _, label := range info.Labels {
+			stringSlice := strings.SplitN(label, "=", 2)
+			if len(stringSlice) > 1 {
+				// If there is a conflict we will throw out an warning
+				if v, ok := labelMap[stringSlice[0]]; ok && v != stringSlice[1] {
+					fmt.Fprintln(dockerCli.Err(), "WARNING: labels with duplicate keys and conflicting values have been deprecated")
+					break
+				}
+				labelMap[stringSlice[0]] = stringSlice[1]
+			}
+		}
 	}
 
 	ioutils.FprintfIfTrue(dockerCli.Out(), "Experimental: %v\n", info.ExperimentalBuild)

cmd/dockerd/daemon.go

@@ -396,6 +396,23 @@ func loadDaemonCliConfig(opts daemonOptions) (*daemon.Config, error) {
 		return nil, err
 	}
 
+	// Labels of the docker engine used to allow multiple values associated with the same key.
+	// This is deprecated in 1.13, and, be removed after 3 release cycles.
+	// The following will check the conflict of labels, and report a warning for deprecation.
+	//
+	// TODO: After 3 release cycles (1.16) an error will be returned, and labels will be
+	// sanitized to consolidate duplicate key-value pairs (config.Labels = newLabels):
+	//
+	// newLabels, err := daemon.GetConflictFreeLabels(config.Labels)
+	// if err != nil {
+	//	return nil, err
+	// }
+	// config.Labels = newLabels
+	//
+	if _, err := daemon.GetConflictFreeLabels(config.Labels); err != nil {
+		logrus.Warnf("Engine labels with duplicate keys and conflicting values have been deprecated: %s", err)
+	}
+
 	// Regardless of whether the user sets it to true or false, if they
 	// specify TLSVerify at all then we need to turn on TLS
 	if config.IsValueSet(cliflags.FlagTLSVerify) {

daemon/config.go

@@ -220,6 +220,30 @@ func parseClusterAdvertiseSettings(clusterStore, clusterAdvertise string) (strin
 	return advertise, nil
 }
 
+// GetConflictFreeLabels validate Labels for conflict
+// In swarm the duplicates for labels are removed
+// so we only take same values here, no conflict values
+// If the key-value is the same we will only take the last label
+func GetConflictFreeLabels(labels []string) ([]string, error) {
+	labelMap := map[string]string{}
+	for _, label := range labels {
+		stringSlice := strings.SplitN(label, "=", 2)
+		if len(stringSlice) > 1 {
+			// If there is a conflict we will return an error
+			if v, ok := labelMap[stringSlice[0]]; ok && v != stringSlice[1] {
+				return nil, fmt.Errorf("conflict labels for %s=%s and %s=%s", stringSlice[0], stringSlice[1], stringSlice[0], v)
+			}
+			labelMap[stringSlice[0]] = stringSlice[1]
+		}
+	}
+
+	newLabels := []string{}
+	for k, v := range labelMap {
+		newLabels = append(newLabels, fmt.Sprintf("%s=%s", k, v))
+	}
+	return newLabels, nil
+}
+
 // ReloadConfiguration reads the configuration in the host and reloads the daemon and server.
 func ReloadConfiguration(configFile string, flags *pflag.FlagSet, reload func(*Config)) error {
 	logrus.Infof("Got signal to reload configuration, reloading from: %s", configFile)
@@ -232,6 +256,23 @@ func ReloadConfiguration(configFile string, flags *pflag.FlagSet, reload func(*C
 		return fmt.Errorf("file configuration validation failed (%v)", err)
 	}
 
+	// Labels of the docker engine used to allow multiple values associated with the same key.
+	// This is deprecated in 1.13, and, be removed after 3 release cycles.
+	// The following will check the conflict of labels, and report a warning for deprecation.
+	//
+	// TODO: After 3 release cycles (1.16) an error will be returned, and labels will be
+	// sanitized to consolidate duplicate key-value pairs (config.Labels = newLabels):
+	//
+	// newLabels, err := GetConflictFreeLabels(newConfig.Labels)
+	// if err != nil {
+	//      return err
+	// }
+	// newConfig.Labels = newLabels
+	//
+	if _, err := GetConflictFreeLabels(newConfig.Labels); err != nil {
+		logrus.Warnf("Engine labels with duplicate keys and conflicting values have been deprecated: %s", err)
+	}
+
 	reload(newConfig)
 	return nil
 }

docs/deprecated.md

@@ -26,6 +26,14 @@ see [Feature Deprecation Policy](index.md#feature-deprecation-policy).
 
 The daemon is moved to a separate binary (`dockerd`), and should be used instead.
 
+### Duplicate keys with conflicting values in engine labels
+**Deprecated In Release: [v1.13](https://github.com/docker/docker/releases/)**
+
+**Target For Removal In Release: v1.16**
+
+Duplicate keys with conflicting values have been deprecated. A warning is displayed
+in the output, and an error will be returned in the future.
+
 ### Three argument form in `docker import`
 **Deprecated In Release: [v0.6.7](https://github.com/docker/docker/releases/tag/v0.6.7)**
 

integration-cli/docker_cli_info_test.go

@@ -219,3 +219,15 @@ func (s *DockerDaemonSuite) TestRegistryMirrors(c *check.C) {
 	c.Assert(out, checker.Contains, fmt.Sprintf(" %s", registryMirror1))
 	c.Assert(out, checker.Contains, fmt.Sprintf(" %s", registryMirror2))
 }
+
+// Test case for #24392
+func (s *DockerDaemonSuite) TestInfoLabels(c *check.C) {
+	testRequires(c, SameHostDaemon, DaemonIsLinux)
+
+	err := s.d.Start("--label", `test.empty=`, "--label", `test.empty=`, "--label", `test.label="1"`, "--label", `test.label="2"`)
+	c.Assert(err, checker.IsNil)
+
+	out, err := s.d.Cmd("info")
+	c.Assert(err, checker.IsNil)
+	c.Assert(out, checker.Contains, "WARNING: labels with duplicate keys and conflicting values have been deprecated")
+}