Svc REST: Add InternalTrafficPolicy tests

Remove older form.

pkg/api/service/testing/make.go

@@ -48,7 +48,8 @@ func MakeService(name string, tweaks ...Tweak) *api.Service {
 	SetTypeClusterIP(svc)
 	// Default to 1 port
 	SetPorts(MakeServicePort("", 93, intstr.FromInt(76), api.ProtocolTCP))(svc)
-	// Default internalTrafficPolicy to "Cluster"
+	// Default internalTrafficPolicy to "Cluster".  This probably should not
+	// apply to ExternalName, but it went into beta and is not worth breaking.
 	SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyCluster)(svc)
 
 	for _, tweak := range tweaks {

pkg/registry/core/service/storage/rest_test.go

@@ -23,11 +23,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	utilnet "k8s.io/apimachinery/pkg/util/net"
-	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/generic"
-	"k8s.io/apiserver/pkg/registry/rest"
 	etcd3testing "k8s.io/apiserver/pkg/storage/etcd3/testing"
-	svctest "k8s.io/kubernetes/pkg/api/service/testing"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	endpointstore "k8s.io/kubernetes/pkg/registry/core/endpoint/storage"
 	"k8s.io/kubernetes/pkg/registry/core/service/ipallocator"
@@ -139,67 +136,3 @@ func makePod(name string, ips ...string) api.Pod {
 
 	return p
 }
-
-// Validate the internalTrafficPolicy field when set to "Cluster" then updated to "Local"
-func TestServiceRegistryInternalTrafficPolicyClusterThenLocal(t *testing.T) {
-	ctx := genericapirequest.NewDefaultContext()
-	storage, server := NewTestREST(t, []api.IPFamily{api.IPv4Protocol})
-	defer server.Terminate(t)
-	svc := svctest.MakeService("internal-traffic-policy-cluster",
-		svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyCluster),
-	)
-	obj, err := storage.Create(ctx, svc, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
-	if obj == nil || err != nil {
-		t.Errorf("Unexpected failure creating service %v", err)
-	}
-
-	createdSvc := obj.(*api.Service)
-	if *createdSvc.Spec.InternalTrafficPolicy != api.ServiceInternalTrafficPolicyCluster {
-		t.Errorf("Expecting internalTrafficPolicy field to have value Cluster, got: %s", *createdSvc.Spec.InternalTrafficPolicy)
-	}
-
-	update := createdSvc.DeepCopy()
-	local := api.ServiceInternalTrafficPolicyLocal
-	update.Spec.InternalTrafficPolicy = &local
-
-	updatedSvc, _, errUpdate := storage.Update(ctx, update.Name, rest.DefaultUpdatedObjectInfo(update), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{})
-	if errUpdate != nil {
-		t.Fatalf("unexpected error during update %v", errUpdate)
-	}
-	updatedService := updatedSvc.(*api.Service)
-	if *updatedService.Spec.InternalTrafficPolicy != api.ServiceInternalTrafficPolicyLocal {
-		t.Errorf("Expected internalTrafficPolicy to be Local, got: %s", *updatedService.Spec.InternalTrafficPolicy)
-	}
-}
-
-// Validate the internalTrafficPolicy field when set to "Local" and then updated to "Cluster"
-func TestServiceRegistryInternalTrafficPolicyLocalThenCluster(t *testing.T) {
-	ctx := genericapirequest.NewDefaultContext()
-	storage, server := NewTestREST(t, []api.IPFamily{api.IPv4Protocol})
-	defer server.Terminate(t)
-	svc := svctest.MakeService("internal-traffic-policy-cluster",
-		svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyLocal),
-	)
-	obj, err := storage.Create(ctx, svc, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
-	if obj == nil || err != nil {
-		t.Errorf("Unexpected failure creating service %v", err)
-	}
-
-	createdSvc := obj.(*api.Service)
-	if *createdSvc.Spec.InternalTrafficPolicy != api.ServiceInternalTrafficPolicyLocal {
-		t.Errorf("Expecting internalTrafficPolicy field to have value Local, got: %s", *createdSvc.Spec.InternalTrafficPolicy)
-	}
-
-	update := createdSvc.DeepCopy()
-	cluster := api.ServiceInternalTrafficPolicyCluster
-	update.Spec.InternalTrafficPolicy = &cluster
-
-	updatedSvc, _, errUpdate := storage.Update(ctx, update.Name, rest.DefaultUpdatedObjectInfo(update), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{})
-	if errUpdate != nil {
-		t.Fatalf("unexpected error during update %v", errUpdate)
-	}
-	updatedService := updatedSvc.(*api.Service)
-	if *updatedService.Spec.InternalTrafficPolicy != api.ServiceInternalTrafficPolicyCluster {
-		t.Errorf("Expected internalTrafficPolicy to be Cluster, got: %s", *updatedService.Spec.InternalTrafficPolicy)
-	}
-}

pkg/registry/core/service/storage/storage_test.go

@@ -10121,40 +10121,45 @@ func helpTestCreateUpdateDeleteWithFamilies(t *testing.T, testCases []cudTestCas
 				return
 			}
 			verifyExpectations(t, storage, tc.create, tc.create.svc, createdSvc)
+			lastSvc := createdSvc
 
-			// Allow callers to do something between create and update.
+			// The update phase is optional.
-			if tc.beforeUpdate != nil {
+			if tc.update.svc != nil {
-				tc.beforeUpdate(t, storage)
+				// Allow callers to do something between create and update.
-			}
+				if tc.beforeUpdate != nil {
+					tc.beforeUpdate(t, storage)
+				}
 
-			// Update the object to the new state and check the results.
+				// Update the object to the new state and check the results.
-			obj, created, err := storage.Update(ctx, tc.update.svc.Name,
+				obj, created, err := storage.Update(ctx, tc.update.svc.Name,
-				rest.DefaultUpdatedObjectInfo(tc.update.svc), rest.ValidateAllObjectFunc,
+					rest.DefaultUpdatedObjectInfo(tc.update.svc), rest.ValidateAllObjectFunc,
-				rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{})
+					rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{})
-			if tc.update.expectError && err != nil {
+				if tc.update.expectError && err != nil {
-				return
+					return
+				}
+				if err != nil {
+					t.Fatalf("unexpected error updating service: %v", err)
+				}
+				if tc.update.expectError && err == nil {
+					t.Fatalf("unexpected success updating service")
+				}
+				if created {
+					t.Fatalf("unexpected create-on-update")
+				}
+				updatedSvc := obj.(*api.Service)
+				if !verifyEquiv(t, "update", &tc.update, updatedSvc) {
+					return
+				}
+				verifyExpectations(t, storage, tc.update, createdSvc, updatedSvc)
+				lastSvc = updatedSvc
 			}
-			if err != nil {
-				t.Fatalf("unexpected error updating service: %v", err)
-			}
-			if tc.update.expectError && err == nil {
-				t.Fatalf("unexpected success updating service")
-			}
-			if created {
-				t.Fatalf("unexpected create-on-update")
-			}
-			updatedSvc := obj.(*api.Service)
-			if !verifyEquiv(t, "update", &tc.update, updatedSvc) {
-				return
-			}
-			verifyExpectations(t, storage, tc.update, createdSvc, updatedSvc)
 
 			// Delete the object and check the results.
-			_, _, err = storage.Delete(ctx, tc.update.svc.Name, rest.ValidateAllObjectFunc, &metav1.DeleteOptions{})
+			_, _, err = storage.Delete(ctx, tc.create.svc.Name, rest.ValidateAllObjectFunc, &metav1.DeleteOptions{})
 			if err != nil {
 				t.Fatalf("unexpected error deleting service: %v", err)
 			}
-			verifyExpectations(t, storage, svcTestCase{ /* all false */ }, updatedSvc, nil)
+			verifyExpectations(t, storage, svcTestCase{ /* all false */ }, lastSvc, nil)
 		})
 	}
 }
@@ -11331,8 +11336,192 @@ func TestFeatureExternalTrafficPolicy(t *testing.T) {
 	helpTestCreateUpdateDelete(t, testCases)
 }
 
+func TestFeatureInternalTrafficPolicy(t *testing.T) {
+	prove := func(proofs ...svcTestProof) []svcTestProof {
+		return proofs
+	}
+	proveITP := func(want api.ServiceInternalTrafficPolicyType) svcTestProof {
+		return func(t *testing.T, storage *GenericREST, before, after *api.Service) {
+			t.Helper()
+			if got := after.Spec.InternalTrafficPolicy; got == nil {
+				if want != "" {
+					t.Errorf("internalTrafficPolicy was nil")
+				}
+			} else if *got != want {
+				if want == "" {
+					want = "nil"
+				}
+				t.Errorf("wrong internalTrafficPoilcy: expected %s, got %s", want, *got)
+			}
+		}
+	}
+
+	testCases := []cudTestCase{{
+		name: "ExternalName_policy:none-ExternalName_policy:Local",
+		create: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetTypeExternalName),
+			prove: prove(proveITP(api.ServiceInternalTrafficPolicyCluster)),
+		},
+		update: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetTypeExternalName,
+				svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyLocal)),
+			prove: prove(proveITP(api.ServiceInternalTrafficPolicyLocal)),
+		},
+	}, {
+		name: "ExternalName_policy:Cluster-ExternalName_policy:Local",
+		create: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetTypeExternalName,
+				svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyCluster)),
+			prove: prove(proveITP(api.ServiceInternalTrafficPolicyCluster)),
+		},
+		update: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetTypeExternalName,
+				svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyLocal)),
+			prove: prove(proveITP(api.ServiceInternalTrafficPolicyLocal)),
+		},
+	}, {
+		name: "ClusterIP_policy:none-ClusterIP_policy:Local",
+		create: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetTypeClusterIP),
+			expectClusterIPs: true,
+			prove:            prove(proveITP(api.ServiceInternalTrafficPolicyCluster)),
+		},
+		update: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetTypeClusterIP,
+				svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyLocal)),
+			expectClusterIPs: true,
+			prove:            prove(proveITP(api.ServiceInternalTrafficPolicyLocal)),
+		},
+	}, {
+		name: "ClusterIP_policy:Cluster-ClusterIP_policy:Local",
+		create: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetTypeClusterIP,
+				svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyCluster)),
+			expectClusterIPs: true,
+			prove:            prove(proveITP(api.ServiceInternalTrafficPolicyCluster)),
+		},
+		update: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetTypeClusterIP,
+				svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyLocal)),
+			expectClusterIPs: true,
+			prove:            prove(proveITP(api.ServiceInternalTrafficPolicyLocal)),
+		},
+	}, {
+		name: "NodePort_policy:none-NodePort_policy:Local",
+		create: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetTypeNodePort),
+			expectClusterIPs: true,
+			expectNodePorts:  true,
+			prove:            prove(proveITP(api.ServiceInternalTrafficPolicyCluster)),
+		},
+		update: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetTypeNodePort,
+				svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyLocal)),
+			expectClusterIPs: true,
+			expectNodePorts:  true,
+			prove:            prove(proveITP(api.ServiceInternalTrafficPolicyLocal)),
+		},
+	}, {
+		name: "NodePort_policy:Cluster-NodePort_policy:Local",
+		create: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetTypeNodePort,
+				svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyCluster)),
+			expectClusterIPs: true,
+			expectNodePorts:  true,
+			prove:            prove(proveITP(api.ServiceInternalTrafficPolicyCluster)),
+		},
+		update: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetTypeNodePort,
+				svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyLocal)),
+			expectClusterIPs: true,
+			expectNodePorts:  true,
+			prove:            prove(proveITP(api.ServiceInternalTrafficPolicyLocal)),
+		},
+	}, {
+		name: "LoadBalancer_policy:none-LoadBalancer_policy:Local",
+		create: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetTypeLoadBalancer),
+			expectClusterIPs: true,
+			expectNodePorts:  true,
+			prove:            prove(proveITP(api.ServiceInternalTrafficPolicyCluster)),
+		},
+		update: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetTypeLoadBalancer,
+				svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyLocal)),
+			expectClusterIPs: true,
+			expectNodePorts:  true,
+			prove:            prove(proveITP(api.ServiceInternalTrafficPolicyLocal)),
+		},
+	}, {
+		name: "LoadBalancer_policy:Cluster-LoadBalancer_policy:Local",
+		create: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetTypeLoadBalancer,
+				svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyCluster)),
+			expectClusterIPs: true,
+			expectNodePorts:  true,
+			prove:            prove(proveITP(api.ServiceInternalTrafficPolicyCluster)),
+		},
+		update: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetTypeLoadBalancer,
+				svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyLocal)),
+			expectClusterIPs: true,
+			expectNodePorts:  true,
+			prove:            prove(proveITP(api.ServiceInternalTrafficPolicyLocal)),
+		},
+	}, {
+		name: "Headless_policy:none-Headless_policy:Local",
+		create: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetHeadless),
+			expectHeadless: true,
+			prove:          prove(proveITP(api.ServiceInternalTrafficPolicyCluster)),
+		},
+		update: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetHeadless,
+				svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyLocal)),
+			expectHeadless: true,
+			prove:          prove(proveITP(api.ServiceInternalTrafficPolicyLocal)),
+		},
+	}, {
+		name: "Headless_policy:Cluster-Headless_policy:Local",
+		create: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetHeadless,
+				svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyCluster)),
+			expectHeadless: true,
+			prove:          prove(proveITP(api.ServiceInternalTrafficPolicyCluster)),
+		},
+		update: svcTestCase{
+			svc: svctest.MakeService("foo",
+				svctest.SetHeadless,
+				svctest.SetInternalTrafficPolicy(api.ServiceInternalTrafficPolicyLocal)),
+			expectHeadless: true,
+			prove:          prove(proveITP(api.ServiceInternalTrafficPolicyLocal)),
+		},
+	}}
+
+	helpTestCreateUpdateDelete(t, testCases)
+}
+
 // FIXME: externalIPs, lbip,
-// lbsourceranges, externalname, itp, PublishNotReadyAddresses,
+// lbsourceranges, externalname, PublishNotReadyAddresses,
 // ipfamilypolicy and list,
 // AllocateLoadBalancerNodePorts, LoadBalancerClass, status