PLZ: add tolerations

[aaron-lunsford-even]

trafficstars

Fixes #427

I didn't see a contribution guideline and I noticed that PLZ tests were being added to the testrun package in #426, so I wasn't sure about adding them to this PR.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[yorugac]

@aaron-lunsford-even Indeed, contribution guideline is still my WIP task; but more than that, it's the first time we get a PR for PLZ which is amazing! But we need to figure out the logistics. PLZ is very closely tied to Grafana Cloud, so new features almost always would require collaboration with Grafana Cloud team. For instance, here, tolerations will need to be passed as part of API, like is done in the PR you linked: https://github.com/grafana/k6-operator/pull/426/files#diff-e01d6388a2fbfc9c44d3a92e7db08cc546c954e1c3e4cf80e4bc8f7b51700c57 But that API must be modified first on the Cloud side.

I'll raise this issue and PR in internal planning, to coordinate how we can proceed here. From purely technical perspective, this PR has lots of similarities to adding custom image as is done in PR #426, so I think after #426 is merged, we should be able to work on this PR in similar fashion and "streamline" adding the changes. But still, it'll take some time, more changes made, some back and forth and, perhaps most importantly, e2e testing can be done only internally. I'll come back to you once I have updates from the internal planning. Meanwhile, let me know how the above looks like to you :slightly_smiling_face:

[aaron-lunsford-even]

@yorugac - Understood! Thanks for the feedback and all of the context around the PLZ flow. There's a lot more going on than I realized! I'll leave the PR as-is and keep an eye out for future updates. If there's anything you'd like me to do from my end, please let me know and I'll be happy to help out. Thanks again!

[aaron-lunsford-even]

Hi @aaron-lunsford-even! Sorry for the delay. We had an internal discussion and it should be possible to go ahead with your PR as is 🎉 In case of tolerations, there is no real need to store them in our cloud: we can store it as part of PLZ resource and that's it. So basically as you did.

I'll have just a couple of small change requests here. Firstly, please make a rebase and sign a CLA: I can't merge anything otherwise. And secondly, if you run make generate, there is .deepcopy.go file that should be auto-updated - please add it.

That's the main thing. For testing, this is a straight-forward case so expanding unit test in pkg/testrun/plz_test.go would be ideal 🙂

Hey @yorugac, thanks for the update. I'll get those changes in and sign the CLA once I get the approval from my company. Should be soon 🤞

[yorugac]

Hi @aaron-lunsford-even, has it been possible to get the approval for CLA? :slightly_smiling_face:

[vinzee]

Is there any update on this?

[vinzee]

Seems the author did not respond further. I can help own and complete this change if that helps.

[yorugac]

@vinzee, are you using PLZ and need tolerations there? If so, could you please describe your use case a bit more?

[vinzee]

Yes, we do. We have multiple NPs in our cluster and we want to run all PLZs/loads-tests on a dedicated NP. We do not want any other workload to be scheduled on this nodepool. For this we can put a taint on this NP, but also need to be able to put tolerations on the PLZs.

This is standard practice in Kubernetes. Hope this clarifies. Happy to share more info.

[yorugac]

@vinzee, got it; thanks for clarification! TBH, from PLZ-centric feedback we received so far, most folk were happy enough with node selectors. But tolerations did come up once or twice too. I guess it depends on how one organizes their cluster and configuration.

I can help own and complete this change if that helps.

AFAIR, this PR was very close to be approved, it just didn't get the signed CLA and that's a mandatory requirement for any Grafana repo. So if you'd like to pick it up, you're very welcome to open another PR :slightly_smiling_face: I'll be closing this one shortly.