Changing nodeSelector key to generic keyname

[imrajdas]

• To support the issues Allow running prombench locally #333 and Allow running prombench on EKS #341.
• Changing nodeSelector key to a generic keyname (node-type) and adding that label to cluster.yaml and nodepools.yaml so that multiple providers can use the nodes without having to change labels.
• Renaming 1a_gcloud_rolebinding.yaml to 1a_rolebinding.yaml

Logs- logs.txt

[imrajdas]

cc @geekodour

[geekodour]

Thanks for the PR :) Looks good.

Was it necessary for the rename of /manifests/cluster-infra/1a_gcloud_rolebinding.yaml for updating the node labels? If not, then it'll be good to add those changes as another PR as that file has other mentions of GKE aswell which need changes.

[imrajdas]

Thanks for the PR :) Looks good.

Was it necessary for the rename of /manifests/cluster-infra/1a_gcloud_rolebinding.yaml for updating the node labels? If not, then it'll be good to add those changes as another PR as that file has other mentions of GKE aswell which need changes.

Not necessary @geekodour, I reverted it to 1a_gcloud_rolebinding.yaml. Will raise another PR for this

[geekodour]

Okay, thanks. :) Could you update the node labels for funcbench aswell in this PR? You'd probably need to rebase with master.

[geekodour]

LGTM, tested it aswell. @krasi-georgiev 's review would be nice :)

But this will break deployment/updating of manifest files like

test-infra/prombench/manifests/cluster-infra/7b_commentmonitor_deployment.yaml

Line 52 in 0180701

cloud.google.com/gke-nodepool: main-node

because the current node(deployed test-infra) does not have the label node-type set. so can probably just set it for the main node and the rest of the nodepools will be created with the new labels for each benchmark anyway.

[krasi-georgiev]

why node-type and not nodeName or node-name?

[imrajdas]

why node-type and not nodeName or node-name?

Hi @krasi-georgiev , Thanks for the review. I changed the node label from node-type to node-name. Please take a look

[geekodour]

nodeName can be confusing as k8s already have nodeName and can be confused without proper context. I think node-type is a better name for this as it says which kind of node in our test-infra we want the pods to go to.

But node-type is a little confusing with things like node-type : funcbench-{{ .PR_NUMBER }}, which directly does not signify a type. Unlike node-type: main-node.

node-name still seems like a good option though but again is more similar to nodeName.

Will merge after @krasi-georgiev 's final review on the changes added.

[krasi-georgiev]

LGTM
this is easy to change again if needed

[geekodour]

Thanks! 🥳