Merge pull request #17378 from wojtek-t/remove_fake_client_from_master_test_2

Auto commit by PR queue bot

pkg/master/master_test.go

@@ -51,7 +51,6 @@ import (
 	"k8s.io/kubernetes/pkg/util"
 	"k8s.io/kubernetes/pkg/util/intstr"
 
-	"github.com/coreos/go-etcd/etcd"
 	"github.com/emicklei/go-restful"
 	"github.com/stretchr/testify/assert"
 )
@@ -459,9 +458,8 @@ type FooList struct {
 	Items []Foo `json:"items"`
 }
 
-func initThirdParty(t *testing.T, version string) (*Master, *tools.FakeEtcdClient, *httptest.Server, *assert.Assertions) {
+func initThirdParty(t *testing.T, version string) (*Master, *etcdtesting.EtcdTestServer, *httptest.Server, *assert.Assertions) {
 	master, etcdserver, _, assert := setUp(t)
-	defer etcdserver.Terminate(t)
 
 	master.thirdPartyResources = map[string]*thirdpartyresourcedatastorage.REST{}
 	api := &extensions.ThirdPartyResource{
@@ -476,17 +474,14 @@ func initThirdParty(t *testing.T, version string) (*Master, *tools.FakeEtcdClien
 		},
 	}
 	master.handlerContainer = restful.NewContainer()
-
+	master.thirdPartyStorage = etcdstorage.NewEtcdStorage(etcdserver.Client, testapi.Extensions.Codec(), etcdtest.PathPrefix())
-	fakeClient := tools.NewFakeEtcdClient(t)
-	fakeClient.Machines = []string{"http://machine1:4001", "http://machine2", "http://machine3:4003"}
-	master.thirdPartyStorage = etcdstorage.NewEtcdStorage(fakeClient, testapi.Extensions.Codec(), etcdtest.PathPrefix())
 
 	if !assert.NoError(master.InstallThirdPartyResource(api)) {
 		t.FailNow()
 	}
 
 	server := httptest.NewServer(master.handlerContainer.ServeMux)
-	return &master, fakeClient, server, assert
+	return &master, etcdserver, server, assert
 }
 
 func TestInstallThirdPartyAPIList(t *testing.T) {
@@ -531,54 +526,64 @@ func testInstallThirdPartyAPIListVersion(t *testing.T, version string) {
 		},
 	}
 	for _, test := range tests {
-		_, fakeClient, server, assert := initThirdParty(t, version)
+		func() {
-		defer server.Close()
+			_, etcdserver, server, assert := initThirdParty(t, version)
+			defer server.Close()
+			defer etcdserver.Terminate(t)
 
-		if test.items == nil {
+			client := etcdserver.Client
-			fakeClient.ExpectNotFoundGet(etcdtest.PathPrefix() + "/ThirdPartyResourceData/company.com/foos/default")
+			if test.items != nil {
-		} else {
+				setupEtcdList(client, "/ThirdPartyResourceData/company.com/foos/default", test.items)
-			setupEtcdList(fakeClient, "/ThirdPartyResourceData/company.com/foos/default", test.items)
-		}
-
-		resp, err := http.Get(server.URL + "/apis/company.com/" + version + "/namespaces/default/foos")
-		if !assert.NoError(err) {
-			return
-		}
-		defer resp.Body.Close()
-
-		assert.Equal(http.StatusOK, resp.StatusCode)
-
-		data, err := ioutil.ReadAll(resp.Body)
-		assert.NoError(err)
-
-		list := FooList{}
-		if err = json.Unmarshal(data, &list); err != nil {
-			t.Errorf("unexpected error: %v", err)
-		}
-
-		if test.items == nil {
-			if len(list.Items) != 0 {
-				t.Errorf("expected no items, saw: %v", list.Items)
 			}
-			continue
-		}
 
-		if len(list.Items) != len(test.items) {
+			resp, err := http.Get(server.URL + "/apis/company.com/" + version + "/namespaces/default/foos")
-			t.Errorf("unexpected length: %d vs %d", len(list.Items), len(test.items))
+			if !assert.NoError(err) {
-			continue
+				return
-		}
-		for ix := range list.Items {
-			// Copy things that are set dynamically on the server
-			expectedObj := test.items[ix]
-			expectedObj.SelfLink = list.Items[ix].SelfLink
-			expectedObj.Namespace = list.Items[ix].Namespace
-			expectedObj.UID = list.Items[ix].UID
-			expectedObj.CreationTimestamp = list.Items[ix].CreationTimestamp
-
-			if !reflect.DeepEqual(list.Items[ix], expectedObj) {
-				t.Errorf("expected:\n%#v\nsaw:\n%#v\n", expectedObj, list.Items[ix])
 			}
-		}
+			defer resp.Body.Close()
+
+			assert.Equal(http.StatusOK, resp.StatusCode)
+
+			data, err := ioutil.ReadAll(resp.Body)
+			assert.NoError(err)
+
+			list := FooList{}
+			if err = json.Unmarshal(data, &list); err != nil {
+				t.Errorf("unexpected error: %v", err)
+			}
+
+			if test.items == nil {
+				if len(list.Items) != 0 {
+					t.Errorf("expected no items, saw: %v", list.Items)
+				}
+				return
+			}
+
+			if len(list.Items) != len(test.items) {
+				t.Errorf("unexpected length: %d vs %d", len(list.Items), len(test.items))
+				return
+			}
+			// The order of elements in LIST is not guaranteed.
+			mapping := make(map[string]int)
+			for ix := range test.items {
+				mapping[test.items[ix].Name] = ix
+			}
+			for ix := range list.Items {
+				// Copy things that are set dynamically on the server
+				expectedObj := test.items[mapping[list.Items[ix].Name]]
+				expectedObj.SelfLink = list.Items[ix].SelfLink
+				expectedObj.ResourceVersion = list.Items[ix].ResourceVersion
+				expectedObj.Namespace = list.Items[ix].Namespace
+				expectedObj.UID = list.Items[ix].UID
+				expectedObj.CreationTimestamp = list.Items[ix].CreationTimestamp
+
+				// We endure the order of items by sorting them (using 'mapping')
+				// so that this function passes.
+				if !reflect.DeepEqual(list.Items[ix], expectedObj) {
+					t.Errorf("expected:\n%#v\nsaw:\n%#v\n", expectedObj, list.Items[ix])
+				}
+			}
+		}()
 	}
 }
 
@@ -594,6 +599,7 @@ func encodeToThirdParty(name string, obj interface{}) ([]byte, error) {
 	return testapi.Extensions.Codec().Encode(&thirdPartyData)
 }
 
+// TODO: Convert to storage.Interface.
 func storeToEtcd(client tools.EtcdClient, path, name string, obj interface{}) error {
 	data, err := encodeToThirdParty(name, obj)
 	if err != nil {
@@ -603,20 +609,13 @@ func storeToEtcd(client tools.EtcdClient, path, name string, obj interface{}) er
 	return err
 }
 
-func setupEtcdList(fakeClient *tools.FakeEtcdClient, path string, list []Foo) error {
+// TODO: Convert to storage.Interface.
-	resp := tools.EtcdResponseWithError{
+func setupEtcdList(client tools.EtcdClient, path string, list []Foo) error {
-		R: &etcd.Response{
-			Node: &etcd.Node{},
-		},
-	}
 	for _, obj := range list {
-		data, err := encodeToThirdParty(obj.Name, obj)
+		if err := storeToEtcd(client, path+"/"+obj.Name, obj.Name, obj); err != nil {
-		if err != nil {
 			return err
 		}
-		resp.R.Node.Nodes = append(resp.R.Node.Nodes, &etcd.Node{Value: string(data)})
 	}
-	fakeClient.Data[etcdtest.PathPrefix()+path] = resp
 	return nil
 }
 
@@ -641,9 +640,11 @@ func TestInstallThirdPartyAPIGet(t *testing.T) {
 }
 
 func testInstallThirdPartyAPIGetVersion(t *testing.T, version string) {
-	_, fakeClient, server, assert := initThirdParty(t, version)
+	_, etcdserver, server, assert := initThirdParty(t, version)
 	defer server.Close()
+	defer etcdserver.Terminate(t)
 
+	client := etcdserver.Client
 	expectedObj := Foo{
 		ObjectMeta: api.ObjectMeta{
 			Name: "test",
@@ -655,7 +656,7 @@ func testInstallThirdPartyAPIGetVersion(t *testing.T, version string) {
 		SomeField:  "test field",
 		OtherField: 10,
 	}
-	if !assert.NoError(storeToEtcd(fakeClient, "/ThirdPartyResourceData/company.com/foos/default/test", "test", expectedObj)) {
+	if !assert.NoError(storeToEtcd(client, "/ThirdPartyResourceData/company.com/foos/default/test", "test", expectedObj)) {
 		t.FailNow()
 		return
 	}
@@ -674,6 +675,7 @@ func testInstallThirdPartyAPIGetVersion(t *testing.T, version string) {
 	}
 	// Fill in data that the apiserver injects
 	expectedObj.SelfLink = item.SelfLink
+	expectedObj.ResourceVersion = item.ResourceVersion
 	if !assert.True(reflect.DeepEqual(item, expectedObj)) {
 		t.Errorf("expected:\n%#v\nsaw:\n%#v\n", expectedObj, item)
 	}
@@ -686,9 +688,11 @@ func TestInstallThirdPartyAPIPost(t *testing.T) {
 }
 
 func testInstallThirdPartyAPIPostForVersion(t *testing.T, version string) {
-	_, fakeClient, server, assert := initThirdParty(t, version)
+	_, etcdserver, server, assert := initThirdParty(t, version)
 	defer server.Close()
+	defer etcdserver.Terminate(t)
 
+	client := etcdserver.Client
 	inputObj := Foo{
 		ObjectMeta: api.ObjectMeta{
 			Name: "test",
@@ -719,6 +723,7 @@ func testInstallThirdPartyAPIPostForVersion(t *testing.T, version string) {
 	// fill in fields set by the apiserver
 	expectedObj := inputObj
 	expectedObj.SelfLink = item.SelfLink
+	expectedObj.ResourceVersion = item.ResourceVersion
 	expectedObj.Namespace = item.Namespace
 	expectedObj.UID = item.UID
 	expectedObj.CreationTimestamp = item.CreationTimestamp
@@ -726,7 +731,7 @@ func testInstallThirdPartyAPIPostForVersion(t *testing.T, version string) {
 		t.Errorf("expected:\n%v\nsaw:\n%v\n", expectedObj, item)
 	}
 
-	etcdResp, err := fakeClient.Get(etcdtest.PathPrefix()+"/ThirdPartyResourceData/company.com/foos/default/test", false, false)
+	etcdResp, err := client.Get(etcdtest.PathPrefix()+"/ThirdPartyResourceData/company.com/foos/default/test", false, false)
 	if !assert.NoError(err) {
 		t.FailNow()
 	}
@@ -754,9 +759,11 @@ func TestInstallThirdPartyAPIDelete(t *testing.T) {
 }
 
 func testInstallThirdPartyAPIDeleteVersion(t *testing.T, version string) {
-	_, fakeClient, server, assert := initThirdParty(t, version)
+	_, etcdserver, server, assert := initThirdParty(t, version)
 	defer server.Close()
+	defer etcdserver.Terminate(t)
 
+	client := etcdserver.Client
 	expectedObj := Foo{
 		ObjectMeta: api.ObjectMeta{
 			Name:      "test",
@@ -768,7 +775,7 @@ func testInstallThirdPartyAPIDeleteVersion(t *testing.T, version string) {
 		SomeField:  "test field",
 		OtherField: 10,
 	}
-	if !assert.NoError(storeToEtcd(fakeClient, "/ThirdPartyResourceData/company.com/foos/default/test", "test", expectedObj)) {
+	if !assert.NoError(storeToEtcd(client, "/ThirdPartyResourceData/company.com/foos/default/test", "test", expectedObj)) {
 		t.FailNow()
 		return
 	}
@@ -785,6 +792,7 @@ func testInstallThirdPartyAPIDeleteVersion(t *testing.T, version string) {
 
 	// Fill in fields set by the apiserver
 	expectedObj.SelfLink = item.SelfLink
+	expectedObj.ResourceVersion = item.ResourceVersion
 	expectedObj.Namespace = item.Namespace
 	if !assert.True(reflect.DeepEqual(item, expectedObj)) {
 		t.Errorf("expected:\n%v\nsaw:\n%v\n", expectedObj, item)
@@ -804,10 +812,10 @@ func testInstallThirdPartyAPIDeleteVersion(t *testing.T, version string) {
 
 	assert.Equal(http.StatusNotFound, resp.StatusCode)
 
-	expectDeletedKeys := []string{etcdtest.PathPrefix() + "/ThirdPartyResourceData/company.com/foos/default/test"}
+	expectedDeletedKey := etcdtest.AddPrefix("ThirdPartyResourceData/company.com/foos/default/test")
-
+	_, err = client.Get(expectedDeletedKey, false, false)
-	if !assert.True(reflect.DeepEqual(fakeClient.DeletedKeys, expectDeletedKeys)) {
+	if !etcdstorage.IsEtcdNotFound(err) {
-		t.Errorf("unexpected deleted keys: %v", fakeClient.DeletedKeys)
+		t.Errorf("expected deletion didn't happen: %v", err)
 	}
 }
 
@@ -827,9 +835,11 @@ func TestInstallThirdPartyResourceRemove(t *testing.T) {
 }
 
 func testInstallThirdPartyResourceRemove(t *testing.T, version string) {
-	master, fakeClient, server, assert := initThirdParty(t, version)
+	master, etcdserver, server, assert := initThirdParty(t, version)
 	defer server.Close()
+	defer etcdserver.Terminate(t)
 
+	client := etcdserver.Client
 	expectedObj := Foo{
 		ObjectMeta: api.ObjectMeta{
 			Name: "test",
@@ -840,19 +850,17 @@ func testInstallThirdPartyResourceRemove(t *testing.T, version string) {
 		SomeField:  "test field",
 		OtherField: 10,
 	}
-	if !assert.NoError(storeToEtcd(fakeClient, "/ThirdPartyResourceData/company.com/foos/default/test", "test", expectedObj)) {
+	if !assert.NoError(storeToEtcd(client, "/ThirdPartyResourceData/company.com/foos/default/test", "test", expectedObj)) {
 		t.FailNow()
 		return
 	}
 	secondObj := expectedObj
 	secondObj.Name = "bar"
-	if !assert.NoError(storeToEtcd(fakeClient, "/ThirdPartyResourceData/company.com/foos/default/bar", "bar", secondObj)) {
+	if !assert.NoError(storeToEtcd(client, "/ThirdPartyResourceData/company.com/foos/default/bar", "bar", secondObj)) {
 		t.FailNow()
 		return
 	}
 
-	setupEtcdList(fakeClient, "/ThirdPartyResourceData/company.com/foos/default", []Foo{expectedObj, secondObj})
-
 	resp, err := http.Get(server.URL + "/apis/company.com/" + version + "/namespaces/default/foos/test")
 	if !assert.NoError(err) {
 		t.FailNow()
@@ -886,12 +894,16 @@ func testInstallThirdPartyResourceRemove(t *testing.T, version string) {
 	if resp.StatusCode != http.StatusNotFound {
 		t.Errorf("unexpected status: %v", resp)
 	}
-	expectDeletedKeys := []string{
+
-		etcdtest.PathPrefix() + "/ThirdPartyResourceData/company.com/foos/default/test",
+	expectedDeletedKeys := []string{
-		etcdtest.PathPrefix() + "/ThirdPartyResourceData/company.com/foos/default/bar",
+		etcdtest.AddPrefix("/ThirdPartyResourceData/company.com/foos/default/test"),
+		etcdtest.AddPrefix("/ThirdPartyResourceData/company.com/foos/default/bar"),
 	}
-	if !assert.True(reflect.DeepEqual(fakeClient.DeletedKeys, expectDeletedKeys)) {
+	for _, key := range expectedDeletedKeys {
-		t.Errorf("unexpected deleted keys: %v", fakeClient.DeletedKeys)
+		_, err := client.Get(key, false, false)
+		if !etcdstorage.IsEtcdNotFound(err) {
+			t.Errorf("expected deletion didn't happen: %v", err)
+		}
 	}
 	installed := master.ListThirdPartyResources()
 	if len(installed) != 0 {