Clean up Endpoint / BaseEndpointInfo a bit

Remove NodeName, which was unused because we only care about IsLocal
which was tracked separately.

Remove Zone, which was unused because it's from the old topology
system?

Fix up some comments which still referred to Endpoints vs
EndpointSlice differences.

Also remove an unhelpful helper function in endpoints_test.go
This commit is contained in:
Dan Winship 2023-09-26 10:01:33 -04:00
parent 2879ec10d5
commit e3357d0c5f
6 changed files with 29 additions and 86 deletions

View File

@ -50,27 +50,16 @@ type BaseEndpointInfo struct {
// ZoneHints represent the zone hints for the endpoint. This is based on
// endpoint.hints.forZones[*].name in the EndpointSlice API.
ZoneHints sets.Set[string]
// Ready indicates whether this endpoint is ready and NOT terminating.
// For pods, this is true if a pod has a ready status and a nil deletion timestamp.
// This is only set when watching EndpointSlices. If using Endpoints, this is always
// true since only ready endpoints are read from Endpoints.
// TODO: Ready can be inferred from Serving and Terminating below when enabled by default.
// Ready indicates whether this endpoint is ready and NOT terminating, unless
// PublishNotReadyAddresses is set on the service, in which case it will just
// always be true.
Ready bool
// Serving indiciates whether this endpoint is ready regardless of its terminating state.
// Serving indicates whether this endpoint is ready regardless of its terminating state.
// For pods this is true if it has a ready status regardless of its deletion timestamp.
// This is only set when watching EndpointSlices. If using Endpoints, this is always
// true since only ready endpoints are read from Endpoints.
Serving bool
// Terminating indicates whether this endpoint is terminating.
// For pods this is true if it has a non-nil deletion timestamp.
// This is only set when watching EndpointSlices. If using Endpoints, this is always
// false since terminating endpoints are always excluded from Endpoints.
Terminating bool
// NodeName is the name of the node this endpoint belongs to
NodeName string
// Zone is the name of the zone this endpoint belongs to
Zone string
}
var _ Endpoint = &BaseEndpointInfo{}
@ -117,18 +106,7 @@ func (info *BaseEndpointInfo) Port() (int, error) {
return proxyutil.PortPart(info.Endpoint)
}
// GetNodeName returns the NodeName for this endpoint.
func (info *BaseEndpointInfo) GetNodeName() string {
return info.NodeName
}
// GetZone returns the Zone for this endpoint.
func (info *BaseEndpointInfo) GetZone() string {
return info.Zone
}
func newBaseEndpointInfo(IP, nodeName, zone string, port int, isLocal bool,
ready, serving, terminating bool, zoneHints sets.Set[string]) *BaseEndpointInfo {
func newBaseEndpointInfo(IP string, port int, isLocal, ready, serving, terminating bool, zoneHints sets.Set[string]) *BaseEndpointInfo {
return &BaseEndpointInfo{
Endpoint: net.JoinHostPort(IP, strconv.Itoa(port)),
IsLocal: isLocal,
@ -136,8 +114,6 @@ func newBaseEndpointInfo(IP, nodeName, zone string, port int, isLocal bool,
Serving: serving,
Terminating: terminating,
ZoneHints: zoneHints,
NodeName: nodeName,
Zone: zone,
}
}

View File

@ -1654,7 +1654,11 @@ func TestCheckoutChanges(t *testing.T) {
expectedChanges: []*endpointsChange{{
previous: EndpointsMap{},
current: EndpointsMap{
svcPortName0: []Endpoint{newTestEp("10.0.1.1:80", "host1", true, true, false), newTestEp("10.0.1.2:80", "host1", false, true, true), newTestEp("10.0.1.3:80", "host1", false, false, false)},
svcPortName0: []Endpoint{
&BaseEndpointInfo{Endpoint: "10.0.1.1:80", Ready: true, Serving: true, Terminating: false},
&BaseEndpointInfo{Endpoint: "10.0.1.2:80", Ready: false, Serving: true, Terminating: true},
&BaseEndpointInfo{Endpoint: "10.0.1.3:80", Ready: false, Serving: false, Terminating: false},
},
},
}},
appliedSlices: []*discovery.EndpointSlice{},
@ -1666,11 +1670,23 @@ func TestCheckoutChanges(t *testing.T) {
endpointsChangeTracker: NewEndpointsChangeTracker("", nil, v1.IPv4Protocol, nil, nil),
expectedChanges: []*endpointsChange{{
previous: EndpointsMap{
svcPortName0: []Endpoint{newTestEp("10.0.1.1:80", "host1", true, true, false), newTestEp("10.0.1.2:80", "host1", true, true, false), newTestEp("10.0.1.3:80", "host1", false, false, false)},
svcPortName1: []Endpoint{newTestEp("10.0.1.1:443", "host1", true, true, false), newTestEp("10.0.1.2:443", "host1", true, true, false), newTestEp("10.0.1.3:443", "host1", false, false, false)},
svcPortName0: []Endpoint{
&BaseEndpointInfo{Endpoint: "10.0.1.1:80", Ready: true, Serving: true, Terminating: false},
&BaseEndpointInfo{Endpoint: "10.0.1.2:80", Ready: true, Serving: true, Terminating: false},
&BaseEndpointInfo{Endpoint: "10.0.1.3:80", Ready: false, Serving: false, Terminating: false},
},
svcPortName1: []Endpoint{
&BaseEndpointInfo{Endpoint: "10.0.1.1:443", Ready: true, Serving: true, Terminating: false},
&BaseEndpointInfo{Endpoint: "10.0.1.2:443", Ready: true, Serving: true, Terminating: false},
&BaseEndpointInfo{Endpoint: "10.0.1.3:443", Ready: false, Serving: false, Terminating: false},
},
},
current: EndpointsMap{
svcPortName0: []Endpoint{newTestEp("10.0.1.1:80", "host1", true, true, false), newTestEp("10.0.1.2:80", "host1", true, true, false), newTestEp("10.0.1.3:80", "host1", false, false, false)},
svcPortName0: []Endpoint{
&BaseEndpointInfo{Endpoint: "10.0.1.1:80", Ready: true, Serving: true, Terminating: false},
&BaseEndpointInfo{Endpoint: "10.0.1.2:80", Ready: true, Serving: true, Terminating: false},
&BaseEndpointInfo{Endpoint: "10.0.1.3:80", Ready: false, Serving: false, Terminating: false},
},
},
}},
appliedSlices: []*discovery.EndpointSlice{
@ -1743,14 +1759,6 @@ func compareEndpointsMapsStr(t *testing.T, newMap EndpointsMap, expected map[Ser
}
}
func newTestEp(ep, host string, ready, serving, terminating bool) *BaseEndpointInfo {
endpointInfo := &BaseEndpointInfo{Endpoint: ep, Ready: ready, Serving: serving, Terminating: terminating}
if host != "" {
endpointInfo.NodeName = host
}
return endpointInfo
}
func initializeCache(endpointSliceCache *EndpointSliceCache, endpointSlices []*discovery.EndpointSlice) {
for _, endpointSlice := range endpointSlices {
endpointSliceCache.updatePending(endpointSlice, false)

View File

@ -294,19 +294,9 @@ func (cache *EndpointSliceCache) addEndpoints(svcPortName *ServicePortName, port
continue
}
isLocal := false
nodeName := ""
if endpoint.NodeName != nil {
isLocal = cache.isLocal(*endpoint.NodeName)
nodeName = *endpoint.NodeName
}
isLocal := endpoint.NodeName != nil && cache.isLocal(*endpoint.NodeName)
zone := ""
if endpoint.Zone != nil {
zone = *endpoint.Zone
}
endpointInfo := newBaseEndpointInfo(endpoint.Addresses[0], nodeName, zone, portNum, isLocal,
endpointInfo := newBaseEndpointInfo(endpoint.Addresses[0], portNum, isLocal,
endpoint.Ready, endpoint.Serving, endpoint.Terminating, endpoint.ZoneHints)
// This logic ensures we're deduplicating potential overlapping endpoints

View File

@ -235,7 +235,6 @@ func TestEndpointInfoByServicePort(t *testing.T) {
"10.0.1.1:80": &BaseEndpointInfo{
Endpoint: "10.0.1.1:80",
IsLocal: false,
NodeName: "host2",
Ready: true,
Serving: true,
Terminating: false,
@ -243,7 +242,6 @@ func TestEndpointInfoByServicePort(t *testing.T) {
"10.0.1.2:80": &BaseEndpointInfo{
Endpoint: "10.0.1.2:80",
IsLocal: true,
NodeName: "host1",
Ready: true,
Serving: true,
Terminating: false,
@ -251,7 +249,6 @@ func TestEndpointInfoByServicePort(t *testing.T) {
"10.0.1.3:80": &BaseEndpointInfo{
Endpoint: "10.0.1.3:80",
IsLocal: false,
NodeName: "host2",
Ready: true,
Serving: true,
Terminating: false,
@ -271,7 +268,6 @@ func TestEndpointInfoByServicePort(t *testing.T) {
"10.0.1.1:80": &BaseEndpointInfo{
Endpoint: "10.0.1.1:80",
IsLocal: false,
NodeName: "host2",
Ready: true,
Serving: true,
Terminating: false,
@ -279,7 +275,6 @@ func TestEndpointInfoByServicePort(t *testing.T) {
"10.0.1.2:80": &BaseEndpointInfo{
Endpoint: "10.0.1.2:80",
IsLocal: true,
NodeName: "host1",
Ready: true,
Serving: true,
Terminating: false,
@ -287,7 +282,6 @@ func TestEndpointInfoByServicePort(t *testing.T) {
"10.0.1.1:8080": &BaseEndpointInfo{
Endpoint: "10.0.1.1:8080",
IsLocal: false,
NodeName: "host2",
Ready: true,
Serving: true,
Terminating: false,
@ -295,7 +289,6 @@ func TestEndpointInfoByServicePort(t *testing.T) {
"10.0.1.2:8080": &BaseEndpointInfo{
Endpoint: "10.0.1.2:8080",
IsLocal: true,
NodeName: "host1",
Ready: true,
Serving: true,
Terminating: false,

View File

@ -110,19 +110,14 @@ type Endpoint interface {
String() string
// GetIsLocal returns true if the endpoint is running in same host as kube-proxy, otherwise returns false.
GetIsLocal() bool
// IsReady returns true if an endpoint is ready and not terminating.
// This is only set when watching EndpointSlices. If using Endpoints, this is always
// true since only ready endpoints are read from Endpoints.
// IsReady returns true if an endpoint is ready and not terminating, or
// if PublishNotReadyAddresses is set on the service.
IsReady() bool
// IsServing returns true if an endpoint is ready. It does not account
// for terminating state.
// This is only set when watching EndpointSlices. If using Endpoints, this is always
// true since only ready endpoints are read from Endpoints.
IsServing() bool
// IsTerminating returns true if an endpoint is terminating. For pods,
// that is any pod with a deletion timestamp.
// This is only set when watching EndpointSlices. If using Endpoints, this is always
// false since terminating endpoints are always excluded from Endpoints.
IsTerminating() bool
// GetZoneHints returns the zone hint for the endpoint. This is based on
// endpoint.hints.forZones[0].name in the EndpointSlice API.
@ -131,10 +126,6 @@ type Endpoint interface {
IP() string
// Port returns the Port part of the endpoint.
Port() (int, error)
// GetNodeName returns the node name for the endpoint
GetNodeName() string
// GetZone returns the zone for the endpoint
GetZone() string
}
// ServiceEndpoint is used to identify a service and one of its endpoint pair.

View File

@ -329,21 +329,6 @@ func (info *endpointInfo) Port() (int, error) {
return int(info.port), nil
}
// Equal is part of proxy.Endpoint interface.
func (info *endpointInfo) Equal(other proxy.Endpoint) bool {
return info.String() == other.String() && info.GetIsLocal() == other.GetIsLocal()
}
// GetNodeName returns the NodeName for this endpoint.
func (info *endpointInfo) GetNodeName() string {
return ""
}
// GetZone returns the Zone for this endpoint.
func (info *endpointInfo) GetZone() string {
return ""
}
// Uses mac prefix and IPv4 address to return a mac address
// This ensures mac addresses are unique for proper load balancing
// There is a possibility of MAC collisions but this Mac address is used for remote endpoints only