Store runner check results in the database

[arbulu89]

Replace the checks result storage from ARA to the Postgres database. The used data is exactly the same, so the unique difference is the storage and the communication between runner/web.
It includes:

• Changes in the trento ansible callback to send the stored results to the trento web api
• A new POST endpoint (checks/{id}/results) which gets the checks result
• Changes in the checks service to store and get the checks results from the database
• Some updates in the checks models and entities. The entity stores the checks result as a raw jsonb data, for each individual group (clusters in our case). Later on the entity is parsed to a model to use it through the code.

Disclaimer: This PR doesn't remove the ARA code. This still exists (even though now it is not used). It will be removed in subsequent PRs

[nelsonkopliku]

LGTM

[nelsonkopliku]

I have some doubts on the enpoint /checks/{id}/results

I see a different variant that makes explicit the Cluster concept.
/clusters/:id/checks/results or
/clusters/:id/checks-results or anything along these lines.

This is mainly because as of now Checks don't exist on their own, they're always somehow bound to a cluster (are they? 😅 )

Yet, no strict need to change anything if you wish, it is sort of a reminder to make discussions about API design.

[arbulu89]

I knew this discussion would arise XD

The thing is that from the runner point of view, it doesn't really know that it is running cluster checks.
It only knows that there are some nodes and groups of nodes (that in our case, by now, are only clustered nodes) that have some assigned checks. With this data we create the inventory. And it will publish the results of this group.

If in the future we add other logical groups (like SAP system checks, individual nodes, SAP landscapes), this implementation would still have sense. If we use any kind of clusters endpoint, we would need to start diverging how we publish and store the results.

That's why I tried to make it generic.
In any case, it is true that by now we only have cluster checks, and at this moment naming the endpoint like clusters/:cluster_id/results would make sense.

[fabriziosestito]

@arbulu89 I think this can go as an iteration, but I would like to keep under the radar how the checks are persisted and exposed through the API. I find we have a bit of noise in terms of model structures, I understand that the structure of the project still doesn't permit much in terms of separation between "view models" and the rest but I think this will need a refactor soonish.
Also, as other mentioned we need to discuss and agree on how to group the checks API under the cluster resources (or maybe not).

I'd like to open a debt issue about the cleanup and refactor of the checks API, wdyt?

[fabriziosestito]

I wouldn't use the ById for a create method, wouldn't be possibile to add the id field to the model directly?

[arbulu89]

Yes, we can do so.
I will do the change

[arbulu89]

@fabriziosestito To understand it better. Do you only refer to change the CreateChecksResultById method or the whole endpoint checks/:id/results ?

[arbulu89]

Implemented in: b0efa61
50ecfc2

[arbulu89]

@arbulu89 I think this can go as an iteration, but I would like to keep under the radar how the checks are persisted and exposed through the API. I find we have a bit of noise in terms of model structures, I understand that the structure of the project still doesn't permit much in terms of separation between "view models" and the rest but I think this will need a refactor soonish. Also, as other mentioned we need to discuss and agree on how to group the checks API under the cluster resources (or maybe not).

I'd like to open a debt issue about the cleanup and refactor of the checks API, wdyt?

Ok. Let's get rid off ARA first (which can affect performance) and apply best practices later.
Here, I was not able to create Read models because the data doesn't go through the projector, and I didn't want to make it much more complex.
If we need to refacator it, we will.

About grouping under cluster resources, I'm partidary of not doing it (in fact, in the original commit I renamed some code to make it generic). I see the checks systems as a way of running any sort of checks, not only cluster related.

[fabriziosestito]

. I see the checks systems as a way of running any sort of checks, not only cluster related.

@arbulu89 Let's have a follow up about this, I totally agree on this but I also don't think that grouping API prevents the checks to be generic.