search

thoth-station / storages

Storage and database adapters for project Thoth
https://thoth-station.github.io/
GNU General Public License v3.0
14 stars 16 forks source link

Missing solver-ubi-8-py38 in database Ecosystem Solver table #2196

Closed pacospace closed 3 years ago

pacospace commented 3 years ago

Describe the bug Missing solver-ubi-8-py38 in database but there is Missing solver-rhel-8-py38 in database. When querying for data in the database for specific solver solver-ubi-8-py38, none are identified. This might affect queries for adviser to look for data for rhel-8 mapped to ubi-8

To Reproduce Steps to reproduce the behavior:

  1. Have a look at the database

Expected behavior In Ecosystem solver table we should find solver-ubi-8-py38

Screenshots If applicable, add screenshots to help explain your problem.

Additional context Add any other context about the problem here.

pacospace commented 3 years ago

I think it does affect queries, as we map to ubi always if someone input rhel. cc @fridex

goern commented 3 years ago

/sig solvers

sesheta commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

/lifecycle stale

goern commented 3 years ago

/remove-lifecycle stale

goern commented 3 years ago

@pacospace is this still relevant, if not please close it

goern commented 3 years ago

is curl -X GET "https://management.moc.thoth-station.ninja/api/v1/solvers" -H "accept: application/json" a valid test for this? is this covered by an integration test?

pacospace commented 3 years ago

is curl -X GET "https://management.moc.thoth-station.ninja/api/v1/solvers" -H "accept: application/json" a valid test for this? is this covered by an integration test?

This is a possible inconsistency in data in the database, not from solver CM. If the map from rhel 8 to ubi 8 is used in every method, we can close this issue.

goern commented 3 years ago

sounds like a query we can run against the database, so that we check for consistency?

pacospace commented 3 years ago

sounds like a query we can run against the database, so that we check for consistency?

We have already that actually, number of solver in CM vs number of solver in database and we also planned an alarm there. So we can close this issue.

fridex commented 3 years ago

If I recall correctly, we had rhel in the database that should not happen as it should be mapped to ubi. If rhel entries will be kept in the database, they will not cause any issues on the user side (as there is no solver installed). The incosystency will arise in monitoring when metrics-exporter exposes info about them. This might be confusing as we do not have "rhel" solvers per say. So the solution to this issue:

@pacospace could you please verify we have only ubi solvers reported in deployments and no rhel solvers?

pacospace commented 3 years ago

If I recall correctly, we had rhel in the database that should not happen as it should be mapped to ubi. If rhel entries will be kept in the database, they will not cause any issues on the user side (as there is no solver installed). The incosystency will arise in monitoring when metrics-exporter exposes info about them. This might be confusing as we do not have "rhel" solvers per say. So the solution to this issue:

  • make sure we map solver names correctly - #2199
  • check if we have rhel entries in the database -- if so, we can remove these entries using the purge-job

@pacospace could you please verify we have only ubi solvers reported in deployments and no rhel solvers?

@fridex from Grafana in prod, it seems to show 8 different solver_name from EcosystemSolver table Screenshot from 2021-06-11 13-40-51

pacospace commented 3 years ago

If I recall correctly, we had rhel in the database that should not happen as it should be mapped to ubi. If rhel entries will be kept in the database, they will not cause any issues on the user side (as there is no solver installed). The incosystency will arise in monitoring when metrics-exporter exposes info about them. This might be confusing as we do not have "rhel" solvers per say. So the solution to this issue:

  • make sure we map solver names correctly - #2199
  • check if we have rhel entries in the database -- if so, we can remove these entries using the purge-job

@pacospace could you please verify we have only ubi solvers reported in deployments and no rhel solvers?

We actually map ubi to rhel in the database. So it will be synced as rhel: https://github.com/thoth-station/common/blob/826ad8f1db5234d2003f21d66b8ec0f6a29d2cea/thoth/common/helpers.py#L182

I will check syncing logic and verify the potential issue

pacospace commented 3 years ago

If I recall correctly, we had rhel in the database that should not happen as it should be mapped to ubi. If rhel entries will be kept in the database, they will not cause any issues on the user side (as there is no solver installed). The incosystency will arise in monitoring when metrics-exporter exposes info about them. This might be confusing as we do not have "rhel" solvers per say. So the solution to this issue:

  • make sure we map solver names correctly - #2199
  • check if we have rhel entries in the database -- if so, we can remove these entries using the purge-job

@pacospace could you please verify we have only ubi solvers reported in deployments and no rhel solvers?

We actually map ubi to rhel in the database. So it will be synced as rhel: https://github.com/thoth-station/common/blob/826ad8f1db5234d2003f21d66b8ec0f6a29d2cea/thoth/common/helpers.py#L182

I will check syncing logic and verify the potential issue

@fridex when you have a moment let's talk about this, because I have some questions on the mapping. Thanks!

fridex commented 3 years ago

@fridex from Grafana in prod, it seems to show 8 different solver_name from EcosystemSolver table

That means the prod deployment is not configured correctly. Number of solvers stated in the configmap should match number of entries in the EcosystemSolver table. Otherwise an alarm should be triggered (I think we had an issue for that).

The question now is what are these 8 entries? Are they consistent with respect to ubi/rhel naming?

  • EcosystemSolver table is synced only by solver!

That sounds correct. We will need to make sure it is properly removed on purge (I do not remember if I've done so).

  • We can delete with purge job all solvers in EcosystemSolver Table that have rhel and fedora basically.

What would solve this?

  • Adjust logic of solver sync, to use always document name and no mapping from ubi to rhel.

We introduced mapping as ubi and rhel are ABI compatible - see https://thoth-station.ninja/j/rhel_ubi.html

@fridex when you have a moment let's talk about this, because I have some questions on the mapping. Thanks!

Let's keep the discussion here so that steps and approaches are documented. Thanks!

pacospace commented 3 years ago

@fridex from Grafana in prod, it seems to show 8 different solver_name from EcosystemSolver table

That means the prod deployment is not configured correctly. Number of solvers stated in the configmap should match number of entries in the EcosystemSolver table. Otherwise an alarm should be triggered (I think we had an issue for that).

The question now is what are these 8 entries? Are they consistent with respect to ubi/rhel naming?

  • EcosystemSolver table is synced only by solver!

That sounds correct. We will need to make sure it is properly removed on purge (I do not remember if I've done so).

  • We can delete with purge job all solvers in EcosystemSolver Table that have rhel and fedora basically.

What would solve this?

  • Adjust logic of solver sync, to use always document name and no mapping from ubi to rhel.

We introduced mapping as ubi and rhel are ABI compatible - see https://thoth-station.ninja/j/rhel_ubi.html

@fridex when you have a moment let's talk about this, because I have some questions on the mapping. Thanks!

Let's keep the discussion here so that steps and approaches are documented. Thanks!

Sure!

So I ll address the issues one by one

fridex commented 3 years ago

We actually map ubi to rhel in the database. So it will be synced as rhel: https://github.com/thoth-station/common/blob/826ad8f1db5234d2003f21d66b8ec0f6a29d2cea/thoth/common/helpers.py#L182 I will check syncing logic and verify the potential issue

Based on this, we should not have os_name=ubi in the database. However, stage and prod keep following entries:

Screenshot_2021-06-11_16-47-12

I will delete them as they are not consistent with the mapping. Also the ecosystem solver table should not keep any ubi entries (as they should be mapped to rhel as well). Checking stage and prod deployment, I can see one UBI entry:

Screenshot_2021-06-11_16-50-19

This one should be also deleted. With this, number of solvers reported from prod deployment should decrease to 7. I will delete these entries from stage and prod database, ACK?

fridex commented 3 years ago

I will delete these entries from stage and prod database, ACK?

Deleted based on 👍🏻 on the comment. Could you please verify number of solvers reported in stage/prod? thanks!

pacospace commented 3 years ago

I will delete these entries from stage and prod database, ACK?

Deleted based on 👍🏻 on the comment. Could you please verify number of solvers reported in stage/prod? thanks!

stage Screenshot from 2021-06-11 18-22-58 EcosystemSolver Table solver-fedora-31-py37, solver-fedora-31-py38, solver-fedora-32-py36, solver-fedora-32-py37, solver-fedora-32-py38, solver-rhel-8-py36, solver-rhel-8-py38

prod Screenshot from 2021-06-11 13-40-51

fridex commented 3 years ago

Looks good, except for prod. We should purge old solvers and install new. These fedora ones are EOL anyway.

pacospace commented 3 years ago

Looks good, except for prod. We should purge old solvers and install new ones. These fedora ones are EOL anyway.

Should we just retrigger purge job in prod?

I closed https://github.com/thoth-station/storages/pull/2345 as we do not need a change in logic syncing to EcosystemSolver which correctly syncs rhel.

But we need to check also the sync is correctly done on SoftwareEnvironment Tables and other tables that consider os_name when syncing, the same mapping should be applied right?

fridex commented 3 years ago

Looks good, except for prod. We should purge old solvers and install new ones. These fedora ones are EOL anyway.

Should we just retrigger purge job in prod?

This is planned for the release - @harshad16 works on this.

But we need to check also the sync is correctly done on SoftwareEnvironment Tables and other tables that consider os_name when syncing, the same mapping should be applied right?

I think we can keep these entries for now as they do not create any preassure on the database.

/close

sesheta commented 3 years ago

@fridex: Closing this issue.

In response to [this](https://github.com/thoth-station/storages/issues/2196#issuecomment-861480058): >> > Looks good, except for prod. We should purge old solvers and install new ones. These fedora ones are EOL anyway. >> >> Should we just retrigger purge job in prod? > >This is planned for the release - @harshad16 works on this. > >> But we need to check also the sync is correctly done on SoftwareEnvironment Tables and other tables that consider os_name when syncing, the same mapping should be applied right? > >I think we can keep these entries for now as they do not create any preassure on the database. > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.