Skip to content

Commit

Permalink
Add check for endpoint count is zero
Browse files Browse the repository at this point in the history 
Add check to make sure neither the total number of endpoints from EPS or
the one from calculated endpoint list is 0. If not, the syncer will
enter the error state.
  • Loading branch information
@sawsa307
sawsa307 committed Oct 27, 2022
1 parent d745701 commit 07b4afd
Show file tree
Hide file tree
Showing 2 changed files with 277 additions and 0 deletions.
29 changes: 29 additions & 0 deletions pkg/neg/syncers/transaction.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,9 @@ func (s *transactionSyncer) syncInternalImpl() error {
}
endpointsData := negtypes.EndpointsDataFromEndpoints(ep.(*apiv1.Endpoints))
targetMap, endpointPodMap, err = s.endpointsCalculator.CalculateEndpoints(endpointsData, currentMap)
if s.isEndpointCountZero(endpointsData, endpointPodMap) {
s.setErrorState()
}
if err != nil {
return fmt.Errorf("endpoints calculation error in mode %q, err: %w", s.endpointsCalculator.Mode(), err)
}
Expand Down Expand Up @@ -346,6 +349,32 @@ func (s *transactionSyncer) ensureNetworkEndpointGroups() error {
return utilerrors.NewAggregate(errList)
}

// endpointCountIsZero returns true if endpoint count from EPS or the one from calculated endpoint list is 0
func (s *transactionSyncer) isEndpointCountZero(eds []negtypes.EndpointsData, endpointPodMap negtypes.EndpointPodMap) bool {
// Endpoint count from EndpointData/EPS
countFromEndpointData := 0
for _, ed := range eds {
for _, endpointAddress := range ed.Addresses {
if endpointAddress.Ready {
countFromEndpointData += 1
}
}
}

// Endpoint count from EndpointPodMap/Endpoint list
countFromPodMap := len(endpointPodMap)

if countFromEndpointData == 0 {
s.logger.Info("Detected error when checking if endpoint count from EPS, expecting a non-zero value", "endpointData", eds)
return true
}
if countFromPodMap == 0 {
s.logger.Info("Detected error when checking if endpoint count from calculated endpoint list, expecting a non-zero value", "endpointPodMap", endpointPodMap)
return true
}
return false
}

// syncNetworkEndpoints spins off go routines to execute NEG operations
func (s *transactionSyncer) syncNetworkEndpoints(addEndpoints, removeEndpoints map[string]negtypes.NetworkEndpointSet) error {
syncFunc := func(endpointMap map[string]negtypes.NetworkEndpointSet, operation transactionOp) error {
Expand Down
248 changes: 248 additions & 0 deletions pkg/neg/syncers/transaction_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1409,6 +1409,254 @@ func TestUnknownNodes(t *testing.T) {
}
}

func TestIsEndpointCountZero(t *testing.T) {
t.Parallel()
_, transactionSyncer := newTestTransactionSyncer(negtypes.NewAdapter(gce.NewFakeGCECloud(gce.DefaultTestClusterValues())), negtypes.VmIpPortEndpointType, false, true)

instance1 := testInstance1
testPortName := "port1"
testServiceNamespace := "namespace"
port80 := int32(80)

testCases := []struct {
desc string
endpointsData []negtypes.EndpointsData
endpointPodMap map[negtypes.NetworkEndpoint]types.NamespacedName
expect bool
}{
{
desc: "endpointData has zero ready endpoint",
endpointsData: []negtypes.EndpointsData{
{
Meta: &metav1.ObjectMeta{
Name: testServiceName + "-1",
Namespace: testServiceNamespace,
},
Ports: []negtypes.PortData{
{
Name: testPortName,
Port: port80,
},
},
Addresses: []negtypes.AddressData{
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: "pod2",
},
NodeName: &instance1,
Addresses: []string{"10.100.1.2"},
Ready: false,
},
},
},
},
endpointPodMap: map[negtypes.NetworkEndpoint]types.NamespacedName{
{
IP: "10.100.1.3",
Port: "80",
Node: instance1,
}: {
Namespace: testServiceNamespace,
Name: "pod5",
},
{
IP: "10.100.1.4",
Port: "80",
Node: instance1,
}: {
Namespace: testServiceNamespace,
Name: "pod6",
},
{
IP: "10.100.1.5",
Port: "80",
Node: instance1,
}: {
Namespace: testServiceNamespace,
Name: "pod9",
},
},
expect: true,
},
{
desc: "endpointPodMap has zero endpoint",
endpointsData: []negtypes.EndpointsData{
{
Meta: &metav1.ObjectMeta{
Name: testServiceName + "-1",
Namespace: testServiceNamespace,
},
Ports: []negtypes.PortData{
{
Name: testPortName,
Port: port80,
},
},
Addresses: []negtypes.AddressData{
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: "pod2",
},
NodeName: &instance1,
Addresses: []string{"10.100.1.2"},
Ready: false,
},
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: "pod5",
},
NodeName: &instance1,
Addresses: []string{"10.100.1.3"},
Ready: true,
},
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: "pod6",
},
NodeName: &instance1,
Addresses: []string{"10.100.1.4"},
Ready: true,
},
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: "pod9",
},
NodeName: &instance1,
Addresses: []string{"10.100.1.5"},
Ready: true,
},
},
},
},
endpointPodMap: map[negtypes.NetworkEndpoint]types.NamespacedName{},
expect: true,
},
{
desc: "endpointData and endpointPodMap both have zero ready endpoint",
endpointsData: []negtypes.EndpointsData{
{
Meta: &metav1.ObjectMeta{
Name: testServiceName + "-1",
Namespace: testServiceNamespace,
},
Ports: []negtypes.PortData{
{
Name: testPortName,
Port: port80,
},
},
Addresses: []negtypes.AddressData{
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: "pod2",
},
NodeName: &instance1,
Addresses: []string{"10.100.1.2"},
Ready: false,
},
},
},
},
endpointPodMap: map[negtypes.NetworkEndpoint]types.NamespacedName{},
expect: true,
},
{
desc: "endpointData and endpointPodMap both have non-zero endpoints",
endpointsData: []negtypes.EndpointsData{
{
Meta: &metav1.ObjectMeta{
Name: testServiceName + "-1",
Namespace: testServiceNamespace,
},
Ports: []negtypes.PortData{
{
Name: testPortName,
Port: port80,
},
},
Addresses: []negtypes.AddressData{
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: "pod2",
},
NodeName: &instance1,
Addresses: []string{"10.100.1.2"},
Ready: false,
},
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: "pod5",
},
NodeName: &instance1,
Addresses: []string{"10.100.1.3"},
Ready: true,
},
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: "pod6",
},
NodeName: &instance1,
Addresses: []string{"10.100.1.4"},
Ready: true,
},
{
TargetRef: &corev1.ObjectReference{
Namespace: testServiceNamespace,
Name: "pod9",
},
NodeName: &instance1,
Addresses: []string{"10.100.1.5"},
Ready: true,
},
},
},
},
endpointPodMap: map[negtypes.NetworkEndpoint]types.NamespacedName{
{
IP: "10.100.1.3",
Port: "80",
Node: instance1,
}: {
Namespace: testServiceNamespace,
Name: "pod5",
},
{
IP: "10.100.1.4",
Port: "80",
Node: instance1,
}: {
Namespace: testServiceNamespace,
Name: "pod6",
},
{
IP: "10.100.1.5",
Port: "80",
Node: instance1,
}: {
Namespace: testServiceNamespace,
Name: "pod9",
},
},
expect: false,
},
}

for _, tc := range testCases {
if got := transactionSyncer.isEndpointCountZero(tc.endpointsData, tc.endpointPodMap); got != tc.expect {
t.Errorf("For case %q, expect equal to be %v, but got %v", tc.desc, tc.expect, got)
}
}
}

func newL4ILBTestTransactionSyncer(fakeGCE negtypes.NetworkEndpointGroupCloud, mode negtypes.EndpointsCalculatorMode, enableEndpointSlices bool) (negtypes.NegSyncer, *transactionSyncer) {
negsyncer, ts := newTestTransactionSyncer(fakeGCE, negtypes.VmIpEndpointType, false, enableEndpointSlices)
ts.endpointsCalculator = GetEndpointsCalculator(ts.nodeLister, ts.podLister, ts.zoneGetter, ts.NegSyncerKey, mode, klog.TODO())
Expand Down

0 comments on commit 07b4afd

Please sign in to comment.