diff --git a/pkg/registry/core/service/ipallocator/allocator.go b/pkg/registry/core/service/ipallocator/allocator.go index 7adb30101bb..fe25a30212c 100644 --- a/pkg/registry/core/service/ipallocator/allocator.go +++ b/pkg/registry/core/service/ipallocator/allocator.go @@ -33,6 +33,9 @@ type Interface interface { AllocateNext() (net.IP, error) Release(net.IP) error ForEach(func(net.IP)) + + // For testing + Has(ip net.IP) bool } var ( diff --git a/pkg/registry/core/service/portallocator/allocator.go b/pkg/registry/core/service/portallocator/allocator.go index 7badfe7ad4c..c73253f2dcb 100644 --- a/pkg/registry/core/service/portallocator/allocator.go +++ b/pkg/registry/core/service/portallocator/allocator.go @@ -34,6 +34,9 @@ type Interface interface { AllocateNext() (int, error) Release(int) error ForEach(func(int)) + + // For testing + Has(int) bool } var ( diff --git a/pkg/registry/core/service/portallocator/operation.go b/pkg/registry/core/service/portallocator/operation.go index 08d9d587815..00dd7f03cf4 100644 --- a/pkg/registry/core/service/portallocator/operation.go +++ b/pkg/registry/core/service/portallocator/operation.go @@ -28,7 +28,7 @@ package portallocator // ... // write(updatedOwner) /// op.Commit() -type portAllocationOperation struct { +type PortAllocationOperation struct { pa Interface allocated []int releaseDeferred []int @@ -36,8 +36,8 @@ type portAllocationOperation struct { } // Creates a portAllocationOperation, tracking a set of allocations & releases -func StartOperation(pa Interface) *portAllocationOperation { - op := &portAllocationOperation{} +func StartOperation(pa Interface) *PortAllocationOperation { + op := &PortAllocationOperation{} op.pa = pa op.allocated = []int{} op.releaseDeferred = []int{} @@ -46,14 +46,14 @@ func StartOperation(pa Interface) *portAllocationOperation { } // Will rollback unless marked as shouldRollback = false by a Commit(). Call from a defer block -func (op *portAllocationOperation) Finish() { +func (op *PortAllocationOperation) Finish() { if op.shouldRollback { op.Rollback() } } // (Try to) undo any operations we did -func (op *portAllocationOperation) Rollback() []error { +func (op *PortAllocationOperation) Rollback() []error { errors := []error{} for _, allocated := range op.allocated { @@ -72,7 +72,7 @@ func (op *portAllocationOperation) Rollback() []error { // (Try to) perform any deferred operations. // Note that even if this fails, we don't rollback; we always want to err on the side of over-allocation, // and Commit should be called _after_ the owner is written -func (op *portAllocationOperation) Commit() []error { +func (op *PortAllocationOperation) Commit() []error { errors := []error{} for _, release := range op.releaseDeferred { @@ -94,7 +94,7 @@ func (op *portAllocationOperation) Commit() []error { } // Allocates a port, and record it for future rollback -func (op *portAllocationOperation) Allocate(port int) error { +func (op *PortAllocationOperation) Allocate(port int) error { err := op.pa.Allocate(port) if err == nil { op.allocated = append(op.allocated, port) @@ -103,7 +103,7 @@ func (op *portAllocationOperation) Allocate(port int) error { } // Allocates a port, and record it for future rollback -func (op *portAllocationOperation) AllocateNext() (int, error) { +func (op *PortAllocationOperation) AllocateNext() (int, error) { port, err := op.pa.AllocateNext() if err == nil { op.allocated = append(op.allocated, port) @@ -112,6 +112,6 @@ func (op *portAllocationOperation) AllocateNext() (int, error) { } // Marks a port so that it will be released if this operation Commits -func (op *portAllocationOperation) ReleaseDeferred(port int) { +func (op *PortAllocationOperation) ReleaseDeferred(port int) { op.releaseDeferred = append(op.releaseDeferred, port) } diff --git a/pkg/registry/core/service/rest.go b/pkg/registry/core/service/rest.go index a369708579d..9195e9c5e5e 100644 --- a/pkg/registry/core/service/rest.go +++ b/pkg/registry/core/service/rest.go @@ -357,58 +357,45 @@ func (rs *REST) Update(ctx genericapirequest.Context, name string, objInfo rest. return nil, false, errors.NewInvalid(api.Kind("Service"), service.Name, errs) } + // TODO: this should probably move to strategy.PrepareForCreate() + releaseServiceIP := false + defer func() { + if releaseServiceIP { + if helper.IsServiceIPSet(service) { + rs.serviceIPs.Release(net.ParseIP(service.Spec.ClusterIP)) + } + } + }() + nodePortOp := portallocator.StartOperation(rs.serviceNodePorts) defer nodePortOp.Finish() - assignNodePorts := shouldAssignNodePorts(service) - - oldNodePorts := CollectServiceNodePorts(oldService) - - newNodePorts := []int{} - if assignNodePorts { - for i := range service.Spec.Ports { - servicePort := &service.Spec.Ports[i] - nodePort := int(servicePort.NodePort) - if nodePort != 0 { - if !contains(oldNodePorts, nodePort) { - err := nodePortOp.Allocate(nodePort) - if err != nil { - el := field.ErrorList{field.Invalid(field.NewPath("spec", "ports").Index(i).Child("nodePort"), nodePort, err.Error())} - return nil, false, errors.NewInvalid(api.Kind("Service"), service.Name, el) - } - } - } else { - nodePort, err = nodePortOp.AllocateNext() - if err != nil { - // TODO: what error should be returned here? It's not a - // field-level validation failure (the field is valid), and it's - // not really an internal error. - return nil, false, errors.NewInternalError(fmt.Errorf("failed to allocate a nodePort: %v", err)) - } - servicePort.NodePort = int32(nodePort) - } - // Detect duplicate node ports; this should have been caught by validation, so we panic - if contains(newNodePorts, nodePort) { - panic("duplicate node port") - } - newNodePorts = append(newNodePorts, nodePort) + // Update service from ExternalName to non-ExternalName, should initialize ClusterIP. + if oldService.Spec.Type == api.ServiceTypeExternalName && service.Spec.Type != api.ServiceTypeExternalName { + if releaseServiceIP, err = rs.initClusterIP(service); err != nil { + return nil, false, err } - } else { - // Validate should have validated that nodePort == 0 } - - // The comparison loops are O(N^2), but we don't expect N to be huge - // (there's a hard-limit at 2^16, because they're ports; and even 4 ports would be a lot) - for _, oldNodePort := range oldNodePorts { - if contains(newNodePorts, oldNodePort) { - continue + // Update service from non-ExternalName to ExternalName, should release ClusterIP if exists. + if oldService.Spec.Type != api.ServiceTypeExternalName && service.Spec.Type == api.ServiceTypeExternalName { + if helper.IsServiceIPSet(oldService) { + rs.serviceIPs.Release(net.ParseIP(oldService.Spec.ClusterIP)) } - nodePortOp.ReleaseDeferred(oldNodePort) } - - // Remove any LoadBalancerStatus now if Type != LoadBalancer; - // although loadbalancer delete is actually asynchronous, we don't need to expose the user to that complexity. + // Update service from NodePort or LoadBalancer to ExternalName or ClusterIP, should release NodePort if exists. + if (oldService.Spec.Type == api.ServiceTypeNodePort || oldService.Spec.Type == api.ServiceTypeLoadBalancer) && + (service.Spec.Type == api.ServiceTypeExternalName || service.Spec.Type == api.ServiceTypeClusterIP) { + rs.releaseNodePort(oldService, nodePortOp) + } + // Update service from any type to NodePort or LoadBalancer, should update NodePort. + if service.Spec.Type == api.ServiceTypeNodePort || service.Spec.Type == api.ServiceTypeLoadBalancer { + if err := rs.updateNodePort(oldService, service, nodePortOp); err != nil { + return nil, false, err + } + } + // Update service from LoadBalancer to non-LoadBalancer, should remove any LoadBalancerStatus. if service.Spec.Type != api.ServiceTypeLoadBalancer { + // Although loadbalancer delete is actually asynchronous, we don't need to expose the user to that complexity. service.Status.LoadBalancer = api.LoadBalancerStatus{} } @@ -425,13 +412,14 @@ func (rs *REST) Update(ctx genericapirequest.Context, name string, objInfo rest. } out, err := rs.registry.UpdateService(ctx, service) - if err == nil { el := nodePortOp.Commit() if el != nil { // problems should be fixed by an eventual reconciliation / restart glog.Errorf("error(s) committing NodePorts changes: %v", el) } + + releaseServiceIP = false } return out, false, err @@ -570,3 +558,82 @@ func (rs *REST) allocateHealthCheckNodePort(service *api.Service) error { } return nil } + +// The return bool value indicates if a cluster IP is allocated successfully. +func (rs *REST) initClusterIP(service *api.Service) (bool, error) { + switch { + case service.Spec.ClusterIP == "": + // Allocate next available. + ip, err := rs.serviceIPs.AllocateNext() + if err != nil { + // TODO: what error should be returned here? It's not a + // field-level validation failure (the field is valid), and it's + // not really an internal error. + return false, errors.NewInternalError(fmt.Errorf("failed to allocate a serviceIP: %v", err)) + } + service.Spec.ClusterIP = ip.String() + return true, nil + case service.Spec.ClusterIP != api.ClusterIPNone && service.Spec.ClusterIP != "": + // Try to respect the requested IP. + if err := rs.serviceIPs.Allocate(net.ParseIP(service.Spec.ClusterIP)); err != nil { + // TODO: when validation becomes versioned, this gets more complicated. + el := field.ErrorList{field.Invalid(field.NewPath("spec", "clusterIP"), service.Spec.ClusterIP, err.Error())} + return false, errors.NewInvalid(api.Kind("Service"), service.Name, el) + } + return true, nil + } + + return false, nil +} + +func (rs *REST) updateNodePort(oldService, newService *api.Service, nodePortOp *portallocator.PortAllocationOperation) error { + oldNodePorts := CollectServiceNodePorts(oldService) + + newNodePorts := []int{} + for i := range newService.Spec.Ports { + servicePort := &newService.Spec.Ports[i] + nodePort := int(servicePort.NodePort) + if nodePort != 0 { + if !contains(oldNodePorts, nodePort) { + err := nodePortOp.Allocate(nodePort) + if err != nil { + el := field.ErrorList{field.Invalid(field.NewPath("spec", "ports").Index(i).Child("nodePort"), nodePort, err.Error())} + return errors.NewInvalid(api.Kind("Service"), newService.Name, el) + } + } + } else { + nodePort, err := nodePortOp.AllocateNext() + if err != nil { + // TODO: what error should be returned here? It's not a + // field-level validation failure (the field is valid), and it's + // not really an internal error. + return errors.NewInternalError(fmt.Errorf("failed to allocate a nodePort: %v", err)) + } + servicePort.NodePort = int32(nodePort) + } + // Detect duplicate node ports; this should have been caught by validation, so we panic + if contains(newNodePorts, nodePort) { + panic("duplicate node port") + } + newNodePorts = append(newNodePorts, nodePort) + } + + // The comparison loops are O(N^2), but we don't expect N to be huge + // (there's a hard-limit at 2^16, because they're ports; and even 4 ports would be a lot) + for _, oldNodePort := range oldNodePorts { + if contains(newNodePorts, oldNodePort) { + continue + } + nodePortOp.ReleaseDeferred(oldNodePort) + } + + return nil +} + +func (rs *REST) releaseNodePort(service *api.Service, nodePortOp *portallocator.PortAllocationOperation) { + nodePorts := CollectServiceNodePorts(service) + + for _, nodePort := range nodePorts { + nodePortOp.ReleaseDeferred(nodePort) + } +} diff --git a/pkg/registry/core/service/rest_test.go b/pkg/registry/core/service/rest_test.go index f6c1b1bed98..100eadb26c5 100644 --- a/pkg/registry/core/service/rest_test.go +++ b/pkg/registry/core/service/rest_test.go @@ -1276,3 +1276,269 @@ func TestServiceRegistryExternalTrafficAnnotationClusterIP(t *testing.T) { t.Errorf("Unexpected allocation of health check node port annotation %s", api.BetaAnnotationHealthCheckNodePort) } } + +func TestInitClusterIP(t *testing.T) { + storage, _ := NewTestREST(t, nil) + + testCases := []struct { + name string + svc *api.Service + expectClusterIP bool + }{ + { + name: "Allocate new ClusterIP", + svc: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ServiceSpec{ + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeClusterIP, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: intstr.FromInt(6502), + }}, + }, + }, + expectClusterIP: true, + }, + { + name: "Allocate specified ClusterIP", + svc: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ServiceSpec{ + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeClusterIP, + ClusterIP: "1.2.3.4", + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: intstr.FromInt(6502), + }}, + }, + }, + expectClusterIP: true, + }, + { + name: "Shouldn't allocate ClusterIP", + svc: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ServiceSpec{ + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeClusterIP, + ClusterIP: api.ClusterIPNone, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: intstr.FromInt(6502), + }}, + }, + }, + expectClusterIP: false, + }, + } + + for _, test := range testCases { + hasAllocatedIP, err := storage.initClusterIP(test.svc) + if err != nil { + t.Errorf("%q: unexpected error: %v", test.name, err) + } + + if hasAllocatedIP != test.expectClusterIP { + t.Errorf("%q: expected %v, but got %v", test.name, test.expectClusterIP, hasAllocatedIP) + } + + if test.expectClusterIP { + if !storage.serviceIPs.Has(net.ParseIP(test.svc.Spec.ClusterIP)) { + t.Errorf("%q: unexpected ClusterIP %q, out of range", test.name, test.svc.Spec.ClusterIP) + } + } + + if test.name == "Allocate specified ClusterIP" && test.svc.Spec.ClusterIP != "1.2.3.4" { + t.Errorf("%q: expected ClusterIP %q, but got %q", test.name, "1.2.3.4", test.svc.Spec.ClusterIP) + } + } +} + +func TestUpdateNodePort(t *testing.T) { + storage, _ := NewTestREST(t, nil) + nodePortOp := portallocator.StartOperation(storage.serviceNodePorts) + defer nodePortOp.Finish() + + testCases := []struct { + name string + oldService *api.Service + newService *api.Service + expectSpecifiedNodePorts []int + }{ + { + name: "Old service and new service have the same NodePort", + oldService: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ServiceSpec{ + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeNodePort, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: intstr.FromInt(6502), + NodePort: 30053, + }}, + }, + }, + newService: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ServiceSpec{ + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeNodePort, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: intstr.FromInt(6502), + NodePort: 30053, + }}, + }, + }, + expectSpecifiedNodePorts: []int{30053}, + }, + { + name: "Old service has more NodePorts than new service has", + oldService: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ServiceSpec{ + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeNodePort, + Ports: []api.ServicePort{ + { + Name: "port-tcp", + Port: 53, + TargetPort: intstr.FromInt(6502), + Protocol: api.ProtocolTCP, + NodePort: 30053, + }, + { + Name: "port-udp", + Port: 53, + TargetPort: intstr.FromInt(6502), + Protocol: api.ProtocolUDP, + NodePort: 30053, + }, + }, + }, + }, + newService: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ServiceSpec{ + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeNodePort, + Ports: []api.ServicePort{ + { + Name: "port-tcp", + Port: 53, + TargetPort: intstr.FromInt(6502), + Protocol: api.ProtocolTCP, + NodePort: 30053, + }, + }, + }, + }, + expectSpecifiedNodePorts: []int{30053}, + }, + { + name: "Change protocol of ServicePort without changing NodePort", + oldService: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ServiceSpec{ + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeNodePort, + Ports: []api.ServicePort{ + { + Name: "port-tcp", + Port: 53, + TargetPort: intstr.FromInt(6502), + Protocol: api.ProtocolTCP, + NodePort: 30053, + }, + }, + }, + }, + newService: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ServiceSpec{ + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeNodePort, + Ports: []api.ServicePort{ + { + Name: "port-udp", + Port: 53, + TargetPort: intstr.FromInt(6502), + Protocol: api.ProtocolUDP, + NodePort: 30053, + }, + }, + }, + }, + expectSpecifiedNodePorts: []int{30053}, + }, + { + name: "Should allocate NodePort when changing service type to NodePort", + oldService: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ServiceSpec{ + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeClusterIP, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: intstr.FromInt(6502), + }}, + }, + }, + newService: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: api.ServiceSpec{ + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeNodePort, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: intstr.FromInt(6502), + }}, + }, + }, + expectSpecifiedNodePorts: []int{}, + }, + } + + for _, test := range testCases { + err := storage.updateNodePort(test.oldService, test.newService, nodePortOp) + if err != nil { + t.Errorf("%q: unexpected error: %v", test.name, err) + continue + } + _ = nodePortOp.Commit() + + serviceNodePorts := CollectServiceNodePorts(test.newService) + + if len(test.expectSpecifiedNodePorts) == 0 { + for _, nodePort := range serviceNodePorts { + if !storage.serviceNodePorts.Has(nodePort) { + t.Errorf("%q: unexpected NodePort %d, out of range", test.name, nodePort) + } + } + } else if !reflect.DeepEqual(serviceNodePorts, test.expectSpecifiedNodePorts) { + t.Errorf("%q: expected NodePorts %v, but got %v", test.name, test.expectSpecifiedNodePorts, serviceNodePorts) + } + + } +} diff --git a/test/e2e/dns.go b/test/e2e/dns.go index 5c27ef39a24..456d7f8bed5 100644 --- a/test/e2e/dns.go +++ b/test/e2e/dns.go @@ -225,7 +225,6 @@ func assertFilesContain(fileNames []string, fileDir string, pod *v1.Pod, client } func validateDNSResults(f *framework.Framework, pod *v1.Pod, fileNames []string) { - By("submitting the pod to kubernetes") podClient := f.ClientSet.Core().Pods(f.Namespace.Name) defer func() { @@ -254,7 +253,6 @@ func validateDNSResults(f *framework.Framework, pod *v1.Pod, fileNames []string) } func validateTargetedProbeOutput(f *framework.Framework, pod *v1.Pod, fileNames []string, value string) { - By("submitting the pod to kubernetes") podClient := f.ClientSet.Core().Pods(f.Namespace.Name) defer func() { @@ -401,6 +399,10 @@ var _ = framework.KubeDescribe("DNS", func() { }) It("should provide DNS for ExternalName services", func() { + // TODO(xiangpengzhao): allow AWS when pull-kubernetes-e2e-kops-aws and pull-kubernetes-e2e-gce-etcd3 + // have the same "service-cluster-ip-range". See: https://github.com/kubernetes/kubernetes/issues/47224 + framework.SkipUnlessProviderIs("gce") + // Create a test ExternalName service. By("Creating a test externalName service") serviceName := "dns-test-service-3" @@ -446,7 +448,7 @@ var _ = framework.KubeDescribe("DNS", func() { By("changing the service to type=ClusterIP") _, err = framework.UpdateService(f.ClientSet, f.Namespace.Name, serviceName, func(s *v1.Service) { s.Spec.Type = v1.ServiceTypeClusterIP - s.Spec.ClusterIP = "127.1.2.3" + s.Spec.ClusterIP = "10.0.0.123" s.Spec.Ports = []v1.ServicePort{ {Port: 80, Name: "http", Protocol: "TCP"}, } @@ -461,6 +463,6 @@ var _ = framework.KubeDescribe("DNS", func() { By("creating a third pod to probe DNS") pod3 := createDNSPod(f.Namespace.Name, wheezyProbeCmd, jessieProbeCmd, true) - validateTargetedProbeOutput(f, pod3, []string{wheezyFileName, jessieFileName}, "127.1.2.3") + validateTargetedProbeOutput(f, pod3, []string{wheezyFileName, jessieFileName}, "10.0.0.123") }) }) diff --git a/test/e2e/framework/service_util.go b/test/e2e/framework/service_util.go index 9de2d907494..114dececb12 100644 --- a/test/e2e/framework/service_util.go +++ b/test/e2e/framework/service_util.go @@ -35,6 +35,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/client/clientset_generated/clientset" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" "k8s.io/kubernetes/pkg/client/retry" @@ -177,6 +178,31 @@ func (j *ServiceTestJig) CreateUDPServiceOrFail(namespace string, tweak func(svc return result } +// CreateExternalNameServiceOrFail creates a new ExternalName type Service based on the jig's defaults. +// Callers can provide a function to tweak the Service object before it is created. +func (j *ServiceTestJig) CreateExternalNameServiceOrFail(namespace string, tweak func(svc *v1.Service)) *v1.Service { + svc := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: j.Name, + Labels: j.Labels, + }, + Spec: v1.ServiceSpec{ + Selector: j.Labels, + ExternalName: "foo.example.com", + Type: v1.ServiceTypeExternalName, + }, + } + if tweak != nil { + tweak(svc) + } + result, err := j.Client.Core().Services(namespace).Create(svc) + if err != nil { + Failf("Failed to create ExternalName Service %q: %v", svc.Name, err) + } + return result +} + func (j *ServiceTestJig) ChangeServiceType(namespace, name string, newType v1.ServiceType, timeout time.Duration) { ingressIP := "" svc := j.UpdateServiceOrFail(namespace, name, func(s *v1.Service) { @@ -373,8 +399,22 @@ func (j *ServiceTestJig) SanityCheckService(svc *v1.Service, svcType v1.ServiceT if svc.Spec.Type != svcType { Failf("unexpected Spec.Type (%s) for service, expected %s", svc.Spec.Type, svcType) } + + if svcType != v1.ServiceTypeExternalName { + if svc.Spec.ExternalName != "" { + Failf("unexpected Spec.ExternalName (%s) for service, expected empty", svc.Spec.ExternalName) + } + if svc.Spec.ClusterIP != api.ClusterIPNone && svc.Spec.ClusterIP == "" { + Failf("didn't get ClusterIP for non-ExternamName service") + } + } else { + if svc.Spec.ClusterIP != "" { + Failf("unexpected Spec.ClusterIP (%s) for ExternamName service, expected empty", svc.Spec.ClusterIP) + } + } + expectNodePorts := false - if svcType != v1.ServiceTypeClusterIP { + if svcType != v1.ServiceTypeClusterIP && svcType != v1.ServiceTypeExternalName { expectNodePorts = true } for i, port := range svc.Spec.Ports { diff --git a/test/e2e/service.go b/test/e2e/service.go index e6d7e4a81b8..c4ec1db67a1 100644 --- a/test/e2e/service.go +++ b/test/e2e/service.go @@ -789,6 +789,101 @@ var _ = framework.KubeDescribe("Services", func() { } }) + It("should be able to change the type from ExternalName to ClusterIP", func() { + serviceName := "externalname-service" + ns := f.Namespace.Name + jig := framework.NewServiceTestJig(cs, serviceName) + + By("creating a service " + serviceName + " with the type=ExternalName in namespace " + ns) + externalNameService := jig.CreateExternalNameServiceOrFail(ns, nil) + defer func() { + framework.Logf("Cleaning up the ExternalName to ClusterIP test service") + err := cs.Core().Services(ns).Delete(serviceName, nil) + Expect(err).NotTo(HaveOccurred()) + }() + jig.SanityCheckService(externalNameService, v1.ServiceTypeExternalName) + By("changing the ExternalName service to type=ClusterIP") + clusterIPService := jig.UpdateServiceOrFail(ns, externalNameService.Name, func(s *v1.Service) { + s.Spec.Type = v1.ServiceTypeClusterIP + s.Spec.ExternalName = "" + s.Spec.Ports = []v1.ServicePort{ + {Port: 80, Name: "http", Protocol: "TCP"}, + } + }) + jig.SanityCheckService(clusterIPService, v1.ServiceTypeClusterIP) + }) + + It("should be able to change the type from ExternalName to NodePort", func() { + serviceName := "externalname-service" + ns := f.Namespace.Name + jig := framework.NewServiceTestJig(cs, serviceName) + + By("creating a service " + serviceName + " with the type=ExternalName in namespace " + ns) + externalNameService := jig.CreateExternalNameServiceOrFail(ns, nil) + defer func() { + framework.Logf("Cleaning up the ExternalName to NodePort test service") + err := cs.Core().Services(ns).Delete(serviceName, nil) + Expect(err).NotTo(HaveOccurred()) + }() + jig.SanityCheckService(externalNameService, v1.ServiceTypeExternalName) + By("changing the ExternalName service to type=NodePort") + nodePortService := jig.UpdateServiceOrFail(ns, externalNameService.Name, func(s *v1.Service) { + s.Spec.Type = v1.ServiceTypeNodePort + s.Spec.ExternalName = "" + s.Spec.Ports = []v1.ServicePort{ + {Port: 80, Name: "http", Protocol: "TCP"}, + } + }) + jig.SanityCheckService(nodePortService, v1.ServiceTypeNodePort) + }) + + It("should be able to change the type from ClusterIP to ExternalName", func() { + serviceName := "clusterip-service" + ns := f.Namespace.Name + jig := framework.NewServiceTestJig(cs, serviceName) + + By("creating a service " + serviceName + " with the type=ClusterIP in namespace " + ns) + clusterIPService := jig.CreateTCPServiceOrFail(ns, nil) + defer func() { + framework.Logf("Cleaning up the ClusterIP to ExternalName test service") + err := cs.Core().Services(ns).Delete(serviceName, nil) + Expect(err).NotTo(HaveOccurred()) + }() + jig.SanityCheckService(clusterIPService, v1.ServiceTypeClusterIP) + By("changing the ClusterIP service to type=ExternalName") + externalNameService := jig.UpdateServiceOrFail(ns, clusterIPService.Name, func(s *v1.Service) { + s.Spec.Type = v1.ServiceTypeExternalName + s.Spec.ExternalName = "foo.example.com" + s.Spec.ClusterIP = "" + }) + jig.SanityCheckService(externalNameService, v1.ServiceTypeExternalName) + }) + + It("should be able to change the type from NodePort to ExternalName", func() { + serviceName := "nodeport-service" + ns := f.Namespace.Name + jig := framework.NewServiceTestJig(cs, serviceName) + + By("creating a service " + serviceName + " with the type=NodePort in namespace " + ns) + nodePortService := jig.CreateTCPServiceOrFail(ns, func(svc *v1.Service) { + svc.Spec.Type = v1.ServiceTypeNodePort + }) + defer func() { + framework.Logf("Cleaning up the NodePort to ExternalName test service") + err := cs.Core().Services(ns).Delete(serviceName, nil) + Expect(err).NotTo(HaveOccurred()) + }() + jig.SanityCheckService(nodePortService, v1.ServiceTypeNodePort) + By("changing the NodePort service to type=ExternalName") + externalNameService := jig.UpdateServiceOrFail(ns, nodePortService.Name, func(s *v1.Service) { + s.Spec.Type = v1.ServiceTypeExternalName + s.Spec.ExternalName = "foo.example.com" + s.Spec.ClusterIP = "" + s.Spec.Ports[0].NodePort = 0 + }) + jig.SanityCheckService(externalNameService, v1.ServiceTypeExternalName) + }) + It("should use same NodePort with same port but different protocols", func() { serviceName := "nodeports" ns := f.Namespace.Name @@ -866,7 +961,7 @@ var _ = framework.KubeDescribe("Services", func() { } port := result.Spec.Ports[0] if port.NodePort == 0 { - framework.Failf("got unexpected Spec.Ports[0].nodePort for new service: %v", result) + framework.Failf("got unexpected Spec.Ports[0].NodePort for new service: %v", result) } By("creating service " + serviceName2 + " with conflicting NodePort")