Skip to content

Commit

Permalink
Merge pull request #2024 from swetharepakula/cp-l4-dual-stack-fix
Browse files Browse the repository at this point in the history 
[Cherry-pick #2012 to 1.22] Process all endpoints and allow endpoint calculators to filter
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Mar 18, 2023
2 parents 10e7085 + 7ab6d90 commit 0f437c7
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 74 deletions.
113 changes: 65 additions & 48 deletions pkg/neg/syncers/transaction_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"google.golang.org/api/compute/v1"
"google.golang.org/api/googleapi"
corev1 "k8s.io/api/core/v1"
discovery "k8s.io/api/discovery/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -2026,18 +2027,20 @@ func TestIsValidEPField(t *testing.T) {
Namespace: testServiceNamespace,
Name: testPodName1,
},
NodeName: &instance1,
Addresses: []string{testIP1},
Ready: true,
NodeName: &instance1,
Addresses: []string{testIP1},
Ready: true,
AddressType: discovery.AddressTypeIPv4,
},
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: testPodName2,
},
NodeName: &instance1,
Addresses: []string{testIP2},
Ready: true,
NodeName: &instance1,
Addresses: []string{testIP2},
Ready: true,
AddressType: discovery.AddressTypeIPv4,
},
},
},
Expand All @@ -2058,18 +2061,20 @@ func TestIsValidEPField(t *testing.T) {
Namespace: testServiceNamespace,
Name: testPodName3,
},
NodeName: &instance2,
Addresses: []string{testIP3},
Ready: true,
NodeName: &instance2,
Addresses: []string{testIP3},
Ready: true,
AddressType: discovery.AddressTypeIPv4,
},
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: testPodName4,
},
NodeName: &instance4,
Addresses: []string{testIP4},
Ready: true,
NodeName: &instance4,
Addresses: []string{testIP4},
Ready: true,
AddressType: discovery.AddressTypeIPv4,
},
},
},
Expand Down Expand Up @@ -2097,18 +2102,20 @@ func TestIsValidEPField(t *testing.T) {
Namespace: testServiceNamespace,
Name: testPodName1,
},
NodeName: nil,
Addresses: []string{testIP1},
Ready: true,
NodeName: nil,
Addresses: []string{testIP1},
Ready: true,
AddressType: discovery.AddressTypeIPv4,
},
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: testPodName2,
},
NodeName: &instance1,
Addresses: []string{testIP2},
Ready: true,
NodeName: &instance1,
Addresses: []string{testIP2},
Ready: true,
AddressType: discovery.AddressTypeIPv4,
},
},
},
Expand All @@ -2129,18 +2136,20 @@ func TestIsValidEPField(t *testing.T) {
Namespace: testServiceNamespace,
Name: testPodName3,
},
NodeName: &instance2,
Addresses: []string{testIP3},
Ready: true,
NodeName: &instance2,
Addresses: []string{testIP3},
Ready: true,
AddressType: discovery.AddressTypeIPv4,
},
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: testPodName4,
},
NodeName: &instance4,
Addresses: []string{testIP4},
Ready: true,
NodeName: &instance4,
Addresses: []string{testIP4},
Ready: true,
AddressType: discovery.AddressTypeIPv4,
},
},
},
Expand Down Expand Up @@ -2168,18 +2177,20 @@ func TestIsValidEPField(t *testing.T) {
Namespace: testServiceNamespace,
Name: testPodName1,
},
NodeName: &emptyNodeName,
Addresses: []string{testIP1},
Ready: true,
NodeName: &emptyNodeName,
Addresses: []string{testIP1},
Ready: true,
AddressType: discovery.AddressTypeIPv4,
},
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: testPodName2,
},
NodeName: &instance1,
Addresses: []string{testIP2},
Ready: true,
NodeName: &instance1,
Addresses: []string{testIP2},
Ready: true,
AddressType: discovery.AddressTypeIPv4,
},
},
},
Expand All @@ -2200,18 +2211,20 @@ func TestIsValidEPField(t *testing.T) {
Namespace: testServiceNamespace,
Name: testPodName3,
},
NodeName: &instance2,
Addresses: []string{testIP3},
Ready: true,
NodeName: &instance2,
Addresses: []string{testIP3},
Ready: true,
AddressType: discovery.AddressTypeIPv4,
},
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: testPodName4,
},
NodeName: &instance4,
Addresses: []string{testIP4},
Ready: true,
NodeName: &instance4,
Addresses: []string{testIP4},
Ready: true,
AddressType: discovery.AddressTypeIPv4,
},
},
},
Expand Down Expand Up @@ -2239,18 +2252,20 @@ func TestIsValidEPField(t *testing.T) {
Namespace: testServiceNamespace,
Name: testPodName1,
},
NodeName: &emptyZoneNodeName,
Addresses: []string{testIP1},
Ready: true,
NodeName: &emptyZoneNodeName,
Addresses: []string{testIP1},
Ready: true,
AddressType: discovery.AddressTypeIPv4,
},
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: testPodName2,
},
NodeName: &instance1,
Addresses: []string{testIP2},
Ready: true,
NodeName: &instance1,
Addresses: []string{testIP2},
Ready: true,
AddressType: discovery.AddressTypeIPv4,
},
},
},
Expand All @@ -2271,18 +2286,20 @@ func TestIsValidEPField(t *testing.T) {
Namespace: testServiceNamespace,
Name: testPodName3,
},
NodeName: &instance2,
Addresses: []string{testIP3},
Ready: true,
NodeName: &instance2,
Addresses: []string{testIP3},
Ready: true,
AddressType: discovery.AddressTypeIPv4,
},
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: testPodName4,
},
NodeName: &instance4,
Addresses: []string{testIP4},
Ready: true,
NodeName: &instance4,
Addresses: []string{testIP4},
Ready: true,
AddressType: discovery.AddressTypeIPv4,
},
},
},
Expand Down
5 changes: 5 additions & 0 deletions pkg/neg/syncers/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
apiv1 "k8s.io/api/core/v1"
discovery "k8s.io/api/discovery/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/tools/cache"
Expand Down Expand Up @@ -251,6 +252,10 @@ func toZoneNetworkEndpointMap(eds []negtypes.EndpointsData, zoneGetter negtypes.
foundMatchingPort = true

for _, endpointAddress := range ed.Addresses {
if endpointAddress.AddressType != discovery.AddressTypeIPv4 {
klog.Infof("Skipping non IPv4 address: %q, in endpoint slice %s/%s", endpointAddress.Addresses, ed.Meta.Namespace, ed.Meta.Name)
continue
}
if endpointAddress.NodeName == nil || len(*endpointAddress.NodeName) == 0 {
klog.V(2).Infof("Endpoint %q in Endpoints %s/%s does not have an associated node. Skipping", endpointAddress.Addresses, ed.Meta.Namespace, ed.Meta.Name)
return nil, nil, dupCount, ErrEPMissingNodeName
Expand Down
44 changes: 44 additions & 0 deletions pkg/neg/syncers/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1412,5 +1412,49 @@ func getTestEndpointSlices(name, namespace string) []*discovery.EndpointSlice {
},
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: name + "-4",
Namespace: namespace,
Labels: map[string]string{
discovery.LabelServiceName: name,
},
},
AddressType: discovery.AddressTypeIPv6,
Endpoints: []discovery.Endpoint{
{
Addresses: []string{"aa:aa"},
NodeName: &instance3,
TargetRef: &v1.ObjectReference{
Namespace: namespace,
Name: "pod10",
},
},
{
Addresses: []string{"aa:ab"},
NodeName: &instance4,
TargetRef: &v1.ObjectReference{
Namespace: namespace,
Name: "pod11",
},
},
{
Addresses: []string{"aa:ac"},
NodeName: &instance4,
TargetRef: &v1.ObjectReference{
Namespace: namespace,
Name: "pod12",
},
Conditions: discovery.EndpointConditions{Ready: &notReady},
},
},
Ports: []discovery.EndpointPort{
{
Name: &emptyNamedPort,
Port: &port80,
Protocol: &protocolTCP,
},
},
},
}
}
15 changes: 6 additions & 9 deletions pkg/neg/types/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,21 +308,18 @@ type PortData struct {
}

type AddressData struct {
TargetRef *apiv1.ObjectReference
NodeName *string
Addresses []string
Ready bool
TargetRef *apiv1.ObjectReference
NodeName *string
Addresses []string
Ready bool
AddressType discovery.AddressType
}

// Converts API EndpointSlice list to the EndpointsData abstraction.
// Terminating endpoints are ignored.
func EndpointsDataFromEndpointSlices(slices []*discovery.EndpointSlice) []EndpointsData {
result := make([]EndpointsData, 0, len(slices))
for _, slice := range slices {
if slice.AddressType != discovery.AddressTypeIPv4 {
// Neg Controller can only attach IPv4 endpoints
continue
}
ports := make([]PortData, 0)
addresses := make([]AddressData, 0)
for _, port := range slice.Ports {
Expand All @@ -343,7 +340,7 @@ func EndpointsDataFromEndpointSlices(slices []*discovery.EndpointSlice) []Endpoi
nodeNameFromTopology := ep.DeprecatedTopology[apiv1.LabelHostname]
nodeName = &nodeNameFromTopology
}
addresses = append(addresses, AddressData{TargetRef: ep.TargetRef, NodeName: nodeName, Addresses: ep.Addresses, Ready: ready})
addresses = append(addresses, AddressData{TargetRef: ep.TargetRef, NodeName: nodeName, Addresses: ep.Addresses, Ready: ready, AddressType: slice.AddressType})
}
result = append(result, EndpointsData{Meta: &slice.ObjectMeta, Ports: ports, Addresses: addresses})
}
Expand Down
23 changes: 6 additions & 17 deletions pkg/neg/types/types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -595,20 +595,18 @@ func TestEndpointsDataFromEndpointSlices(t *testing.T) {

endpointsData := EndpointsDataFromEndpointSlices(endpointSlices)

if len(endpointsData) != 2 {
t.Errorf("Expected the same number of endpoints subsets and endpoints data, got %d endpoints data for 2 subsets", len(endpointsData))
if len(endpointsData) != 3 {
t.Errorf("Expected the same number of endpoints subsets and endpoints data, got %d endpoints data for 3 subsets", len(endpointsData))
}
// This test expects that all the valid EPS are at the beginning
for i, slice := range endpointSlices {
if slice.AddressType != discovery.AddressTypeIPv4 {
continue
}
addressType := slice.AddressType
for j, port := range slice.Ports {
ValidatePortData(endpointsData[i].Ports[j], *port.Port, *port.Name, t)
}
terminatingEndpointsNumber := 0
for _, endpoint := range slice.Endpoints {
found := CheckIfAddressIsPresentInData(endpointsData[i].Addresses, endpoint.Conditions.Ready == nil || *endpoint.Conditions.Ready, endpoint.Addresses[0], endpoint.TargetRef, endpoint.NodeName)
found := CheckIfAddressIsPresentInData(endpointsData[i].Addresses, endpoint.Conditions.Ready == nil || *endpoint.Conditions.Ready, endpoint.Addresses[0], endpoint.TargetRef, endpoint.NodeName, addressType)
if endpoint.Conditions.Terminating != nil && *endpoint.Conditions.Terminating {
terminatingEndpointsNumber++
if found {
Expand Down Expand Up @@ -732,20 +730,11 @@ func ValidatePortData(portData PortData, port int32, name string, t *testing.T)
}
}

func CheckIfAddressIsPresentInData(addressData []AddressData, ready bool, address string, targetRef *v1.ObjectReference, nodeName *string) bool {
func CheckIfAddressIsPresentInData(addressData []AddressData, ready bool, address string, targetRef *v1.ObjectReference, nodeName *string, addressType discovery.AddressType) bool {
for _, data := range addressData {
if data.Ready == ready && len(data.Addresses) == 1 && data.Addresses[0] == address && data.TargetRef == targetRef && data.NodeName == nodeName {
if data.Ready == ready && len(data.Addresses) == 1 && data.Addresses[0] == address && data.TargetRef == targetRef && data.NodeName == nodeName && data.AddressType == addressType {
return true
}
}
return false
}

func ValidateAddressDataForEndpointsAddresses(addressData []AddressData, addresses []v1.EndpointAddress, ready bool, t *testing.T) {
for _, addr := range addresses {
found := CheckIfAddressIsPresentInData(addressData, ready, addr.IP, addr.TargetRef, addr.NodeName)
if !found {
t.Errorf("Endpoint address %v couldn't be found in Address Data %v", addr, addressData)
}
}
}

0 comments on commit 0f437c7

Please sign in to comment.