diff --git a/pkg/api/rest/types.go b/pkg/api/rest/types.go index a43458dd465..5c4ccf90edf 100644 --- a/pkg/api/rest/types.go +++ b/pkg/api/rest/types.go @@ -98,8 +98,6 @@ func (svcStrategy) NamespaceScoped() bool { // ResetBeforeCreate clears fields that are not allowed to be set by end users on creation. func (svcStrategy) ResetBeforeCreate(obj runtime.Object) { service := obj.(*api.Service) - // TODO: Get rid of ProxyPort. - service.Spec.ProxyPort = 0 service.Status = api.ServiceStatus{} } diff --git a/pkg/api/types.go b/pkg/api/types.go index 05f1b0d0ac7..f78b1d7d460 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -672,10 +672,6 @@ type ServiceSpec struct { // not be changed by updates. PortalIP string `json:"portalIP,omitempty"` - // ProxyPort is assigned by the master. If 0, the proxy will choose an ephemeral port. - // TODO: This is awkward - if we had a BoundService, it would be better factored. - ProxyPort int `json:"proxyPort,omitempty"` - // CreateExternalLoadBalancer indicates whether a load balancer should be created for this service. CreateExternalLoadBalancer bool `json:"createExternalLoadBalancer,omitempty"` // PublicIPs are used by external load balancers. diff --git a/pkg/api/v1beta1/conversion.go b/pkg/api/v1beta1/conversion.go index 7780c0e890a..5d7d02bb796 100644 --- a/pkg/api/v1beta1/conversion.go +++ b/pkg/api/v1beta1/conversion.go @@ -634,7 +634,6 @@ func init() { out.PublicIPs = in.Spec.PublicIPs out.ContainerPort = in.Spec.ContainerPort out.PortalIP = in.Spec.PortalIP - out.ProxyPort = in.Spec.ProxyPort if err := s.Convert(&in.Spec.SessionAffinity, &out.SessionAffinity, 0); err != nil { return err } @@ -661,7 +660,6 @@ func init() { out.Spec.PublicIPs = in.PublicIPs out.Spec.ContainerPort = in.ContainerPort out.Spec.PortalIP = in.PortalIP - out.Spec.ProxyPort = in.ProxyPort if err := s.Convert(&in.SessionAffinity, &out.Spec.SessionAffinity, 0); err != nil { return err } diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index 6eb8e11cdda..ceefd4b267a 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -544,7 +544,7 @@ type Service struct { // not be changed by updates. PortalIP string `json:"portalIP,omitempty" description:"IP address of the service; usually assigned by the system; if specified, it will be allocated to the service if unused, and creation of the service will fail otherwise; cannot be updated"` - // ProxyPort is assigned by the master. If specified by the user it will be ignored. + // DEPRECATED: has no implementation. ProxyPort int `json:"proxyPort,omitempty" description:"if non-zero, a pre-allocated host port used for this service by the proxy on each node; assigned by the master and ignored on input"` // Optional: Supports "ClientIP" and "None". Used to maintain session affinity. diff --git a/pkg/api/v1beta2/conversion.go b/pkg/api/v1beta2/conversion.go index 72882cce78f..74c383139fc 100644 --- a/pkg/api/v1beta2/conversion.go +++ b/pkg/api/v1beta2/conversion.go @@ -554,7 +554,6 @@ func init() { out.PublicIPs = in.Spec.PublicIPs out.ContainerPort = in.Spec.ContainerPort out.PortalIP = in.Spec.PortalIP - out.ProxyPort = in.Spec.ProxyPort if err := s.Convert(&in.Spec.SessionAffinity, &out.SessionAffinity, 0); err != nil { return err } @@ -581,7 +580,6 @@ func init() { out.Spec.PublicIPs = in.PublicIPs out.Spec.ContainerPort = in.ContainerPort out.Spec.PortalIP = in.PortalIP - out.Spec.ProxyPort = in.ProxyPort if err := s.Convert(&in.SessionAffinity, &out.Spec.SessionAffinity, 0); err != nil { return err } diff --git a/pkg/api/v1beta2/types.go b/pkg/api/v1beta2/types.go index 4d5898b3247..7cb4dae8592 100644 --- a/pkg/api/v1beta2/types.go +++ b/pkg/api/v1beta2/types.go @@ -508,7 +508,7 @@ type Service struct { // not be changed by updates. PortalIP string `json:"portalIP,omitempty" description:"IP address of the service; usually assigned by the system; if specified, it will be allocated to the service if unused, and creation of the service will fail otherwise; cannot be updated"` - // ProxyPort is assigned by the master. If specified by the user it will be ignored. + // DEPRECATED: has no implementation. ProxyPort int `json:"proxyPort,omitempty" description:"if non-zero, a pre-allocated host port used for this service by the proxy on each node; assigned by the master and ignored on input"` // Optional: Supports "ClientIP" and "None". Used to maintain session affinity. diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go index c8918aa6f69..48ddc7cec30 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -691,9 +691,6 @@ type ServiceSpec struct { // not be changed by updates. PortalIP string `json:"portalIP,omitempty"` - // ProxyPort is assigned by the master. If 0, the proxy will choose an ephemeral port. - ProxyPort int `json:"proxyPort,omitempty"` - // CreateExternalLoadBalancer indicates whether a load balancer should be created for this service. CreateExternalLoadBalancer bool `json:"createExternalLoadBalancer,omitempty"` // PublicIPs are used by external load balancers. diff --git a/pkg/proxy/proxier.go b/pkg/proxy/proxier.go index 5c0f1d604a9..245689cc134 100644 --- a/pkg/proxy/proxier.go +++ b/pkg/proxy/proxier.go @@ -481,8 +481,8 @@ func (proxier *Proxier) OnUpdate(services []api.Service) { glog.Errorf("Failed to stop service %q: %v", service.Name, err) } } - glog.V(1).Infof("Adding new service %q at %s:%d/%s (local :%d)", service.Name, serviceIP, service.Spec.Port, service.Spec.Protocol, service.Spec.ProxyPort) - info, err := proxier.addServiceOnPort(service.Name, service.Spec.Protocol, service.Spec.ProxyPort, udpIdleTimeout) + glog.V(1).Infof("Adding new service %q at %s:%d/%s", service.Name, serviceIP, service.Spec.Port, service.Spec.Protocol) + info, err := proxier.addServiceOnPort(service.Name, service.Spec.Protocol, 0, udpIdleTimeout) if err != nil { glog.Errorf("Failed to start proxy for %q: %v", service.Name, err) continue diff --git a/pkg/proxy/proxier_test.go b/pkg/proxy/proxier_test.go index 81e7345246a..70050965714 100644 --- a/pkg/proxy/proxier_test.go +++ b/pkg/proxy/proxier_test.go @@ -23,7 +23,6 @@ import ( "net/http" "net/http/httptest" "net/url" - "strconv" "sync/atomic" "testing" "time" @@ -384,8 +383,12 @@ func TestTCPProxyUpdateDeleteUpdate(t *testing.T) { } waitForNumProxyLoops(t, p, 0) p.OnUpdate([]api.Service{ - {ObjectMeta: api.ObjectMeta{Name: "echo"}, Spec: api.ServiceSpec{Port: svcInfo.proxyPort, Protocol: "TCP", ProxyPort: svcInfo.proxyPort}, Status: api.ServiceStatus{}}, + {ObjectMeta: api.ObjectMeta{Name: "echo"}, Spec: api.ServiceSpec{Port: svcInfo.proxyPort, Protocol: "TCP"}, Status: api.ServiceStatus{}}, }) + svcInfo, exists := p.getServiceInfo("echo") + if !exists { + t.Fatalf("can't find serviceInfo") + } testEchoTCP(t, "127.0.0.1", svcInfo.proxyPort) waitForNumProxyLoops(t, p, 1) } @@ -419,8 +422,12 @@ func TestUDPProxyUpdateDeleteUpdate(t *testing.T) { } waitForNumProxyLoops(t, p, 0) p.OnUpdate([]api.Service{ - {ObjectMeta: api.ObjectMeta{Name: "echo"}, Spec: api.ServiceSpec{Port: svcInfo.proxyPort, Protocol: "UDP", ProxyPort: svcInfo.proxyPort}, Status: api.ServiceStatus{}}, + {ObjectMeta: api.ObjectMeta{Name: "echo"}, Spec: api.ServiceSpec{Port: svcInfo.proxyPort, Protocol: "UDP"}, Status: api.ServiceStatus{}}, }) + svcInfo, exists := p.getServiceInfo("echo") + if !exists { + t.Fatalf("can't find serviceInfo") + } testEchoUDP(t, "127.0.0.1", svcInfo.proxyPort) waitForNumProxyLoops(t, p, 1) } @@ -444,36 +451,21 @@ func TestTCPProxyUpdatePort(t *testing.T) { testEchoTCP(t, "127.0.0.1", svcInfo.proxyPort) waitForNumProxyLoops(t, p, 1) - // add a new dummy listener in order to get a port that is free - l, _ := net.Listen("tcp", ":0") - _, newPortStr, _ := net.SplitHostPort(l.Addr().String()) - newPort, _ := strconv.Atoi(newPortStr) - l.Close() - - // Wait for the socket to actually get free. - if err := waitForClosedPortTCP(p, newPort); err != nil { - t.Fatalf(err.Error()) - } - if svcInfo.proxyPort == newPort { - t.Errorf("expected difference, got %d %d", newPort, svcInfo.proxyPort) - } p.OnUpdate([]api.Service{ - {ObjectMeta: api.ObjectMeta{Name: "echo"}, Spec: api.ServiceSpec{Port: newPort, Protocol: "TCP", ProxyPort: newPort}, Status: api.ServiceStatus{}}, + {ObjectMeta: api.ObjectMeta{Name: "echo"}, Spec: api.ServiceSpec{Port: 99, Protocol: "TCP"}, Status: api.ServiceStatus{}}, }) + // Wait for the socket to actually get free. if err := waitForClosedPortTCP(p, svcInfo.proxyPort); err != nil { t.Fatalf(err.Error()) } - testEchoTCP(t, "127.0.0.1", newPort) + svcInfo, exists := p.getServiceInfo("echo") + if !exists { + t.Fatalf("can't find serviceInfo") + } + testEchoTCP(t, "127.0.0.1", svcInfo.proxyPort) // This is a bit async, but this should be sufficient. time.Sleep(500 * time.Millisecond) waitForNumProxyLoops(t, p, 1) - - // Ensure the old port is released and re-usable. - l, err = net.Listen("tcp", joinHostPort("", svcInfo.proxyPort)) - if err != nil { - t.Fatalf("can't claim released port: %s", err) - } - l.Close() } func TestUDPProxyUpdatePort(t *testing.T) { @@ -494,34 +486,19 @@ func TestUDPProxyUpdatePort(t *testing.T) { } waitForNumProxyLoops(t, p, 1) - // add a new dummy listener in order to get a port that is free - pc, _ := net.ListenPacket("udp", ":0") - _, newPortStr, _ := net.SplitHostPort(pc.LocalAddr().String()) - newPort, _ := strconv.Atoi(newPortStr) - pc.Close() - - // Wait for the socket to actually get free. - if err := waitForClosedPortUDP(p, newPort); err != nil { - t.Fatalf(err.Error()) - } - if svcInfo.proxyPort == newPort { - t.Errorf("expected difference, got %d %d", newPort, svcInfo.proxyPort) - } p.OnUpdate([]api.Service{ - {ObjectMeta: api.ObjectMeta{Name: "echo"}, Spec: api.ServiceSpec{Port: newPort, Protocol: "UDP", ProxyPort: newPort}, Status: api.ServiceStatus{}}, + {ObjectMeta: api.ObjectMeta{Name: "echo"}, Spec: api.ServiceSpec{Port: 99, Protocol: "UDP"}, Status: api.ServiceStatus{}}, }) + // Wait for the socket to actually get free. if err := waitForClosedPortUDP(p, svcInfo.proxyPort); err != nil { t.Fatalf(err.Error()) } - testEchoUDP(t, "127.0.0.1", newPort) - waitForNumProxyLoops(t, p, 1) - - // Ensure the old port is released and re-usable. - pc, err = net.ListenPacket("udp", joinHostPort("", svcInfo.proxyPort)) - if err != nil { - t.Fatalf("can't claim released port: %s", err) + svcInfo, exists := p.getServiceInfo("echo") + if !exists { + t.Fatalf("can't find serviceInfo") } - pc.Close() + testEchoUDP(t, "127.0.0.1", svcInfo.proxyPort) + waitForNumProxyLoops(t, p, 1) } // TODO: Test UDP timeouts. diff --git a/pkg/registry/service/rest.go b/pkg/registry/service/rest.go index 70ed8ca69ef..638cf592c84 100644 --- a/pkg/registry/service/rest.go +++ b/pkg/registry/service/rest.go @@ -229,10 +229,7 @@ func (rs *REST) Update(ctx api.Context, obj runtime.Object) (<-chan apiserver.RE } // Copy over non-user fields - // TODO: this should be a Status field, since the end user does not set it. // TODO: make this a merge function - service.Spec.ProxyPort = oldService.Spec.ProxyPort - if errs := validation.ValidateServiceUpdate(oldService, service); len(errs) > 0 { return nil, errors.NewInvalid("service", service.Name, errs) } diff --git a/pkg/registry/service/rest_test.go b/pkg/registry/service/rest_test.go index b18d7de0ad8..82469b13f9f 100644 --- a/pkg/registry/service/rest_test.go +++ b/pkg/registry/service/rest_test.go @@ -69,9 +69,6 @@ func TestServiceRegistryCreate(t *testing.T) { if created_service.Spec.PortalIP != "1.2.3.1" { t.Errorf("Unexpected PortalIP: %s", created_service.Spec.PortalIP) } - if created_service.Spec.ProxyPort != 0 { - t.Errorf("Unexpected ProxyPort: %d", created_service.Spec.ProxyPort) - } if len(fakeCloud.Calls) != 0 { t.Errorf("Unexpected call(s): %#v", fakeCloud.Calls) } @@ -509,14 +506,10 @@ func TestServiceRegistryIPUpdate(t *testing.T) { if created_service.Spec.PortalIP != "1.2.3.1" { t.Errorf("Unexpected PortalIP: %s", created_service.Spec.PortalIP) } - if created_service.Spec.ProxyPort != 0 { - t.Errorf("Unexpected ProxyPort: %d", created_service.Spec.ProxyPort) - } update := new(api.Service) *update = *created_service update.Spec.Port = 6503 - update.Spec.ProxyPort = 309 // should be ignored c, _ = rest.Update(ctx, update) updated_svc := <-c @@ -524,9 +517,6 @@ func TestServiceRegistryIPUpdate(t *testing.T) { if updated_service.Spec.Port != 6503 { t.Errorf("Expected port 6503, but got %v", updated_service.Spec.Port) } - if updated_service.Spec.ProxyPort != 0 { // unchanged, despite trying - t.Errorf("Unexpected ProxyPort: %d", updated_service.Spec.ProxyPort) - } *update = *created_service update.Spec.Port = 6503 @@ -563,9 +553,6 @@ func TestServiceRegistryIPExternalLoadBalancer(t *testing.T) { if created_service.Spec.PortalIP != "1.2.3.1" { t.Errorf("Unexpected PortalIP: %s", created_service.Spec.PortalIP) } - if created_service.Spec.ProxyPort != 0 { - t.Errorf("Unexpected ProxyPort: %d", created_service.Spec.ProxyPort) - } update := new(api.Service) *update = *created_service