For the deployment we can reuse the same that's there already and add two containers, like we do in jobs-api for example, that also removes the need to have a service to communicate between them as they would be running in the same pod.

For the CI, we might want to use the same as builds-api for example, that does pre-commit + blubber test. That would mean that we have to add a test variant to the blubber file too ([[https://gitlab.wikimedia.org/repos/cloud/toolforge/builds-api/-/blob/main/.pipeline/blubber.yaml?ref_type=heads | like so]), we don't use in yet in any other that runs python tests (ex. jobs-api), so there's no base for it yet, but should be easy to setup and then we can consolidate all of them to do the same (pre-commit + blubber test, as opposed to only tox using a manually built image).

On the toolforge-deploy side, we will need to add any new values that might be needed (maybe not, or only for debugging reasons in the local.yaml).

In T362299#9705965, @dcaro wrote:

For the deployment we can reuse the same that's there already and add two containers

Do you mean add one container, so there would be two containers all together, nginx + oapi-server?

For the CI, we might want to use the same as builds-api for example, that does pre-commit + blubber test. That would mean that we have to add a test variant to the blubber file

Maybe we can start with just one variant for now, then add one for test once the basic plumbing is working?

In T362299#9706001, @Slst2020 wrote:

In T362299#9705965, @dcaro wrote:

For the deployment we can reuse the same that's there already and add two containers

Do you mean add one container, so there would be two containers all together, nginx + oapi-server?

Two containers in one pod yes.

For the CI, we might want to use the same as builds-api for example, that does pre-commit + blubber test. That would mean that we have to add a test variant to the blubber file

Maybe we can start with just one variant for now, then add one for test once the basic plumbing is working?

The issue is that we use pre-commit in some, and only tox in others, if you want pre-commit + tox we'll have to do a first there

In T362299#9706054, @dcaro wrote:

In T362299#9706001, @Slst2020 wrote:

In T362299#9705965, @dcaro wrote:

For the deployment we can reuse the same that's there already and add two containers

Do you mean add one container, so there would be two containers all together, nginx + oapi-server?

Two containers in one pod yes.

I think it would be better to have two separate pods for isolation purposes. This way the system can schedule/restart/healthcheck them separately.

A k8s Service is just a few firewalling/NAT rules, is not adding much overhead, no?

In T362299#9706084, @aborrero wrote:

In T362299#9706054, @dcaro wrote:

In T362299#9706001, @Slst2020 wrote:

In T362299#9705965, @dcaro wrote:

For the deployment we can reuse the same that's there already and add two containers

Do you mean add one container, so there would be two containers all together, nginx + oapi-server?

Two containers in one pod yes.

I think it would be better to have two separate pods for isolation purposes. This way the system can schedule/restart/healthcheck them separately.

A k8s Service is just a few firewalling/NAT rules, is not adding much overhead, no?

It also complicates debugging, as now your pods will potentially run in different hosts, in different containers, across the network etc.

That also means having to ssl-ify the traffic (tls traffic in-between the pods), so probably not the simplest, we can split them once we have an idea of the flakiness of each with real traffic.

For the CI, we might want to use the same as builds-api for example, that does pre-commit + blubber test. That would mean that we have to add a test variant to the blubber file too ([[https://gitlab.wikimedia.org/repos/cloud/toolforge/builds-api/-/blob/main/.pipeline/blubber.yaml?ref_type=heads | like so]), we don't use in yet in any other that runs python tests (ex. jobs-api), so there's no base for it yet, but should be easy to setup and then we can consolidate all of them to do the same (pre-commit + blubber test, as opposed to only tox using a manually built image).

Actually, we might want to use a pre-built image for pre-commit too, I'm playing with adding pre-caching for the pre-commit hooks we have to the image, that should reduce considerably the runtime and the external fetching of dependencies (that starts being an issue it seems for golanglintci, see https://gitlab.wikimedia.org/repos/cloud/toolforge/builds-api/-/jobs/239414).

In T362299#9706093, @dcaro wrote:

It also complicates debugging, as now your pods will potentially run in different hosts, in different containers, across the network etc.

That also means having to ssl-ify the traffic (tls traffic in-between the pods), so probably not the simplest, we can split them once we have an idea of the flakiness of each with real traffic.

I think I misread the earlier comment, I thought you were suggesting to run this new oapi-server container within the api-gateway deployment pod. Sorry for the noise.

sstefanova opened https://gitlab.wikimedia.org/repos/cloud/toolforge/api-gateway/-/merge_requests/17

Draft: WIP: add oapi-server to gateway

In T362299#9706660, @aborrero wrote:

I think I misread the earlier comment, I thought you were suggesting to run this new oapi-server container within the api-gateway deployment pod. Sorry for the noise.

So, apparently I did not misread it, because the merge request just contains this architecture.

I don't think we should go this route. I don't like this pattern. We should create a separate pod/deployment for this new service.

The API gateway pod is in the hot path for all user-facing things in Toolforge. It should be isolated, this way the system can schedule/restart/healthcheck it separately.

All the concerns mentioned here or in the merge requests (TLS, debugging, separate repo with boilerplate, etc) are not blockers, or complex to introduce for that matter. Is the same pattern we have for the rest of the system.

In T362299#9712838, @aborrero wrote:

In T362299#9706660, @aborrero wrote:

I think I misread the earlier comment, I thought you were suggesting to run this new oapi-server container within the api-gateway deployment pod. Sorry for the noise.

So, apparently I did not misread it, because the merge request just contains this architecture.

I don't think we should go this route. I don't like this pattern. We should create a separate pod/deployment for this new service.

The API gateway pod is in the hot path for all user-facing things in Toolforge. It should be isolated, this way the system can schedule/restart/healthcheck it separately.

All the concerns mentioned here or in the merge requests (TLS, debugging, separate repo with boilerplate, etc) are not blockers, or complex to introduce for that matter. Is the same pattern we have for the rest of the system.

I think you are confusing the api gateway (nginx + code) with nginx only, both things are "the api gateway", and yes, I agree it should be resilient and treated carefully. I do disagree that the way to make it so is to split it and make it distributed.

dcaro merged https://gitlab.wikimedia.org/repos/cloud/toolforge/api-gateway/-/merge_requests/17

[oapi-spec] add oapi-server to gateway

dcaro merged https://gitlab.wikimedia.org/repos/cloud/toolforge/toolforge-deploy/-/merge_requests/281

api-gateway: bump to 0.0.21-20240514125202-02c6ab3b