Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cloud_name should be configurable in provider config #1

Open
maigl opened this issue Apr 12, 2023 · 4 comments
Open

cloud_name should be configurable in provider config #1

maigl opened this issue Apr 12, 2023 · 4 comments

Comments

@maigl
Copy link
Contributor

maigl commented Apr 12, 2023

Hi,

finally I had some time to try out the new provider..

One small finding is that I would love to be able to configure the cloud name in the provider config.
We do configure multiple clouds (and even multiple regions) in our clouds.yaml. You explicitly
state that one cannot overwrite that:

# This option can NOT be overwritten using extra_specs.
cloud = "my_cloud"

.. but it would be very convenient for us to be able to change this via the extra specs.
Ideally, we could overwrite cloud and region.

Michael Kuhnt [email protected] Mercedes-Benz Tech Innovation GmbH (ProviderInformation)

@gabriel-samfira
Copy link
Member

Hi @maigl !

The reason this was made a static config value is because (accidentally) changing it will cause garm to loose track of existing runners. When it tries to consolidate, it will notice they are gone from the provider, but are somehow still active in github.

It may cause confusion during debug and lingering resources. If the runner gets consumed by gthub, garm will think it's already gone and will stay on the provider in the previously configured cloud and region.

This option can be made configurable, but that flexibility comes with a huge warning.

@maigl
Copy link
Contributor Author

maigl commented Apr 12, 2023

okay .. understood. My alternative is to have different but very similar provider configs for each cloud/region.

But if I change the value in this config the effect would probably be the same, right?

@gabriel-samfira
Copy link
Member

Yes. It's just a bit more annoying to do, and less of a chance to change it by mistake 😄. This was a "best judgement" call, but I am not rigid on the matter. If it helps UX, I am all for making this configurable, given that we add warnings making it clear what would happen if this changed.

@gabriel-samfira
Copy link
Member

Will add that customization as soon as time permits. Should be relatively straight forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants