Service REST test: Remove pointless scaffolding

These fields don't add much value in actually proving it all works, and
they make the upcoming de-layering hard.
This commit is contained in:
Tim Hockin 2021-06-28 20:11:16 -07:00
parent 175f4f3387
commit 7e8882d189

View File

@ -61,10 +61,6 @@ var (
// in a completely different way. We should unify it. // in a completely different way. We should unify it.
type serviceStorage struct { type serviceStorage struct {
GottenID string
UpdatedID string
CreatedID string
DeletedID string
Service *api.Service Service *api.Service
ServiceList *api.ServiceList ServiceList *api.ServiceList
} }
@ -74,12 +70,15 @@ func (s *serviceStorage) NamespaceScoped() bool {
} }
func (s *serviceStorage) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { func (s *serviceStorage) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) {
s.GottenID = name
return s.Service, nil return s.Service, nil
} }
func (s *serviceStorage) GetService(ctx context.Context, name string, options *metav1.GetOptions) (*api.Service, error) { func getService(getter rest.Getter, ctx context.Context, name string, options *metav1.GetOptions) (*api.Service, error) {
return s.Service, nil obj, err := getter.Get(ctx, name, options)
if err != nil {
return nil, err
}
return obj.(*api.Service), nil
} }
func (s *serviceStorage) NewList() runtime.Object { func (s *serviceStorage) NewList() runtime.Object {
@ -116,7 +115,6 @@ func (s *serviceStorage) Create(ctx context.Context, obj runtime.Object, createV
return obj, nil return obj, nil
} }
svc := obj.(*api.Service) svc := obj.(*api.Service)
s.CreatedID = obj.(metav1.Object).GetName()
s.Service = svc.DeepCopy() s.Service = svc.DeepCopy()
s.Service.ResourceVersion = "1" s.Service.ResourceVersion = "1"
@ -134,16 +132,12 @@ func (s *serviceStorage) Update(ctx context.Context, name string, objInfo rest.U
return nil, false, err return nil, false, err
} }
if !dryrun.IsDryRun(options.DryRun) { if !dryrun.IsDryRun(options.DryRun) {
s.UpdatedID = name
s.Service = obj.(*api.Service) s.Service = obj.(*api.Service)
} }
return obj, false, nil return obj, false, nil
} }
func (s *serviceStorage) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { func (s *serviceStorage) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) {
if !dryrun.IsDryRun(options.DryRun) {
s.DeletedID = name
}
return s.Service, false, nil return s.Service, false, nil
} }
@ -346,7 +340,7 @@ func TestServiceRegistryCreate(t *testing.T) {
t.Errorf("Unexpected ClusterIP: %s", createdService.Spec.ClusterIPs[i]) t.Errorf("Unexpected ClusterIP: %s", createdService.Spec.ClusterIPs[i])
} }
} }
srv, err := registry.GetService(ctx, tc.svc.Name, &metav1.GetOptions{}) srv, err := getService(registry, ctx, tc.svc.Name, &metav1.GetOptions{})
if err != nil { if err != nil {
t.Errorf("unexpected error: %v", err) t.Errorf("unexpected error: %v", err)
} }
@ -412,7 +406,7 @@ func TestServiceRegistryCreateDryRun(t *testing.T) {
} }
} }
srv, err := registry.GetService(ctx, tc.svc.Name, &metav1.GetOptions{}) srv, err := getService(registry, ctx, tc.svc.Name, &metav1.GetOptions{})
if err != nil { if err != nil {
t.Errorf("unexpected error: %v", err) t.Errorf("unexpected error: %v", err)
} }
@ -442,7 +436,7 @@ func TestDryRunNodePort(t *testing.T) {
if storage.serviceNodePorts.Has(int(createdSvc.Spec.Ports[0].NodePort)) { if storage.serviceNodePorts.Has(int(createdSvc.Spec.Ports[0].NodePort)) {
t.Errorf("unexpected side effect: NodePort allocated") t.Errorf("unexpected side effect: NodePort allocated")
} }
srv, err := registry.GetService(ctx, svc.Name, &metav1.GetOptions{}) srv, err := getService(registry, ctx, svc.Name, &metav1.GetOptions{})
if err != nil { if err != nil {
t.Errorf("unexpected error: %v", err) t.Errorf("unexpected error: %v", err)
} }
@ -472,7 +466,7 @@ func TestDryRunNodePort(t *testing.T) {
t.Errorf("unexpected side effect: NodePort allocated") t.Errorf("unexpected side effect: NodePort allocated")
} }
} }
srv, err = registry.GetService(ctx, svc.Name, &metav1.GetOptions{}) srv, err = getService(registry, ctx, svc.Name, &metav1.GetOptions{})
if err != nil { if err != nil {
t.Errorf("unexpected error: %v", err) t.Errorf("unexpected error: %v", err)
} }
@ -564,7 +558,7 @@ func TestServiceRegistryCreateMultiNodePortsService(t *testing.T) {
if !reflect.DeepEqual(serviceNodePorts, test.expectNodePorts) { if !reflect.DeepEqual(serviceNodePorts, test.expectNodePorts) {
t.Errorf("Expected %v, but got %v", test.expectNodePorts, serviceNodePorts) t.Errorf("Expected %v, but got %v", test.expectNodePorts, serviceNodePorts)
} }
srv, err := registry.GetService(ctx, test.name, &metav1.GetOptions{}) srv, err := getService(registry, ctx, test.name, &metav1.GetOptions{})
if err != nil { if err != nil {
t.Errorf("unexpected error: %v", err) t.Errorf("unexpected error: %v", err)
} }
@ -634,9 +628,6 @@ func TestServiceRegistryUpdate(t *testing.T) {
if updatedService.Name != "foo" { if updatedService.Name != "foo" {
t.Errorf("Expected foo, but got %v", updatedService.Name) t.Errorf("Expected foo, but got %v", updatedService.Name)
} }
if e, a := "foo", registry.UpdatedID; e != a {
t.Errorf("Expected %v, but got %v", e, a)
}
} }
func TestServiceRegistryUpdateDryRun(t *testing.T) { func TestServiceRegistryUpdateDryRun(t *testing.T) {
@ -667,9 +658,6 @@ func TestServiceRegistryUpdateDryRun(t *testing.T) {
if storage.serviceNodePorts.Has(int(svc.Spec.Ports[0].NodePort)) { if storage.serviceNodePorts.Has(int(svc.Spec.Ports[0].NodePort)) {
t.Errorf("unexpected side effect: NodePort allocated") t.Errorf("unexpected side effect: NodePort allocated")
} }
if e, a := "", registry.UpdatedID; e != a {
t.Errorf("Expected %q, but got %q", e, a)
}
// Test dry run update request external name to cluster ip // Test dry run update request external name to cluster ip
new2 := svc.DeepCopy() new2 := svc.DeepCopy()
@ -760,7 +748,7 @@ func TestServiceRegistryExternalService(t *testing.T) {
if err != nil { if err != nil {
t.Errorf("Failed to create service: %#v", err) t.Errorf("Failed to create service: %#v", err)
} }
srv, err := registry.GetService(ctx, svc.Name, &metav1.GetOptions{}) srv, err := getService(registry, ctx, svc.Name, &metav1.GetOptions{})
if err != nil { if err != nil {
t.Errorf("Unexpected error: %v", err) t.Errorf("Unexpected error: %v", err)
} }
@ -832,7 +820,7 @@ func TestAllocateLoadBalancerNodePorts(t *testing.T) {
} }
t.Errorf("%s; Failed to create service: %#v", tc.name, err) t.Errorf("%s; Failed to create service: %#v", tc.name, err)
} }
srv, err := registry.GetService(ctx, tc.svc.Name, &metav1.GetOptions{}) srv, err := getService(registry, ctx, tc.svc.Name, &metav1.GetOptions{})
if err != nil { if err != nil {
t.Errorf("%s; Unexpected error: %v", tc.name, err) t.Errorf("%s; Unexpected error: %v", tc.name, err)
} }
@ -862,15 +850,15 @@ func TestServiceRegistryDelete(t *testing.T) {
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
storage.Delete(ctx, svc.Name, rest.ValidateAllObjectFunc, &metav1.DeleteOptions{}) _, _, err = storage.Delete(ctx, svc.Name, rest.ValidateAllObjectFunc, &metav1.DeleteOptions{})
if e, a := "foo", registry.DeletedID; e != a { if err != nil {
t.Errorf("Expected %v, but got %v", e, a) t.Fatalf("unexpected error: %v", err)
} }
} }
func TestServiceRegistryDeleteDryRun(t *testing.T) { func TestServiceRegistryDeleteDryRun(t *testing.T) {
ctx := genericapirequest.NewDefaultContext() ctx := genericapirequest.NewDefaultContext()
storage, registry, server := NewTestREST(t, nil, singleStackIPv4) storage, _, server := NewTestREST(t, nil, singleStackIPv4)
defer server.Terminate(t) defer server.Terminate(t)
// Test dry run delete request with cluster ip // Test dry run delete request with cluster ip
@ -890,9 +878,6 @@ func TestServiceRegistryDeleteDryRun(t *testing.T) {
if err != nil { if err != nil {
t.Fatalf("Expected no error: %v", err) t.Fatalf("Expected no error: %v", err)
} }
if e, a := "", registry.DeletedID; e != a {
t.Errorf("Expected %v, but got %v", e, a)
}
if !storage.serviceIPAllocatorsByFamily[storage.defaultServiceIPFamily].Has(net.ParseIP(createdSvc.Spec.ClusterIP)) { if !storage.serviceIPAllocatorsByFamily[storage.defaultServiceIPFamily].Has(net.ParseIP(createdSvc.Spec.ClusterIP)) {
t.Errorf("unexpected side effect: ip unallocated") t.Errorf("unexpected side effect: ip unallocated")
} }
@ -917,9 +902,6 @@ func TestServiceRegistryDeleteDryRun(t *testing.T) {
if err != nil { if err != nil {
t.Fatalf("Expected no error: %v", err) t.Fatalf("Expected no error: %v", err)
} }
if e, a := "", registry.DeletedID; e != a {
t.Errorf("Expected %v, but got %v", e, a)
}
if !storage.serviceNodePorts.Has(int(createdSvc.Spec.Ports[0].NodePort)) { if !storage.serviceNodePorts.Has(int(createdSvc.Spec.Ports[0].NodePort)) {
t.Errorf("unexpected side effect: NodePort unallocated") t.Errorf("unexpected side effect: NodePort unallocated")
} }
@ -930,7 +912,7 @@ func TestDualStackServiceRegistryDeleteDryRun(t *testing.T) {
// dry run for non dualstack // dry run for non dualstack
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.IPv6DualStack, true)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.IPv6DualStack, true)()
dualstack_storage, dualstack_registry, dualstack_server := NewTestREST(t, nil, []api.IPFamily{api.IPv4Protocol, api.IPv6Protocol}) dualstack_storage, _, dualstack_server := NewTestREST(t, nil, []api.IPFamily{api.IPv4Protocol, api.IPv6Protocol})
defer dualstack_server.Terminate(t) defer dualstack_server.Terminate(t)
// Test dry run delete request with cluster ip // Test dry run delete request with cluster ip
dualstack_svc := svctest.MakeService("foo", dualstack_svc := svctest.MakeService("foo",
@ -947,9 +929,6 @@ func TestDualStackServiceRegistryDeleteDryRun(t *testing.T) {
if err != nil { if err != nil {
t.Fatalf("Expected no error: %v", err) t.Fatalf("Expected no error: %v", err)
} }
if e, a := "", dualstack_registry.DeletedID; e != a {
t.Errorf("Expected %v, but got %v", e, a)
}
for i, family := range dualstack_svc.Spec.IPFamilies { for i, family := range dualstack_svc.Spec.IPFamilies {
if !dualstack_storage.serviceIPAllocatorsByFamily[family].Has(net.ParseIP(dualstack_svc.Spec.ClusterIPs[i])) { if !dualstack_storage.serviceIPAllocatorsByFamily[family].Has(net.ParseIP(dualstack_svc.Spec.ClusterIPs[i])) {
t.Errorf("unexpected side effect: ip unallocated %v", dualstack_svc.Spec.ClusterIPs[i]) t.Errorf("unexpected side effect: ip unallocated %v", dualstack_svc.Spec.ClusterIPs[i])
@ -966,9 +945,9 @@ func TestServiceRegistryDeleteExternal(t *testing.T) {
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
storage.Delete(ctx, svc.Name, rest.ValidateAllObjectFunc, &metav1.DeleteOptions{}) _, _, err = storage.Delete(ctx, svc.Name, rest.ValidateAllObjectFunc, &metav1.DeleteOptions{})
if e, a := "foo", registry.DeletedID; e != a { if err != nil {
t.Errorf("Expected %v, but got %v", e, a) t.Fatalf("Expected no error: %v", err)
} }
} }
@ -1033,8 +1012,9 @@ func TestServiceRegistryGet(t *testing.T) {
if err != nil { if err != nil {
t.Fatalf("error creating service: %v", err) t.Fatalf("error creating service: %v", err)
} }
storage.Get(ctx, "foo", &metav1.GetOptions{}) obj, _ := storage.Get(ctx, "foo", &metav1.GetOptions{})
if e, a := "foo", registry.GottenID; e != a { svc := obj.(*api.Service)
if e, a := "foo", svc.Name; e != a {
t.Errorf("Expected %v, but got %v", e, a) t.Errorf("Expected %v, but got %v", e, a)
} }
} }