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
endpointData or the one from endpointPodMap is 0. If not, the syncer will
enter the error state.
  • Loading branch information
@sawsa307
sawsa307 committed Nov 9, 2022
1 parent 839a894 commit ad2eb11
Show file tree
Hide file tree
Showing 2 changed files with 245 additions and 0 deletions.
24 changes: 24 additions & 0 deletions pkg/neg/syncers/transaction.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,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 @@ -352,6 +355,27 @@ func (s *transactionSyncer) ensureNetworkEndpointGroups() error {
return utilerrors.NewAggregate(errList)
}

// endpointCountIsZero returns true if endpoint count from endpointData or the one from endpointPodMap is 0
func (s *transactionSyncer) isEndpointCountZero(eds []negtypes.EndpointsData, endpointPodMap negtypes.EndpointPodMap) bool {
// Endpoint count from EndpointPodMap/Endpoint list
countFromPodMap := len(endpointPodMap)
if countFromPodMap == 0 {
s.logger.Info("Detected error when checking endpoint count from endpointPodMap, expecting a non-zero value", "endpointPodMap", endpointPodMap)
return true
}

// Endpoint count from EndpointData/EPS
countFromEndpointData := 0
for _, ed := range eds {
countFromEndpointData += len(ed.Addresses)
}
if countFromEndpointData == 0 {
s.logger.Info("Detected error when checking endpoint count from endpointData, expecting a non-zero value", "endpointData", eds)
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
221 changes: 221 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,227 @@ 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{},
},
},
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: true,
},
{
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 endpoint",
endpointsData: []negtypes.EndpointsData{
{
Meta: &metav1.ObjectMeta{
Name: testServiceName + "-1",
Namespace: testServiceNamespace,
},
Ports: []negtypes.PortData{
{
Name: testPortName,
Port: port80,
},
},
Addresses: []negtypes.AddressData{},
},
},
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: "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 {
t.Run(tc.desc, func(t *testing.T) {
if got := transactionSyncer.isEndpointCountZero(tc.endpointsData, tc.endpointPodMap); got != tc.expect {
t.Errorf("isEndpointCountZero() = %t, expected %t", got, tc.expect)
}
})
}
}

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 ad2eb11

Please sign in to comment.