Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Part 2 of plural ports: make endpoints a struct #4580

Merged
merged 2 commits into from
Feb 20, 2015
Merged

Part 2 of plural ports: make endpoints a struct #4580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
34eaa0d
Part 1 of plural ports: Add protocol to Endpoints
thockin Feb 18, 2015
ae0062d
Part 2 of plural ports: make endpoints a struct
thockin Feb 19, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions pkg/api/testing/fuzzer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package testing

import (
"fmt"
"math/rand"
"strconv"
"testing"
Expand Down Expand Up @@ -233,6 +234,11 @@ func FuzzerFor(t *testing.T, version string, src rand.Source) *fuzz.Fuzzer {

s.Type = api.SecretTypeOpaque
},
func(ep *api.Endpoint, c fuzz.Continue) {
// TODO: If our API used a particular type for IP fields we could just catch that here.
ep.IP = fmt.Sprintf("%d.%d.%d.%d", c.Rand.Intn(256), c.Rand.Intn(256), c.Rand.Intn(256), c.Rand.Intn(256))
ep.Port = c.Rand.Intn(65536)
},
)
return f
}
17 changes: 15 additions & 2 deletions pkg/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -742,12 +742,25 @@ type Service struct {
}

// Endpoints is a collection of endpoints that implement the actual service, for example:
// Name: "mysql", Endpoints: ["10.10.1.1:1909", "10.10.2.2:8834"]
// Name: "mysql", Endpoints: [{"ip": "10.10.1.1", "port": 1909}, {"ip": "10.10.2.2", "port": 8834}]
type Endpoints struct {
TypeMeta `json:",inline"`
ObjectMeta `json:"metadata,omitempty"`

Endpoints []string `json:"endpoints,omitempty"`
// Optional: The IP protocol for these endpoints. Supports "TCP" and
// "UDP". Defaults to "TCP".
Protocol Protocol `json:"protocol,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Protocol needs to go into the Endpoint struct. Example: DNS service listening on both UDP 53 and TCP 53.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it will migrate, but I wanted to make clean commits that each are plausibly correct on their own.

Endpoints []Endpoint `json:"endpoints,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Endpoints.Endpoints seems redundant (and also kind of confusing)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It gets worse as this PR series evolves - EndpointsList.Items[x].Endpoints[y].Ports[z].IP

I didn't choose the name, I just rolled with what was there. I'm happy to change it if you have a solid suggestion :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HostPorts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The real fix (IMO) would be to rename the struct to ServiceEndpoints or something, but ain't nobody got time to rename REST objects :)

HostPorts is wrong because it implies something like Container[x].Ports[y].HostPort, which is something else. PodPorts is wrong because endpoints are supposed to be only loosely coupled to kubernetes, and may not even be pods. Targets? Destinations? I didn't find any of these to be significantly better than Endpoints, so I left it as it was.

}

// Endpoint is a single IP endpoint of a service.
type Endpoint struct {
// Required: The IP of this endpoint.
// TODO: This should allow hostname or IP, see #4447.
IP string `json:"ip"`

// Required: The destination port to access.
Port int `json:"port"`
}

// EndpointsList is a list of endpoints.
Expand Down
43 changes: 43 additions & 0 deletions pkg/api/v1beta1/conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1beta1

import (
"fmt"
"net"
"strconv"

newer "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
Expand Down Expand Up @@ -1113,6 +1114,48 @@ func init() {
out.TimeoutSeconds = in.TimeoutSeconds
return nil
},
func(in *newer.Endpoints, out *Endpoints, s conversion.Scope) error {
if err := s.Convert(&in.TypeMeta, &out.TypeMeta, 0); err != nil {
return err
}
if err := s.Convert(&in.ObjectMeta, &out.TypeMeta, 0); err != nil {
return err
}
if err := s.Convert(&in.Protocol, &out.Protocol, 0); err != nil {
return err
}
for i := range in.Endpoints {
ep := &in.Endpoints[i]
out.Endpoints = append(out.Endpoints, net.JoinHostPort(ep.IP, strconv.Itoa(ep.Port)))
}
return nil
},
func(in *Endpoints, out *newer.Endpoints, s conversion.Scope) error {
if err := s.Convert(&in.TypeMeta, &out.TypeMeta, 0); err != nil {
return err
}
if err := s.Convert(&in.TypeMeta, &out.ObjectMeta, 0); err != nil {
return err
}
if err := s.Convert(&in.Protocol, &out.Protocol, 0); err != nil {
return err
}
for i := range in.Endpoints {
out.Endpoints = append(out.Endpoints, newer.Endpoint{})
ep := &out.Endpoints[i]
host, port, err := net.SplitHostPort(in.Endpoints[i])
if err != nil {
return err
}
ep.IP = host
pn, err := strconv.Atoi(port)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can simplify this to:

if ep.Port, err = strconv.Atoi(port); err != nil {
   return err
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. In the relentless goal of a dumber, I mean SIMPLER grammar, Go has done away with the idea of an lvalue and just has a "name" token. ep.Port is not a name. There's a bug open against go to allow this syntax, but it's at least a year old and not fixed.

if err != nil {
return err
}
ep.Port = pn
}
return nil
},
)
if err != nil {
// If one of the conversion functions is malformed, detect it immediately.
Expand Down
69 changes: 69 additions & 0 deletions pkg/api/v1beta1/conversion_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,3 +382,72 @@ func TestContainerConversion(t *testing.T) {
}
}
}

func TestEndpointsConversion(t *testing.T) {
testCases := []struct {
given current.Endpoints
expected newer.Endpoints
}{
{
given: current.Endpoints{
TypeMeta: current.TypeMeta{
ID: "empty",
},
Protocol: current.ProtocolTCP,
Endpoints: []string{},
},
expected: newer.Endpoints{
Protocol: newer.ProtocolTCP,
Endpoints: []newer.Endpoint{},
},
},
{
given: current.Endpoints{
TypeMeta: current.TypeMeta{
ID: "one",
},
Protocol: current.ProtocolTCP,
Endpoints: []string{"1.2.3.4:88"},
},
expected: newer.Endpoints{
Protocol: newer.ProtocolTCP,
Endpoints: []newer.Endpoint{{IP: "1.2.3.4", Port: 88}},
},
},
{
given: current.Endpoints{
TypeMeta: current.TypeMeta{
ID: "several",
},
Protocol: current.ProtocolUDP,
Endpoints: []string{"1.2.3.4:88", "1.2.3.4:89", "1.2.3.4:90"},
},
expected: newer.Endpoints{
Protocol: newer.ProtocolUDP,
Endpoints: []newer.Endpoint{{IP: "1.2.3.4", Port: 88}, {IP: "1.2.3.4", Port: 89}, {IP: "1.2.3.4", Port: 90}},
},
},
}

for i, tc := range testCases {
// Convert versioned -> internal.
got := newer.Endpoints{}
if err := Convert(&tc.given, &got); err != nil {
t.Errorf("[Case: %d] Unexpected error: %v", i, err)
continue
}
if got.Protocol != tc.expected.Protocol || !newer.Semantic.DeepEqual(got.Endpoints, tc.expected.Endpoints) {
t.Errorf("[Case: %d] Expected %v, got %v", i, tc.expected, got)
}

// Convert internal -> versioned.
got2 := current.Endpoints{}
if err := Convert(&got, &got2); err != nil {
t.Errorf("[Case: %d] Unexpected error: %v", i, err)
continue
}
if got2.Protocol != tc.given.Protocol || !newer.Semantic.DeepEqual(got2.Endpoints, tc.given.Endpoints) {
t.Errorf("[Case: %d] Expected %v, got %v", i, tc.given, got2)
}
}
}
5 changes: 5 additions & 0 deletions pkg/api/v1beta1/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,5 +85,10 @@ func init() {
obj.Type = SecretTypeOpaque
}
},
func(obj *Endpoints) {
if obj.Protocol == "" {
obj.Protocol = "TCP"
}
},
)
}
10 changes: 10 additions & 0 deletions pkg/api/v1beta1/defaults_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,13 @@ func TestSetDefaultSecret(t *testing.T) {
t.Errorf("Expected secret type %v, got %v", current.SecretTypeOpaque, s2.Type)
}
}

func TestSetDefaulEndpointsProtocol(t *testing.T) {
in := &current.Endpoints{}
obj := roundTrip(t, runtime.Object(in))
out := obj.(*current.Endpoints)

if out.Protocol != current.ProtocolTCP {
t.Errorf("Expected protocol %s, got %s", current.ProtocolTCP, out.Protocol)
}
}
5 changes: 4 additions & 1 deletion pkg/api/v1beta1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,10 @@ type Service struct {
// Endpoints is a collection of endpoints that implement the actual service, for example:
// Name: "mysql", Endpoints: ["10.10.1.1:1909", "10.10.2.2:8834"]
type Endpoints struct {
TypeMeta `json:",inline"`
TypeMeta `json:",inline"`
// Optional: The IP protocol for these endpoints. Supports "TCP" and
// "UDP". Defaults to "TCP".
Protocol Protocol `json:"protocol,omitempty" description:"IP protocol for endpoint ports; must be UDP or TCP; TCP if unspecified"`
Endpoints []string `json:"endpoints,omitempty" description:"list of endpoints corresponding to a service, of the form address:port, such as 10.10.1.1:1909"`
}

Expand Down
43 changes: 43 additions & 0 deletions pkg/api/v1beta2/conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1beta2

import (
"fmt"
"net"
"strconv"

newer "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
Expand Down Expand Up @@ -1028,6 +1029,48 @@ func init() {
out.TimeoutSeconds = in.TimeoutSeconds
return nil
},
func(in *newer.Endpoints, out *Endpoints, s conversion.Scope) error {
if err := s.Convert(&in.TypeMeta, &out.TypeMeta, 0); err != nil {
return err
}
if err := s.Convert(&in.ObjectMeta, &out.TypeMeta, 0); err != nil {
return err
}
if err := s.Convert(&in.Protocol, &out.Protocol, 0); err != nil {
return err
}
for i := range in.Endpoints {
ep := &in.Endpoints[i]
out.Endpoints = append(out.Endpoints, net.JoinHostPort(ep.IP, strconv.Itoa(ep.Port)))
}
return nil
},
func(in *Endpoints, out *newer.Endpoints, s conversion.Scope) error {
if err := s.Convert(&in.TypeMeta, &out.TypeMeta, 0); err != nil {
return err
}
if err := s.Convert(&in.TypeMeta, &out.ObjectMeta, 0); err != nil {
return err
}
if err := s.Convert(&in.Protocol, &out.Protocol, 0); err != nil {
return err
}
for i := range in.Endpoints {
out.Endpoints = append(out.Endpoints, newer.Endpoint{})
ep := &out.Endpoints[i]
host, port, err := net.SplitHostPort(in.Endpoints[i])
if err != nil {
return err
}
ep.IP = host
pn, err := strconv.Atoi(port)
if err != nil {
return err
}
ep.Port = pn
}
return nil
},
)
if err != nil {
// If one of the conversion functions is malformed, detect it immediately.
Expand Down
69 changes: 69 additions & 0 deletions pkg/api/v1beta2/conversion_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,3 +212,72 @@ func TestContainerConversion(t *testing.T) {
}
}
}

func TestEndpointsConversion(t *testing.T) {
testCases := []struct {
given current.Endpoints
expected newer.Endpoints
}{
{
given: current.Endpoints{
TypeMeta: current.TypeMeta{
ID: "empty",
},
Protocol: current.ProtocolTCP,
Endpoints: []string{},
},
expected: newer.Endpoints{
Protocol: newer.ProtocolTCP,
Endpoints: []newer.Endpoint{},
},
},
{
given: current.Endpoints{
TypeMeta: current.TypeMeta{
ID: "one",
},
Protocol: current.ProtocolTCP,
Endpoints: []string{"1.2.3.4:88"},
},
expected: newer.Endpoints{
Protocol: newer.ProtocolTCP,
Endpoints: []newer.Endpoint{{IP: "1.2.3.4", Port: 88}},
},
},
{
given: current.Endpoints{
TypeMeta: current.TypeMeta{
ID: "several",
},
Protocol: current.ProtocolUDP,
Endpoints: []string{"1.2.3.4:88", "1.2.3.4:89", "1.2.3.4:90"},
},
expected: newer.Endpoints{
Protocol: newer.ProtocolUDP,
Endpoints: []newer.Endpoint{{IP: "1.2.3.4", Port: 88}, {IP: "1.2.3.4", Port: 89}, {IP: "1.2.3.4", Port: 90}},
},
},
}

for i, tc := range testCases {
// Convert versioned -> internal.
got := newer.Endpoints{}
if err := newer.Scheme.Convert(&tc.given, &got); err != nil {
t.Errorf("[Case: %d] Unexpected error: %v", i, err)
continue
}
if got.Protocol != tc.expected.Protocol || !newer.Semantic.DeepEqual(got.Endpoints, tc.expected.Endpoints) {
t.Errorf("[Case: %d] Expected %v, got %v", i, tc.expected, got)
}

// Convert internal -> versioned.
got2 := current.Endpoints{}
if err := newer.Scheme.Convert(&got, &got2); err != nil {
t.Errorf("[Case: %d] Unexpected error: %v", i, err)
continue
}
if got2.Protocol != tc.given.Protocol || !newer.Semantic.DeepEqual(got2.Endpoints, tc.given.Endpoints) {
t.Errorf("[Case: %d] Expected %v, got %v", i, tc.given, got2)
}
}
}
5 changes: 5 additions & 0 deletions pkg/api/v1beta2/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,5 +87,10 @@ func init() {
obj.Type = SecretTypeOpaque
}
},
func(obj *Endpoints) {
if obj.Protocol == "" {
obj.Protocol = "TCP"
}
},
)
}
10 changes: 10 additions & 0 deletions pkg/api/v1beta2/defaults_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,13 @@ func TestSetDefaultSecret(t *testing.T) {
t.Errorf("Expected secret type %v, got %v", current.SecretTypeOpaque, s2.Type)
}
}

func TestSetDefaulEndpointsProtocol(t *testing.T) {
in := &current.Endpoints{}
obj := roundTrip(t, runtime.Object(in))
out := obj.(*current.Endpoints)

if out.Protocol != current.ProtocolTCP {
t.Errorf("Expected protocol %s, got %s", current.ProtocolTCP, out.Protocol)
}
}
5 changes: 4 additions & 1 deletion pkg/api/v1beta2/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,10 @@ type Service struct {
// Endpoints is a collection of endpoints that implement the actual service, for example:
// Name: "mysql", Endpoints: ["10.10.1.1:1909", "10.10.2.2:8834"]
type Endpoints struct {
TypeMeta `json:",inline"`
TypeMeta `json:",inline"`
// Optional: The IP protocol for these endpoints. Supports "TCP" and
// "UDP". Defaults to "TCP".
Protocol Protocol `json:"protocol,omitempty" description:"IP protocol for endpoint ports; must be UDP or TCP; TCP if unspecified"`
Endpoints []string `json:"endpoints,omitempty" description:"list of endpoints corresponding to a service, of the form address:port, such as 10.10.1.1:1909"`
}

Expand Down
5 changes: 5 additions & 0 deletions pkg/api/v1beta3/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,10 @@ func init() {
obj.Type = SecretTypeOpaque
}
},
func(obj *Endpoints) {
if obj.Protocol == "" {
obj.Protocol = "TCP"
}
},
)
}