uselagoon / lagoon

Lagoon, the developer-focused application delivery platform
https://docs.lagoon.sh/
Apache License 2.0
552 stars 149 forks source link

Improvement: add additional information to environment services #3641

Closed shreddedbacon closed 5 months ago

shreddedbacon commented 6 months ago

General Checklist

Database Migrations

This just extends the existing environment_services by adding the service type and containers associated with the service. This will allow better visibility of what services an environment has deployed in the API.

Some benefits of this will also make the implementation of workload resources easier, because now the API will be able to determine what service types an environment has.

Additionally, this information being available can allow for the UI to be able to generate command examples for being able to SSH to services and containers.

This will require changes in the remote-controller so that it sends the new information, this work has not been done yet, but as the API has been defined, and it contains backward compatible implementation (the setEnvironmentServices mutation still works for now, and the support in the actions-handler remains). The remote-controller is currently staging a bump to the CRD version so that we can introduce the schema changes to the CRD and not have to rely on users having to remember to update the CRDs after installation, as the new CRD version will be installed.

It would be great to get this merged so that at least the support is there for when the remote-controller gets the schema changes. This means that there will be a period where old and new controllers will be able to send their messages to core to update environment services. Just older remotes will not send the right payload to update the services with the new information.

As there are no other services in the API or Lagoon generally that could, or do, use this additional information, the delay in the remote-controller sending the information should not be seen as a blocker to this merging. It may just mean that the final removal of the deprecated mutations may be delayed longer.

Requires https://github.com/uselagoon/machinery/pull/39

tobybellwood commented 5 months ago

Looking at this, i've got questions about how the name and type are (or should be) populated.

A new core (in testing) - without a remote sending any updated information, appears to be putting the type into the name column

Screenshot from 2024-02-09 18-10-22

Is it the addOrUpdateEnvironmentService mutation on environment 3 in the screenshot that's setting the name and type?

I can't fully understand how the name currently gets set - it appears to be a combination of type and service name, but maybe this is our chance to straighten it all up? image

shreddedbacon commented 5 months ago

Looking at this, i've got questions about how the name and type are (or should be) populated.

A new core (in testing) - without a remote sending any updated information, appears to be putting the type into the name column

Is it the addOrUpdateEnvironmentService mutation on environment 3 in the screenshot that's setting the name and type?

I can't fully understand how the name currently gets set - it appears to be a combination of type and service name, but maybe this is our chance to straighten it all up?

Only the name will be populated at the moment as there is no version of the remote-controller published that contains the code to set the new fields. The current controller sends this information back currently for the name.

The proposed changes to remote are still to be decided, but a rough first run is here

shreddedbacon commented 5 months ago

So after digging, what you're seeing is just something that has always been a problem, the remote-controller uses the "container name" which is incorrectly consumed (and has been since forever) the current controller sends the same container name https://github.com/uselagoon/remote-controller/blob/v0.15.4/controllers/v1beta1/podmonitor_buildhandlers.go#L299-L314

So the opensearch ones uses the chart name, which seems to be something we haven't really managed to get right in the templates for a lot of the helm charts we currently have, mixed bag for different services https://github.com/uselagoon/build-deploy-tool/blob/main/legacy/helmcharts/opensearch/templates/deployment.yaml#L69 https://github.com/uselagoon/build-deploy-tool/blob/main/legacy/helmcharts/opensearch/Chart.yaml#L2