DeepCopy() input objects in Service REST test

Since the PR to do this deeper in the stack was declined, we'll do it
ourselves.  This ensures that we don't accidentally mutate the input and
then compare that mutated form to the result (which caused previous test
failures).
This commit is contained in:
Tim Hockin 2021-08-22 12:30:47 -07:00
parent 4ac7c73b2e
commit 4718a0f214

View File

@ -17,6 +17,7 @@ limitations under the License.
package storage package storage
import ( import (
"context"
"fmt" "fmt"
"net" "net"
"reflect" "reflect"
@ -87,11 +88,11 @@ func portIsAllocated(t *testing.T, alloc portallocator.Interface, port int32) bo
return alloc.Has(int(port)) return alloc.Has(int(port))
} }
func newStorage(t *testing.T, ipFamilies []api.IPFamily) (*GenericREST, *StatusREST, *etcd3testing.EtcdTestServer) { func newStorage(t *testing.T, ipFamilies []api.IPFamily) (*wrapperRESTForTests, *StatusREST, *etcd3testing.EtcdTestServer) {
return newStorageWithPods(t, ipFamilies, nil, nil) return newStorageWithPods(t, ipFamilies, nil, nil)
} }
func newStorageWithPods(t *testing.T, ipFamilies []api.IPFamily, pods []api.Pod, endpoints []*api.Endpoints) (*GenericREST, *StatusREST, *etcd3testing.EtcdTestServer) { func newStorageWithPods(t *testing.T, ipFamilies []api.IPFamily, pods []api.Pod, endpoints []*api.Endpoints) (*wrapperRESTForTests, *StatusREST, *etcd3testing.EtcdTestServer) {
etcdStorage, server := registrytest.NewEtcdStorage(t, "") etcdStorage, server := registrytest.NewEtcdStorage(t, "")
restOptions := generic.RESTOptions{ restOptions := generic.RESTOptions{
StorageConfig: etcdStorage.ForResource(schema.GroupResource{Resource: "services"}), StorageConfig: etcdStorage.ForResource(schema.GroupResource{Resource: "services"}),
@ -158,7 +159,21 @@ func newStorageWithPods(t *testing.T, ipFamilies []api.IPFamily, pods []api.Pod,
if err != nil { if err != nil {
t.Fatalf("unexpected error from REST storage: %v", err) t.Fatalf("unexpected error from REST storage: %v", err)
} }
return serviceStorage, statusStorage, server return &wrapperRESTForTests{serviceStorage}, statusStorage, server
}
// wrapperRESTForTests is a *trivial* wrapper for the real REST, which allows us to do
// things that are specifically to enhance test safety.
type wrapperRESTForTests struct {
*GenericREST
}
func (f *wrapperRESTForTests) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
// Making a DeepCopy here ensures that any in-place mutations of the input
// are not going to propagate to verification code, which used to happen
// resulting in tests that passed when they shouldn't have.
obj = obj.DeepCopyObject()
return f.GenericREST.Create(ctx, obj, createValidation, options)
} }
// This is used in generic registry tests. // This is used in generic registry tests.
@ -6301,14 +6316,14 @@ func TestUpdatePatchAllocatedValues(t *testing.T) {
return proofs return proofs
} }
proveClusterIP := func(idx int, ip string) svcTestProof { proveClusterIP := func(idx int, ip string) svcTestProof {
return func(t *testing.T, storage *GenericREST, before, after *api.Service) { return func(t *testing.T, storage *wrapperRESTForTests, before, after *api.Service) {
if want, got := ip, after.Spec.ClusterIPs[idx]; want != got { if want, got := ip, after.Spec.ClusterIPs[idx]; want != got {
t.Errorf("wrong ClusterIPs[%d]: want %q, got %q", idx, want, got) t.Errorf("wrong ClusterIPs[%d]: want %q, got %q", idx, want, got)
} }
} }
} }
proveNodePort := func(idx int, port int32) svcTestProof { proveNodePort := func(idx int, port int32) svcTestProof {
return func(t *testing.T, storage *GenericREST, before, after *api.Service) { return func(t *testing.T, storage *wrapperRESTForTests, before, after *api.Service) {
got := after.Spec.Ports[idx].NodePort got := after.Spec.Ports[idx].NodePort
if port > 0 && got != port { if port > 0 && got != port {
t.Errorf("wrong Ports[%d].NodePort: want %d, got %d", idx, port, got) t.Errorf("wrong Ports[%d].NodePort: want %d, got %d", idx, port, got)
@ -6318,7 +6333,7 @@ func TestUpdatePatchAllocatedValues(t *testing.T) {
} }
} }
proveHCNP := func(port int32) svcTestProof { proveHCNP := func(port int32) svcTestProof {
return func(t *testing.T, storage *GenericREST, before, after *api.Service) { return func(t *testing.T, storage *wrapperRESTForTests, before, after *api.Service) {
got := after.Spec.HealthCheckNodePort got := after.Spec.HealthCheckNodePort
if port > 0 && got != port { if port > 0 && got != port {
t.Errorf("wrong HealthCheckNodePort: want %d, got %d", port, got) t.Errorf("wrong HealthCheckNodePort: want %d, got %d", port, got)
@ -6568,7 +6583,7 @@ func TestUpdateIPsFromSingleStack(t *testing.T) {
return proofs return proofs
} }
proveNumFamilies := func(n int) svcTestProof { proveNumFamilies := func(n int) svcTestProof {
return func(t *testing.T, storage *GenericREST, before, after *api.Service) { return func(t *testing.T, storage *wrapperRESTForTests, before, after *api.Service) {
t.Helper() t.Helper()
if got := len(after.Spec.IPFamilies); got != n { if got := len(after.Spec.IPFamilies); got != n {
t.Errorf("wrong number of ipFamilies: expected %d, got %d", n, got) t.Errorf("wrong number of ipFamilies: expected %d, got %d", n, got)
@ -7328,7 +7343,7 @@ func TestUpdateIPsFromSingleStack(t *testing.T) {
expectClusterIPs: true, expectClusterIPs: true,
prove: prove(proveNumFamilies(1)), prove: prove(proveNumFamilies(1)),
}, },
beforeUpdate: func(t *testing.T, storage *GenericREST) { beforeUpdate: func(t *testing.T, storage *wrapperRESTForTests) {
alloc := storage.alloc.serviceIPAllocatorsByFamily[api.IPv6Protocol] alloc := storage.alloc.serviceIPAllocatorsByFamily[api.IPv6Protocol]
ip := "2000::1" ip := "2000::1"
if err := alloc.Allocate(netutils.ParseIPSloppy(ip)); err != nil { if err := alloc.Allocate(netutils.ParseIPSloppy(ip)); err != nil {
@ -8036,7 +8051,7 @@ func TestUpdateIPsFromSingleStack(t *testing.T) {
expectClusterIPs: true, expectClusterIPs: true,
prove: prove(proveNumFamilies(1)), prove: prove(proveNumFamilies(1)),
}, },
beforeUpdate: func(t *testing.T, storage *GenericREST) { beforeUpdate: func(t *testing.T, storage *wrapperRESTForTests) {
alloc := storage.alloc.serviceIPAllocatorsByFamily[api.IPv4Protocol] alloc := storage.alloc.serviceIPAllocatorsByFamily[api.IPv4Protocol]
ip := "10.0.0.1" ip := "10.0.0.1"
if err := alloc.Allocate(netutils.ParseIPSloppy(ip)); err != nil { if err := alloc.Allocate(netutils.ParseIPSloppy(ip)); err != nil {
@ -8280,7 +8295,7 @@ func TestUpdateIPsFromDualStack(t *testing.T) {
return proofs return proofs
} }
proveNumFamilies := func(n int) svcTestProof { proveNumFamilies := func(n int) svcTestProof {
return func(t *testing.T, storage *GenericREST, before, after *api.Service) { return func(t *testing.T, storage *wrapperRESTForTests, before, after *api.Service) {
t.Helper() t.Helper()
if got := len(after.Spec.IPFamilies); got != n { if got := len(after.Spec.IPFamilies); got != n {
t.Errorf("wrong number of ipFamilies: expected %d, got %d", n, got) t.Errorf("wrong number of ipFamilies: expected %d, got %d", n, got)
@ -9889,7 +9904,7 @@ type svcTestCase struct {
prove []svcTestProof prove []svcTestProof
} }
type svcTestProof func(t *testing.T, storage *GenericREST, before, after *api.Service) type svcTestProof func(t *testing.T, storage *wrapperRESTForTests, before, after *api.Service)
func callName(before, after *api.Service) string { func callName(before, after *api.Service) string {
if before == nil && after != nil { if before == nil && after != nil {
@ -9904,7 +9919,7 @@ func callName(before, after *api.Service) string {
panic("this test is broken: before and after are both nil") panic("this test is broken: before and after are both nil")
} }
func proveClusterIPsAllocated(t *testing.T, storage *GenericREST, before, after *api.Service) { func proveClusterIPsAllocated(t *testing.T, storage *wrapperRESTForTests, before, after *api.Service) {
t.Helper() t.Helper()
if sing, plur := after.Spec.ClusterIP, after.Spec.ClusterIPs[0]; sing != plur { if sing, plur := after.Spec.ClusterIP, after.Spec.ClusterIPs[0]; sing != plur {
@ -9980,7 +9995,7 @@ func proveClusterIPsAllocated(t *testing.T, storage *GenericREST, before, after
} }
} }
func proveClusterIPsDeallocated(t *testing.T, storage *GenericREST, before, after *api.Service) { func proveClusterIPsDeallocated(t *testing.T, storage *wrapperRESTForTests, before, after *api.Service) {
t.Helper() t.Helper()
if after != nil && after.Spec.ClusterIP != api.ClusterIPNone { if after != nil && after.Spec.ClusterIP != api.ClusterIPNone {
@ -10007,7 +10022,7 @@ func proveClusterIPsDeallocated(t *testing.T, storage *GenericREST, before, afte
} }
} }
func proveHeadless(t *testing.T, storage *GenericREST, before, after *api.Service) { func proveHeadless(t *testing.T, storage *wrapperRESTForTests, before, after *api.Service) {
t.Helper() t.Helper()
if sing, plur := after.Spec.ClusterIP, after.Spec.ClusterIPs[0]; sing != plur { if sing, plur := after.Spec.ClusterIP, after.Spec.ClusterIPs[0]; sing != plur {
@ -10018,7 +10033,7 @@ func proveHeadless(t *testing.T, storage *GenericREST, before, after *api.Servic
} }
} }
func proveNodePortsAllocated(t *testing.T, storage *GenericREST, before, after *api.Service) { func proveNodePortsAllocated(t *testing.T, storage *wrapperRESTForTests, before, after *api.Service) {
t.Helper() t.Helper()
for _, p := range after.Spec.Ports { for _, p := range after.Spec.Ports {
@ -10028,7 +10043,7 @@ func proveNodePortsAllocated(t *testing.T, storage *GenericREST, before, after *
} }
} }
func proveNodePortsDeallocated(t *testing.T, storage *GenericREST, before, after *api.Service) { func proveNodePortsDeallocated(t *testing.T, storage *wrapperRESTForTests, before, after *api.Service) {
t.Helper() t.Helper()
if after != nil { if after != nil {
@ -10048,7 +10063,7 @@ func proveNodePortsDeallocated(t *testing.T, storage *GenericREST, before, after
} }
} }
func proveHealthCheckNodePortAllocated(t *testing.T, storage *GenericREST, before, after *api.Service) { func proveHealthCheckNodePortAllocated(t *testing.T, storage *wrapperRESTForTests, before, after *api.Service) {
t.Helper() t.Helper()
if !portIsAllocated(t, storage.alloc.serviceNodePorts, after.Spec.HealthCheckNodePort) { if !portIsAllocated(t, storage.alloc.serviceNodePorts, after.Spec.HealthCheckNodePort) {
@ -10056,7 +10071,7 @@ func proveHealthCheckNodePortAllocated(t *testing.T, storage *GenericREST, befor
} }
} }
func proveHealthCheckNodePortDeallocated(t *testing.T, storage *GenericREST, before, after *api.Service) { func proveHealthCheckNodePortDeallocated(t *testing.T, storage *wrapperRESTForTests, before, after *api.Service) {
t.Helper() t.Helper()
if after != nil { if after != nil {
@ -10076,7 +10091,7 @@ type cudTestCase struct {
name string name string
line string // if not empty, will be logged with errors, use line() to set line string // if not empty, will be logged with errors, use line() to set
create svcTestCase create svcTestCase
beforeUpdate func(t *testing.T, storage *GenericREST) beforeUpdate func(t *testing.T, storage *wrapperRESTForTests)
update svcTestCase update svcTestCase
} }
@ -10380,7 +10395,7 @@ func TestVerifyEquiv(t *testing.T) {
} }
} }
func verifyExpectations(t *testing.T, storage *GenericREST, tc svcTestCase, before, after *api.Service) { func verifyExpectations(t *testing.T, storage *wrapperRESTForTests, tc svcTestCase, before, after *api.Service) {
t.Helper() t.Helper()
if tc.expectClusterIPs { if tc.expectClusterIPs {
@ -11341,7 +11356,7 @@ func TestFeatureInternalTrafficPolicy(t *testing.T) {
return proofs return proofs
} }
proveITP := func(want api.ServiceInternalTrafficPolicyType) svcTestProof { proveITP := func(want api.ServiceInternalTrafficPolicyType) svcTestProof {
return func(t *testing.T, storage *GenericREST, before, after *api.Service) { return func(t *testing.T, storage *wrapperRESTForTests, before, after *api.Service) {
t.Helper() t.Helper()
if got := after.Spec.InternalTrafficPolicy; got == nil { if got := after.Spec.InternalTrafficPolicy; got == nil {
if want != "" { if want != "" {