From 4d8598ad0506b29c12632c1b8ed92eb58fc2f0e2 Mon Sep 17 00:00:00 2001
From: Brian Goff <cpuguy83@gmail.com>
Date: Tue, 19 Sep 2017 14:34:41 -0400
Subject: [PATCH] Create labels when volume exists only remotely

Before this, if a volume exists in a driver but not in the local cache,
the store would just return a bare volume. This means that if a user
supplied options or labels, they will not get stored.

Instead only return early if we have the volume stored locally. Note
this could still have an issue with labels/opts passed in by the user
differing from what is stored, however this isn't really a new problem.

This fixes a problem where if there is a shared storage backend between
two docker nodes, a create on one node will have labels stored and a
create on the other node will not.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
---
 volume/store/store.go      | 26 ++++++++++++++------------
 volume/store/store_test.go | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/volume/store/store.go b/volume/store/store.go
index 5402c6bd9f..fd1ca616ca 100644
--- a/volume/store/store.go
+++ b/volume/store/store.go
@@ -385,35 +385,37 @@ func (s *VolumeStore) create(name, driverName string, opts, labels map[string]st
 	}
 
 	if v != nil {
-		return v, nil
+		// there is an existing volume, if we already have this stored locally, return it.
+		// TODO: there could be some inconsistent details such as labels here
+		if vv, _ := s.getNamed(v.Name()); vv != nil {
+			return vv, nil
+		}
 	}
 
 	// Since there isn't a specified driver name, let's see if any of the existing drivers have this volume name
 	if driverName == "" {
-		v, _ := s.getVolume(name)
+		v, _ = s.getVolume(name)
 		if v != nil {
 			return v, nil
 		}
 	}
 
 	vd, err := volumedrivers.CreateDriver(driverName)
-
 	if err != nil {
 		return nil, &OpErr{Op: "create", Name: name, Err: err}
 	}
 
 	logrus.Debugf("Registering new volume reference: driver %q, name %q", vd.Name(), name)
-
-	if v, _ := vd.Get(name); v != nil {
-		return v, nil
-	}
-	v, err = vd.Create(name, opts)
-	if err != nil {
-		if _, err := volumedrivers.ReleaseDriver(driverName); err != nil {
-			logrus.WithError(err).WithField("driver", driverName).Error("Error releasing reference to volume driver")
+	if v, _ = vd.Get(name); v == nil {
+		v, err = vd.Create(name, opts)
+		if err != nil {
+			if _, err := volumedrivers.ReleaseDriver(driverName); err != nil {
+				logrus.WithError(err).WithField("driver", driverName).Error("Error releasing reference to volume driver")
+			}
+			return nil, err
 		}
-		return nil, err
 	}
+
 	s.globalLock.Lock()
 	s.labels[name] = labels
 	s.options[name] = opts
diff --git a/volume/store/store_test.go b/volume/store/store_test.go
index f5f00255a1..eb78c85cbe 100644
--- a/volume/store/store_test.go
+++ b/volume/store/store_test.go
@@ -7,6 +7,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/docker/docker/volume"
 	"github.com/docker/docker/volume/drivers"
 	volumetestutils "github.com/docker/docker/volume/testutils"
 )
@@ -232,3 +233,36 @@ func TestDerefMultipleOfSameRef(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
+func TestCreateKeepOptsLabelsWhenExistsRemotely(t *testing.T) {
+	vd := volumetestutils.NewFakeDriver("fake")
+	volumedrivers.Register(vd, "fake")
+	dir, err := ioutil.TempDir("", "test-same-deref")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+	s, err := New(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Create a volume in the driver directly
+	if _, err := vd.Create("foo", nil); err != nil {
+		t.Fatal(err)
+	}
+
+	v, err := s.Create("foo", "fake", nil, map[string]string{"hello": "world"})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	switch dv := v.(type) {
+	case volume.DetailedVolume:
+		if dv.Labels()["hello"] != "world" {
+			t.Fatalf("labels don't match")
+		}
+	default:
+		t.Fatalf("got unexpected type: %T", v)
+	}
+}