search

OCSInventory-NG / OCSInventory-Docker-Image

Docker image for OCSInventory Server
GNU General Public License v3.0
80 stars 96 forks source link

Fix Api Conf Bug #77

Closed wiltonsr closed 2 years ago

wiltonsr commented 2 years ago

There is no OCS_DB_LOCAL env in Api. Should be OCS_DB_NAME

Already create a PR in OCSInventory-NG/OCSInventory-Server#365

Must read before submitting

Please, take a look to our contributing guidelines before submitting your pull request. There's some simple rules that will help us to speed up the review process and avoid any misunderstanding

Contributors GuideLines

Status

READY/IN DEVELOPMENT/HOLD

Description

A few sentences describing the overall goals of the pull request's commits.

Related Issues

Put here all the related issues link

Todos

Test environment

If some tests has been already made, please give us your test environment' specs

General informations

Docker host's operating system : Mysql Server version :

Docker informations

Docker compose version : Docker version :

Deploy Notes

Notes regarding deployment the contained body of work. These should note any dependencies changes, logical changes, etc.

1.

Impacted Areas in Application

List general components of the application that this PR will affect:

*

gillesdubois commented 2 years ago

Hi @wiltonsr

I think there is a little misunderstanding here. Mojolicious and plack handler use their own env variable to manage the rest api.

In the rest api configuration, all the $ENV{SOMETHING} are not meant for docker but for the API itself. That why they don't match the docker ENV vars

Regards, Gilles.

wiltonsr commented 2 years ago

Hi @gillesdubois, thanks to reply.

In the rest api configuration, all the $ENV{SOMETHING} are not meant for docker but for the API itself. That why they don't match the docker ENV vars

I understand that, but the issue is that there isn't OCS_DB_LOCAL env in API, because of that I also opened OCSInventory-NG/Ocsinventory-Server#365, but the docker image doesn't work correctly when your database name is different from ocsweb, as could be seen here.

I agree that the main place to fix the problem is in the API repository, but as the docker repository has a copy of the config file, I decided to open the fix for both

wiltonsr commented 2 years ago

Hi @gillesdubois,

I made some more tests and find what was the problem.

If we run the docker-compose example file for v2.9, even changing the database name it will run correctly. This ocurrs because the compose example run all servers (api, reports and inventory) on same container. And server conf file is setting the OCS_DB_LOCAL value.

But, on my production environment I run a container only for API. And in this cenario, dont settings OCS_DB_LOCAL will cause a DBI Connection error.

On OCSInventory-Server both variables, OCS_DB_LOCAL and OCS_DB_NAME are tested to make connection. So the same should be made here or only changes the variable name like the PR does.

A complete refactoring to standardize the DB connection variable names in the 3 services and in Docker would also be a good option to prevent this kind of misunderstanding.