diff --git a/pkg/api/service/testing/make.go b/pkg/api/service/testing/make.go index ed0d4ddf347..87b86930cf4 100644 --- a/pkg/api/service/testing/make.go +++ b/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 { diff --git a/pkg/registry/core/service/storage/rest_test.go b/pkg/registry/core/service/storage/rest_test.go index e060a0094d6..9d8156ff04a 100644 --- a/pkg/registry/core/service/storage/rest_test.go +++ b/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) - } -} diff --git a/pkg/registry/core/service/storage/storage_test.go b/pkg/registry/core/service/storage/storage_test.go index d5fad0fd5dd..e26899a6c21 100644 --- a/pkg/registry/core/service/storage/storage_test.go +++ b/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. - if tc.beforeUpdate != nil { - tc.beforeUpdate(t, storage) - } + // The update phase is optional. + if tc.update.svc != nil { + // 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. - obj, created, err := storage.Update(ctx, tc.update.svc.Name, - rest.DefaultUpdatedObjectInfo(tc.update.svc), rest.ValidateAllObjectFunc, - rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{}) - if tc.update.expectError && err != nil { - return + // Update the object to the new state and check the results. + obj, created, err := storage.Update(ctx, tc.update.svc.Name, + rest.DefaultUpdatedObjectInfo(tc.update.svc), rest.ValidateAllObjectFunc, + rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{}) + if tc.update.expectError && err != nil { + 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