Merge pull request #103379 from thockin/rest-hooks-use-by-svc-prep

A collection of Service REST cleanups

pkg/api/endpoints/testing/make.go

@@ -0,0 +1,70 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package testing
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	api "k8s.io/kubernetes/pkg/apis/core"
+)
+
+// Tweak is a function that modifies a Endpoints.
+type Tweak func(*api.Endpoints)
+
+// MakeEndpoints helps construct Endpoints objects (which pass API validation)
+// more legibly and tersely than a Go struct definition.
+func MakeEndpoints(name string, addrs []api.EndpointAddress, ports []api.EndpointPort, tweaks ...Tweak) *api.Endpoints {
+	// NOTE: Any field that would be populated by defaulting needs to be
+	// present and valid here.
+	eps := &api.Endpoints{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: metav1.NamespaceDefault,
+		},
+		Subsets: []api.EndpointSubset{{
+			Addresses: addrs,
+			Ports:     ports,
+		}},
+	}
+
+	for _, tweak := range tweaks {
+		tweak(eps)
+	}
+
+	return eps
+}
+
+// MakeEndpointAddress helps construct EndpointAddress objects which pass API
+// validation.
+func MakeEndpointAddress(ip string, pod string) api.EndpointAddress {
+	return api.EndpointAddress{
+		IP: ip,
+		TargetRef: &api.ObjectReference{
+			Name:      pod,
+			Namespace: metav1.NamespaceDefault,
+		},
+	}
+}
+
+// MakeEndpointPort helps construct EndpointPort objects which pass API
+// validation.
+func MakeEndpointPort(name string, port int) api.EndpointPort {
+	return api.EndpointPort{
+		Name: name,
+		Port: int32(port),
+	}
+}

pkg/api/service/testing/make.go

@@ -0,0 +1,142 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package testing
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+
+	api "k8s.io/kubernetes/pkg/apis/core"
+)
+
+// Tweak is a function that modifies a Service.
+type Tweak func(*api.Service)
+
+// MakeService helps construct Service objects (which pass API validation) more
+// legibly and tersely than a Go struct definition.  By default this produces
+// a ClusterIP service with a single port and a trivial selector.  The caller
+// can pass any number of tweak functions to further modify the result.
+func MakeService(name string, tweaks ...Tweak) *api.Service {
+	// NOTE: Any field that would be populated by defaulting needs to be
+	// present and valid here.
+	svc := &api.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: metav1.NamespaceDefault,
+		},
+		Spec: api.ServiceSpec{
+			Selector:        map[string]string{"k": "v"},
+			SessionAffinity: api.ServiceAffinityNone,
+		},
+	}
+	// Default to ClusterIP
+	SetTypeClusterIP(svc)
+	// Default to 1 port
+	SetPorts(MakeServicePort("", 93, intstr.FromInt(76), api.ProtocolTCP))(svc)
+
+	for _, tweak := range tweaks {
+		tweak(svc)
+	}
+
+	return svc
+}
+
+// SetTypeClusterIP sets the service type to ClusterIP and clears other fields.
+func SetTypeClusterIP(svc *api.Service) {
+	svc.Spec.Type = api.ServiceTypeClusterIP
+	for i := range svc.Spec.Ports {
+		svc.Spec.Ports[i].NodePort = 0
+	}
+	svc.Spec.ExternalName = ""
+	svc.Spec.ExternalTrafficPolicy = ""
+}
+
+// SetTypeNodePort sets the service type to NodePort and clears other fields.
+func SetTypeNodePort(svc *api.Service) {
+	svc.Spec.Type = api.ServiceTypeNodePort
+	svc.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeCluster
+	svc.Spec.ExternalName = ""
+}
+
+// SetTypeLoadBalancer sets the service type to LoadBalancer and clears other fields.
+func SetTypeLoadBalancer(svc *api.Service) {
+	svc.Spec.Type = api.ServiceTypeLoadBalancer
+	svc.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeCluster
+	svc.Spec.ExternalName = ""
+}
+
+// SetTypeExternalName sets the service type to ExternalName and clears other fields.
+func SetTypeExternalName(svc *api.Service) {
+	svc.Spec.Type = api.ServiceTypeExternalName
+	svc.Spec.ExternalName = "example.com"
+	svc.Spec.ExternalTrafficPolicy = ""
+	svc.Spec.ClusterIP = ""
+	svc.Spec.ClusterIPs = nil
+}
+
+// SetPorts sets the service ports list.
+func SetPorts(ports ...api.ServicePort) Tweak {
+	return func(svc *api.Service) {
+		svc.Spec.Ports = ports
+	}
+}
+
+// MakeServicePort helps construct ServicePort objects which pass API
+// validation.
+func MakeServicePort(name string, port int, tgtPort intstr.IntOrString, proto api.Protocol) api.ServicePort {
+	return api.ServicePort{
+		Name:       name,
+		Port:       int32(port),
+		TargetPort: tgtPort,
+		Protocol:   proto,
+	}
+}
+
+// SetClusterIPs sets the service ClusterIP and ClusterIPs fields.
+func SetClusterIPs(ips ...string) Tweak {
+	return func(svc *api.Service) {
+		svc.Spec.ClusterIP = ips[0]
+		svc.Spec.ClusterIPs = ips
+	}
+}
+
+// SetIPFamilies sets the service IPFamilies field.
+func SetIPFamilies(families ...api.IPFamily) Tweak {
+	return func(svc *api.Service) {
+		svc.Spec.IPFamilies = families
+	}
+}
+
+// SetIPFamilyPolicy sets the service IPFamilyPolicy field.
+func SetIPFamilyPolicy(policy api.IPFamilyPolicyType) Tweak {
+	return func(svc *api.Service) {
+		svc.Spec.IPFamilyPolicy = &policy
+	}
+}
+
+// SetNodePorts sets the values for each node port, in order.  If less values
+// are specified than there are ports, the rest are untouched.
+func SetNodePorts(values ...int) Tweak {
+	return func(svc *api.Service) {
+		for i := range svc.Spec.Ports {
+			if i >= len(values) {
+				break
+			}
+			svc.Spec.Ports[i].NodePort = int32(values[i])
+		}
+	}
+}

pkg/registry/core/service/ipallocator/allocator.go

@@ -35,8 +35,7 @@ type Interface interface {
 	Release(net.IP) error
 	ForEach(func(net.IP))
 	CIDR() net.IPNet
-
-	// For testing
+	IPFamily() api.IPFamily
 	Has(ip net.IP) bool
 }
 
@@ -76,6 +75,8 @@ type Range struct {
 	base *big.Int
 	// max is the maximum size of the usable addresses in the range
 	max int
+	// family is the IP family of this range
+	family api.IPFamily
 
 	alloc allocator.Interface
 }
@@ -85,13 +86,16 @@ func NewAllocatorCIDRRange(cidr *net.IPNet, allocatorFactory allocator.Allocator
 	max := utilnet.RangeSize(cidr)
 	base := utilnet.BigForIP(cidr.IP)
 	rangeSpec := cidr.String()
+	var family api.IPFamily
 
 	if utilnet.IsIPv6CIDR(cidr) {
+		family = api.IPv6Protocol
 		// Limit the max size, since the allocator keeps a bitmap of that size.
 		if max > 65536 {
 			max = 65536
 		}
 	} else {
+		family = api.IPv4Protocol
 		// Don't use the IPv4 network's broadcast address.
 		max--
 	}
@@ -101,9 +105,10 @@ func NewAllocatorCIDRRange(cidr *net.IPNet, allocatorFactory allocator.Allocator
 	max--
 
 	r := Range{
-		net:  cidr,
-		base: base,
-		max:  maximum(0, int(max)),
+		net:    cidr,
+		base:   base,
+		max:    maximum(0, int(max)),
+		family: family,
 	}
 	var err error
 	r.alloc, err = allocatorFactory(r.max, rangeSpec)
@@ -219,6 +224,11 @@ func (r *Range) Has(ip net.IP) bool {
 	return r.alloc.Has(offset)
 }
 
+// IPFamily returns the IP family of this range.
+func (r *Range) IPFamily() api.IPFamily {
+	return r.family
+}
+
 // Snapshot saves the current state of the pool.
 func (r *Range) Snapshot(dst *api.RangeAllocation) error {
 	snapshottable, ok := r.alloc.(allocator.Snapshottable)

pkg/registry/core/service/ipallocator/allocator_test.go

@@ -28,6 +28,7 @@ func TestAllocate(t *testing.T) {
 	testCases := []struct {
 		name             string
 		cidr             string
+		family           api.IPFamily
 		free             int
 		released         string
 		outOfRange       []string
@@ -36,6 +37,7 @@ func TestAllocate(t *testing.T) {
 		{
 			name:     "IPv4",
 			cidr:     "192.168.1.0/24",
+			family:   api.IPv4Protocol,
 			free:     254,
 			released: "192.168.1.5",
 			outOfRange: []string{
@@ -49,6 +51,7 @@ func TestAllocate(t *testing.T) {
 		{
 			name:     "IPv6",
 			cidr:     "2001:db8:1::/48",
+			family:   api.IPv6Protocol,
 			free:     65535,
 			released: "2001:db8:1::5",
 			outOfRange: []string{
@@ -79,6 +82,10 @@ func TestAllocate(t *testing.T) {
 			t.Errorf("allocator returned a different cidr")
 		}
 
+		if r.IPFamily() != tc.family {
+			t.Errorf("allocator returned wrong IP family")
+		}
+
 		if f := r.Used(); f != 0 {
 			t.Errorf("Test %s unexpected used %d", tc.name, f)
 		}

pkg/registry/core/service/portallocator/allocator.go

@@ -34,8 +34,6 @@ type Interface interface {
 	AllocateNext() (int, error)
 	Release(int) error
 	ForEach(func(int))
-
-	// For testing
 	Has(int) bool
 }
 

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

@@ -612,7 +612,7 @@ func (rs *REST) allocClusterIPs(service *api.Service, toAlloc map[api.IPFamily]s
 		} else {
 			parsedIP := net.ParseIP(ip)
 			if err := allocator.Allocate(parsedIP); err != nil {
-				el := field.ErrorList{field.Invalid(field.NewPath("spec", "clusterIPs"), service.Spec.ClusterIPs, fmt.Sprintf("failed to allocated ip:%v with error:%v", ip, err))}
+				el := field.ErrorList{field.Invalid(field.NewPath("spec", "clusterIPs"), service.Spec.ClusterIPs, fmt.Sprintf("failed to allocate IP %v: %v", ip, err))}
 				return allocated, errors.NewInvalid(api.Kind("Service"), service.Name, el)
 			}
 			allocated[family] = ip

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

@@ -45,7 +45,7 @@ type GenericREST struct {
 	secondaryFamily *api.IPFamily
 }
 
-// NewREST returns a RESTStorage object that will work against services.
+// NewGenericREST returns a RESTStorage object that will work against services.
 func NewGenericREST(optsGetter generic.RESTOptionsGetter, serviceCIDR net.IPNet, hasSecondary bool) (*GenericREST, *StatusREST, error) {
 	strategy, _ := registry.StrategyForServiceCIDRs(serviceCIDR, hasSecondary)