eni: Fix node manager test

[errordeveloper]

The test was failing due resulting values being greater then expected,
however it is evident that minAllocate is a minimum value and should
be treated as such.

Fixed #11560.

[maintainer-s-little-helper]

Please set the appropriate release note label.

[maintainer-s-little-helper]

Please set the appropriate release note label.

[errordeveloper]

Having fixed the first issues, I am still seeing it fail on something else:

FAIL: node_manager_test.go:526: ENISuite.TestNodeManagerExceedENICapacity

node_manager_test.go:556:
    c.Assert(node.Stats().AvailableIPs, check.Equals, 45)
... obtained int = 20
... expected int = 45

But I cannot really tell what's going on, maybe the tests ends up looking at the wrong actual node or something like that? Or perhaps another assertion is missing at an earlier stage? I know virtually nothing about this code, but I'd say I do understand the domain of ENI IPAM.

[coveralls]

Coverage increased (+0.004%) to 36.839% when pulling 4777edd on pr/errordeveloper/fix-11560 into 05daeef on master.

[errordeveloper]

On another several attempts, different values, this time higher then expected:

FAIL: node_manager_test.go:526: ENISuite.TestNodeManagerExceedENICapacity

node_manager_test.go:556:
    c.Assert(node.Stats().AvailableIPs, check.Equals, 45)
... obtained int = 54
... expected int = 45

It looks like this is not random.

Looks like 20 relates to this:

newCiliumNode("node2", "i-testNodeManagerExceedENICapacity-1", "m4.xlarge", "us-west-1", "vpc-1", 1, 8, 20, 0)

However, 54 doesn't seem to relate to anything and that's what I'm seeing most often.

[christarazi]

I think we should remove that this PR fixes the issue because the underlying cause doesn't seem to be fixed. From my previous investigation, I noticed that when a failure occurs, there's always a message like:

Instance not found! ...

which is coming from here:

cilium/pkg/aws/eni/node.go

Line 487 in 5e5c626

scopedLog.Warning("Instance not found! Please delete corresponding ciliumnode if instance has already been deleted.")

and also from here:

cilium/pkg/ipam/node.go

Line 363 in 5e5c626

scopedLog.Warning("Instance not found! Please delete corresponding ciliumnode if instance has already been deleted.")

I think the cause of these failures is that the some Stats fields are zeroed out:

		scopedLog.Warning("Instance not found! Please delete corresponding ciliumnode if instance has already been deleted.")
		// Avoid any further action
		n.stats.NeededIPs = 0
		n.stats.ExcessIPs = 0

when the condition that causes the log message to occur is hit. I can't seem to put a finger on why it happens, as most runs it works just fine. I think there's some more raciness going on with the async triggers / reconciliation code.

[christarazi]

On second thought, if the new failure you mentioned @errordeveloper is occurring less often, then we could merge this fix and continue investigating.

[errordeveloper]

@christarazi yes, I thin there is some sort of raciness for sure!

[christarazi]

Great, merging and will keep a note on the original issue that it may come back.