allocator: Retry failed allocations immediately upon a deallocation

[aaronlehmann]

We retry failed allocations every 5 minutes. If something else gets
deallocated, we should trigger the retry immediately in case the
allocations were failing due to IP exhaustion, and the deallocation
freed up an IP.

cc @abhinandanpb

[abhi]

will this still need a cluster event ?

[aaronlehmann]

Yes

[abhi]

isnt that the main problem ? If there is no cluster event we will not allocate even if somethingWasDeallocated is set ?

[aaronlehmann]

somethingWasDeallocated is only set during an event. These events are always followed by EventCommit.

[abhi]

sg . Thanks

[abhi]

LGTM thanks for taking care of this.
Related moby/moby#33619

[aluzzardi]

Is there even a point to try to allocate if something was not de-allocated?

[aluzzardi]

I don't have a quick solution in mind, but is there a way to avoid spreading somethingWasDeallocated around? It's very likely we'll end up forgetting to update one one way or the other, and the bug will be extremely subtle since eventually it's going to work (5 minutes delay at most), we'll just experience rare allocation slowness on certain conditions (IP exhaustion)

[aaronlehmann]

Is there even a point to try to allocate if something was not de-allocated?

Being out of IPs is only one possible error. I don't have a good idea of what the others are and what situations would make the tasks allocatable again, but one thing I can think of is a missing ingress network, where creating the network again would allow forward progress.

[aaronlehmann]

I don't have a quick solution in mind, but is there a way to avoid spreading somethingWasDeallocated around?

The only thing I can think of is to write a deallocation function that takes an interface{} and does a type switch to figure out how to deallocate the object, but I'm not sure it's worth it.

[aluzzardi]

Maybe something simpler?

We're setting somethingWasDeallocated 7 times around, it's going to be easy to forget.

How about:

 			a.procUnallocatedNetworks(ctx)
  			a.procUnallocatedServices(ctx)		  		
  			a.procTasksNetwork(ctx, true)

Can we have that at some sort of tick (like we do in the scheduler/orchestrator), which we call every N minutes or every time we detect a change in nodes/services/tasks?

That way we'd just call tick in those places rather than keeping track of state changes around

[aaronlehmann]

That way we'd just call tick in those places rather than keeping track of state changes around

Those places should not call tick before the commit event, otherwise a batch of deallocations would trigger many ticks. This is the motivation for somethingWasDeallocated.

Also, I don't understand why calling tick is harder to forget than setting the flag.

[aluzzardi]

Yeah you're right. LGTM!