Browse code

Remove deprecated support for duplicate label-keys

Support for duplicate labels (but different values) was
deprecated in commit e4c9079d091a2eeac8a74a0356e3f348db873b87
(Docker 1.13), and scheduled for removal in 17.12

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Sebastiaan van Stijn authored on 2017/11/12 11:09:28
Showing 5 changed files
... ...
@@ -480,22 +480,12 @@ func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) {
480 480
 		logrus.Warnf(`The "-g / --graph" flag is deprecated. Please use "--data-root" instead`)
481 481
 	}
482 482
 
483
-	// Labels of the docker engine used to allow multiple values associated with the same key.
484
-	// This is deprecated in 1.13, and, be removed after 3 release cycles.
485
-	// The following will check the conflict of labels, and report a warning for deprecation.
486
-	//
487
-	// TODO: After 3 release cycles (17.12) an error will be returned, and labels will be
488
-	// sanitized to consolidate duplicate key-value pairs (config.Labels = newLabels):
489
-	//
490
-	// newLabels, err := daemon.GetConflictFreeLabels(config.Labels)
491
-	// if err != nil {
492
-	//	return nil, err
493
-	// }
494
-	// config.Labels = newLabels
495
-	//
496
-	if _, err := config.GetConflictFreeLabels(conf.Labels); err != nil {
497
-		logrus.Warnf("Engine labels with duplicate keys and conflicting values have been deprecated: %s", err)
483
+	// Check if duplicate label-keys with different values are found
484
+	newLabels, err := config.GetConflictFreeLabels(conf.Labels)
485
+	if err != nil {
486
+		return nil, err
498 487
 	}
488
+	conf.Labels = newLabels
499 489
 
500 490
 	// Regardless of whether the user sets it to true or false, if they
501 491
 	// specify TLSVerify at all then we need to turn on TLS
... ...
@@ -61,6 +61,28 @@ func TestLoadDaemonCliConfigWithConflicts(t *testing.T) {
61 61
 	testutil.ErrorContains(t, err, "as a flag and in the configuration file: labels")
62 62
 }
63 63
 
64
+func TestLoadDaemonCliWithConflictingLabels(t *testing.T) {
65
+	opts := defaultOptions("")
66
+	flags := opts.flags
67
+
68
+	assert.NoError(t, flags.Set("label", "foo=bar"))
69
+	assert.NoError(t, flags.Set("label", "foo=baz"))
70
+
71
+	_, err := loadDaemonCliConfig(opts)
72
+	assert.EqualError(t, err, "conflict labels for foo=baz and foo=bar")
73
+}
74
+
75
+func TestLoadDaemonCliWithDuplicateLabels(t *testing.T) {
76
+	opts := defaultOptions("")
77
+	flags := opts.flags
78
+
79
+	assert.NoError(t, flags.Set("label", "foo=the-same"))
80
+	assert.NoError(t, flags.Set("label", "foo=the-same"))
81
+
82
+	_, err := loadDaemonCliConfig(opts)
83
+	assert.NoError(t, err)
84
+}
85
+
64 86
 func TestLoadDaemonCliConfigWithTLSVerify(t *testing.T) {
65 87
 	tempFile := fs.NewFile(t, "config", fs.WithContent(`{"tlsverify": true}`))
66 88
 	defer tempFile.Remove()
... ...
@@ -258,22 +258,12 @@ func Reload(configFile string, flags *pflag.FlagSet, reload func(*Config)) error
258 258
 		return fmt.Errorf("file configuration validation failed (%v)", err)
259 259
 	}
260 260
 
261
-	// Labels of the docker engine used to allow multiple values associated with the same key.
262
-	// This is deprecated in 1.13, and, be removed after 3 release cycles.
263
-	// The following will check the conflict of labels, and report a warning for deprecation.
264
-	//
265
-	// TODO: After 3 release cycles (17.12) an error will be returned, and labels will be
266
-	// sanitized to consolidate duplicate key-value pairs (config.Labels = newLabels):
267
-	//
268
-	// newLabels, err := GetConflictFreeLabels(newConfig.Labels)
269
-	// if err != nil {
270
-	//      return err
271
-	// }
272
-	// newConfig.Labels = newLabels
273
-	//
274
-	if _, err := GetConflictFreeLabels(newConfig.Labels); err != nil {
275
-		logrus.Warnf("Engine labels with duplicate keys and conflicting values have been deprecated: %s", err)
261
+	// Check if duplicate label-keys with different values are found
262
+	newLabels, err := GetConflictFreeLabels(newConfig.Labels)
263
+	if err != nil {
264
+		return err
276 265
 	}
266
+	newConfig.Labels = newLabels
277 267
 
278 268
 	reload(newConfig)
279 269
 	return nil
... ...
@@ -9,6 +9,7 @@ import (
9 9
 	"github.com/docker/docker/daemon/discovery"
10 10
 	"github.com/docker/docker/internal/testutil"
11 11
 	"github.com/docker/docker/opts"
12
+	"github.com/gotestyourself/gotestyourself/fs"
12 13
 	"github.com/spf13/pflag"
13 14
 	"github.com/stretchr/testify/assert"
14 15
 )
... ...
@@ -459,3 +460,29 @@ func TestReloadBadDefaultConfig(t *testing.T) {
459 459
 	assert.Error(t, err)
460 460
 	testutil.ErrorContains(t, err, "unable to configure the Docker daemon with file")
461 461
 }
462
+
463
+func TestReloadWithConflictingLabels(t *testing.T) {
464
+	tempFile := fs.NewFile(t, "config", fs.WithContent(`{"labels":["foo=bar","foo=baz"]}`))
465
+	defer tempFile.Remove()
466
+	configFile := tempFile.Path()
467
+
468
+	var lbls []string
469
+	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
470
+	flags.String("config-file", configFile, "")
471
+	flags.StringSlice("labels", lbls, "")
472
+	err := Reload(configFile, flags, func(c *Config) {})
473
+	testutil.ErrorContains(t, err, "conflict labels for foo=baz and foo=bar")
474
+}
475
+
476
+func TestReloadWithDuplicateLabels(t *testing.T) {
477
+	tempFile := fs.NewFile(t, "config", fs.WithContent(`{"labels":["foo=the-same","foo=the-same"]}`))
478
+	defer tempFile.Remove()
479
+	configFile := tempFile.Path()
480
+
481
+	var lbls []string
482
+	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
483
+	flags.String("config-file", configFile, "")
484
+	flags.StringSlice("labels", lbls, "")
485
+	err := Reload(configFile, flags, func(c *Config) {})
486
+	assert.NoError(t, err)
487
+}
... ...
@@ -233,17 +233,6 @@ func (s *DockerDaemonSuite) TestRegistryMirrors(c *check.C) {
233 233
 	c.Assert(out, checker.Contains, fmt.Sprintf(" %s", registryMirror2))
234 234
 }
235 235
 
236
-// Test case for #24392
237
-func (s *DockerDaemonSuite) TestInfoLabels(c *check.C) {
238
-	testRequires(c, SameHostDaemon, DaemonIsLinux)
239
-
240
-	s.d.Start(c, "--label", `test.empty=`, "--label", `test.empty=`, "--label", `test.label="1"`, "--label", `test.label="2"`)
241
-
242
-	out, err := s.d.Cmd("info")
243
-	c.Assert(err, checker.IsNil)
244
-	c.Assert(out, checker.Contains, "WARNING: labels with duplicate keys and conflicting values have been deprecated")
245
-}
246
-
247 236
 func existingContainerStates(c *check.C) map[string]int {
248 237
 	out, _ := dockerCmd(c, "info", "--format", "{{json .}}")
249 238
 	var m map[string]interface{}