neoave / mrack

Multicloud use-case based multihost async provisioner for CIs and testing during development
Apache License 2.0
12 stars 14 forks source link

feat(OpenStack): Provide a way to disable network spreading #230

Closed Tiboris closed 1 year ago

Tiboris commented 1 year ago

The network spreading feature has aimed to utilize networks usage however for some particular products it might be not optimal to have resources on different networks. This patch is adding options to mrack.conf which can disable the feature and also set the acceptable threshold of network utilization for OpenStack.

network-spread = no/allow/force

Signed-off-by: Tibor Dudlák tdudlak@redhat.com

pvoborni commented 1 year ago

About the interface of this feature (the config options):

I like it being configurable, the idea with different max utilization is also good. And I also like the concept of a small job which goes into the "single-network mode".

But I'm thinking if the force-network-spread is the best name. I can imagine following scenarios:

  1. the job doesn't care about the the used way
  2. the job requires spreading
  3. the job requires one network - e.g. because it will fail otherwise

The usecase which we are solving here is #3 but the configuration options doesn't allow it. Spreading will be used if the job is not considered small - so it might be unreliable. I think that it might be better to fail the job at provisioning, saying there is no suitable network rather then randomly failing in the job as it got a bad network.

For that reason, I'd propose to change the force-network-spread into:

The logic can be:

WDYT?

Tiboris commented 1 year ago

This looks like minor change to existing code and sane workflow procedure for provisioning let me re-write a bit to support this.

pvoborni commented 1 year ago

Could you also add some tests which demonstrates that it works? It's easier to test in mock situation than in prod, where you cannot easily simulate the conditions.

Tiboris commented 1 year ago

@pvoborni I have added some tests

Tiboris commented 1 year ago

Thanks for the review @pvoborni !