search

jenkinsci / hetzner-cloud-plugin

Hetzner cloud integration for Jenkins
https://plugins.jenkins.io/hetzner-cloud/
Apache License 2.0
24 stars 8 forks source link

Use first server template if requested node label is empty/null #12

Closed jwwk closed 2 years ago

jwwk commented 2 years ago

Feature Request

I propose matching the first server template when there isn't an explicitly requested node label. This is more in line with other implementations, such as the Docker Plugin or the EC2 Plugin. Currently, an empty list is returned if the label is empty.

Also, I had difficulties understanding the context of "Label" in this form:

I needed to look up the code in this repo to find out that this is referring to hudson.model.Label and not to the labels at Hetzner Cloud. Maybe it would be possible to add another field for assigning Hetzner Cloud labels.

Besides these initial difficulties, the plugin works fine 🎉 Thanks for your effort!

rkosegi commented 2 years ago

First of all, thank you for bug report.

I propose matching the first server template when there isn't an explicitly requested node label. This is more in line with other implementations, such as the Docker Plugin or the EC2 Plugin. Currently, an empty list is returned if the label is empty.

This is actually result of decision to treat every new agent node to be used in Exclusive mode, which essentially means that it can execute only job with exact same label.

We can add configuration option so it will have similar behavior as you are proposing.

Also, I had difficulties understanding the context of "Label" in this form

That's something that is used commonly used in different plugins as well, for example in Docker Plugin that you mentioned. I will make wording more clear in help.

Maybe it would be possible to add another field for assigning Hetzner Cloud labels.

I'm not strongly against placing arbitrary labels on Hetzner nodes, but I don't see what benefit it would have. Also plugin itself places some specific labels used for identification. If user interferes with these labels, it could disrupt ability for plugin to clean node instances.

jwwk commented 2 years ago

You are right, adding arbitrary labels is not necessary. The existing labels should suffice to handle most use cases. Adding this behavior to the docs would be great, though 👍

rkosegi commented 2 years ago

@jwwk you should now be able to configure mode in template setting as Use this node as much as possible. If you don't provide any labels, provisioned node should be able to handle any job. Please let us know in case of issues.