feat: allow configuring in which network instances are created

[4censord]

For our usecase, we needed the runner instances to be created in a private network.

If no network is specified, will use the public by default

[href]

Hi there, thank you for your contribution. We have this on our roadmap as well, but might do a slightly different implementation: We will require multiple networks to be specifyable (so the runner has access to multiple networks), with one network being the one that is returned to the GitLab runner manager.

Or at least, that's how I envision it currently. I haven't done all the leg-work yet. Work on this should commence within the next two weeks, as we have other customers interested in this.

So I hope you don't mind I hold off on this for a bit, until I know for certain what we need the implementation to look like.

[4censord]

I don't mind; This is just how I made it work because we just need one network.
As I'm working on this already, I'll have a look if I can just add that. The thing that might be problematic is selecting what network to return to the runner manager.

FYI: I have another PR pending for creating instances in parallel, as we are using the plugin with the instance executor type, and creating machines sequentially significantly slows down our pipelines. (also, docs for howto do that incomming)

[4censord]

Actually, it was rather easy to make it accept multiple networks

[4censord]

Its currently implemented like this:

• You need to specify a primary network. This is the one the runner controller uses for comunication. It will default to the public network if nothing is set
• You can specify 0..n additional networks that are added to the instances.

[href]

That looks reasonable. Let me have a closer look at this next week and I'll get back to you.

Parallel creation also sounds interesting, looking forward to getting into that once you have a PR 🙂

[href]

I took a look at this, and I think this is a pretty good solution already, though I have some feedback I'd like to see implemented:

Combine Network Config

I think combining the network config into a single item as follows, would be preferrable:

networks = [
    "public"
]

The first network would be the one meant for the GitLab controller, the others would be extra networks. I think this makes it slightly cleaner conceptually, and ties in into my next point.

Use Interfaces Objects

In the API, we have a number of ways to define interfaces for servers:
https://www.cloudscale.ch/en/api/v1#interfaces-attribute-specification

For example, we might start a server with the following interfaces config:

[
    {"network": "UUID3", "addresses": [{"subnet": "UUID4"}]}
]

I think in many cases, passing the network UUID alone is sufficient, but we might end up with customers that want something more specific down the line, and then we would have to deprecate the old config, and move to a new one.

So ideally, the config just resolves into the Go SDKs interface objects:
https://github.com/cloudscale-ch/cloudscale-go-sdk/blob/1baae3ab99a83a25221c77a8c6f4c69838979312/servers.go#L67

This way, we would be able to support all kinds of configurations down the line. Consequently, we would end up with something like this in the config (I think interfaces would then be the right way to call this):

interfaces = [{ network = "UUID3", addresses = [{ subnet = "UUID4" }] }]

This would be the default:

interfaces = [{ network = "public" }]

Support Legacy Config

We want to support updating the fleeting plugin in this manner:

gitlab-runner fleeting install <image> --upgrade

For this to work, the existing config should still work. Currently this fails, because the new fleeting image does not have a default network config. That is to say: Writing the default into the config.toml.template is not enough going forward. It is possible for this value to not exist at all, and the plugin code should deal with that gracefully.

---

Since this adds some complexity that might not be relevant for your use-case, let me know if you'd like me to take on some, if not all of these changes. I'm happy to review any further changes as well though. So let me know what you think, and how you would like to proceed.

[4censord]

So, i've implemented the interfaces config.
It uses cloudscale.InterfaceRequest instead of cloudscale.Interface directly.

This currently sits on top of #2, but if needed i can rebase it to be standalone as well.

What i'm not quite sure about is when one would want to specify an address. Wouldn't that limit your scale to 1 instance, as everything else gives ip collisions?

Also, the actual implementation was surprisingly simple

[href]

Thanks, I think this is about good to go. Since I have some things to remark on the other PR, I would indeed appreciate it if you could separate them again (then we can go ahead and merge this one first).

One comment, and this ties into your observation:

What i'm not quite sure about is when one would want to specify an address. Wouldn't that limit your scale to 1 instance, as everything else gives ip collisions?

I could see some use-case where this might be useful (say someone wants to scale from 0 to one single beefy runner at certain times, and wants to set the IP for that), but I think it's far fetched. And so I would actually remove the example you give about setting the IP address.

My main point about this implementation is to be forwards-compatible. For example, currently networks have but one subnet. But that may change in the future, and we could support this in Fleeting without a breaking change.

We might also support running without DHCP (which happens if you set addresses to []), though that would require us to somehow get the IP address we would no longer be able to read from the API into the GitLab Runner. I'm not sure how we would do that yet, but it would be possible and once the use-case arises, we can implement it without breaking change as well.

[4censord]

Ok, i've separated them again, and removed the example with the explicitly specified address

[href]

I'm ready to merge this once small fix above is implemented.

[href]

Note: I tried to enable github workflow builds for all pull requests targeting main. You might have to rebase for that to work.

[href]

Since the tests are successful with the little fix mentioned above, I'll go ahead and merge this, then fix it inmain.

[href]

FYI, there is now a beta release that we'll be testing, in case if you want to run this on your infrastructure:
quay.io/cloudscalech/fleeting-plugin-cloudscale-beta:latest

And thanks again for your contributions! 🙂

[4censord]

Nice, i'll be testing the beta version somewhen next week!