Add a node parameter to the FIO benchmark

[mmotejlek]

I've added a parameter to the FIO benchmark which allows to specify a node on which to deploy the pod which runs the benchmark. Internally it sets the Node field in the Pod definition. The notation is --node or -N and the default value is empty, meaning no node is specified.

Examples of when setting a specific node matters:

• Unequal connectivity bandwidth to nodes in the cluster.
• Benchmarking on nodes which do not contain privisioned storage.
• Keeping the same conditions when repeating benchmarks or trying different FIO configurations.

[ihcsim]

Thanks for the PR - this looks useful 👍. Any reasons why you pick nodeName over nodeSelector? Selector seems to offer more flexibility, and also more consistent with most K8s CLI tooling.

[mmotejlek]

Thanks for the PR - this looks useful 👍. Any reasons why you pick nodeName over nodeSelector? Selector seems to offer more flexibility, and also more consistent with most K8s CLI tooling.

Good call, I originally wanted it in order to be able to repeat benchmarks on the same node and select any node without having to set up labels, but I found out that the "kubernetes.io/hostname" label would allow to do the same using hostnames. I'll change it to nodeSelector and notify this PR.

I've had a look at the current flags and conventions and it seems that it will be --nodeselector and either -N or -S, is that ok?

[ihcsim]

How about just follow the kubectl style?

$ kubectl get -h | grep -w "\-l"
  -l, --selector='': Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)

I think just supporting = is fine.

[ihcsim]

Re-reading our earlier conversations, I think --node-selector="key=value" makes sense. I don't know if we really need a short-hand. If we do, -N is fine. Thanks for working on this.

[mmotejlek]

I've pushed an implementation of a flag which sets the nodeSelector field in a Pod object. The existence of a node that satisfies the selector is validated before anything is created in Kubernetes.

--nodeselector -N

The name "nodeselector" was chosen without a dash for most consistency with existing flag names.

Usage: --nodeselector key1=value[,key2=value] [-N key3=value]

The used syntax is a subset of kubectl's --selector flag's syntax EXCEPT that it allows to write -N key1=value -N key2=value, while --selector only allows the -l key1=value,key2=value form. It should not be a problem in case of a future extension, it would just need to be taken into account; alternatively the current syntax would have to be restricted.

I'm not sure to which extend kubestr cares about backwards compatibility though; I've realized that the current versions are 0.x.

Tolerations need to be solved in order to be able to create pods anywhere but I'll create a separate issue discussing that.

Edit: Tolerations issue is #111

[ihcsim]

@mmotejlek thanks for the PR. Sorry for the late response - I just got back from vacation. I will review this shortly.

[ihcsim]

@mmotejlek Code change LGTM and works great. Also, appreciate the tests! Can you resolve the conflicts?

[mmotejlek]

@ihcsim Conflicts resolved. Thanks for all the feedback on this.

[ihcsim]

LGTM 👍. Thanks!

[ihcsim]

@pavannd1 @bathina2 PTAL and merge, when ready. Thanks.

[pavannd1]

[nit] this should probably move to the list below with other third-party imports

Not blocking the PR, we can fix it as a follow-up

[pavannd1]

Thanks for the PR @mmotejlek 🙏🏼