Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docker PGadmin only loads servers.json on first startup, not after. #8071

Open
deefdragon opened this issue Oct 26, 2024 · 5 comments
Open
Labels

Comments

@deefdragon
Copy link

Describe the bug

Loading in of the servers.json file only occurs once when using the docker file, even if changed.

To Reproduce

  1. Install pgadmin with servers.json file in place.
  2. start docker container
  3. (servers exist)
  4. change servers.json
  5. restart pgadmin
  6. servers unchainged.

alternatively

  1. Install pgadmin with no servers.json file
  2. start docker container
  3. no servers in list
  4. add servers.json file
  5. restart pgadmin
  6. still no servers

Expected behavior

If the servers.json file has changed, load in the new servers, or provide some other method of forcing the servers to be loaded in on restart. (this would likely require gating behind a new env variable to be set as #7811 implies that, atleast to some, re-importing every boot is undesirable)

Desktop (please complete the following information):

  • OS: k8s docker, via helm
  • Version: 8.12
  • Mode: server
  • Browser (if running in server mode): firefox
  • Package type: docker helm

Additional context

The specific root cause is that, due to the call to load in the servers here being inside an if block here (that only runs if the pgadmin.db file has not been created yet), changing/adding the server.json file after the first boot will not load in the servers as they have been updated.

I believe that this is the root cause of a number of un-reproducable bugs reported in both the pgadmin repository, and the helm chart repository.

@deefdragon deefdragon added the Bug label Oct 26, 2024
@moelaj
Copy link

moelaj commented Oct 27, 2024

Having the same issues with docker dpage/pgadmin4:2024-10-19-2 on SAP BTP CLoud Foundry.
After application restart, I loose servers and users.

@deefdragon
Copy link
Author

deefdragon commented Oct 27, 2024

@moelaj that sounds like you don't have persistence set up. this is.a different issue. the servers.Json file Allows pre-populating the servers a user sees on login, and is useful for ie setups deployed with infrastructure as code.

The issue I'm describing here is that updating the list of servers in the servers.Json file (ie If your infrastructure as code updates what servers should be vissable ) does not update the servers actually being shared due to when loading the servers.Json file happens.

@dpage
Copy link
Contributor

dpage commented Oct 28, 2024

This is the intended and documented behaviour. From: https://www.pgadmin.org/docs/pgadmin4/latest/container_deployment.html#mapped-files-and-directories

/pgadmin4/servers.json

If this file is mapped, server definitions found in it will be loaded at launch time. This allows connection information to be pre-loaded into the instance of pgAdmin in the container. Note that server definitions are only loaded on first launch, i.e. when the configuration database is created, and not on subsequent launches using the same configuration database.

This primarily stems from the original thinking that pgAdmin containers would not be immutable as their configuration would be stored within them, and thus simply re-launching would cause the configuration database to be recreated and loaded with the latest version of the servers.json file.

That is of course at odd with more recent deployments where people often map /var/lib/pgadmin to a persistent location, thus effectively making the container itself immutable (which is generally considered the correct way to deploy nowadays).

I don't see any particular reason why we couldn't allow additional servers to be loaded, should someone take an interest in writing the code to do so. That is probably a touch more complicated that it might seem at first glance, because it would be necessary to avoid re-loading servers that have previously been loaded, and allow for any that users have manually loaded through the UI.

@deefdragon
Copy link
Author

I was unaware of that particular piece of documentation. I was referencing the import/export page, which describes the servers.Json format, and that has no mention of it loading only once. at minimum that piece of docs shoup probably be duplicated over there.

I have a few ideas on tackling loading in the servers every boot. I'll take a look at them soon.

@deefdragon
Copy link
Author

Having dug into the code a little bit, I have a few solutions and would like some thoughts on them.

  1. Treat servers akin to groups, and assume that a server with the same displayed name & that is part of a given group is the one the user intended to update.

    This is the easiest solution as it should only require about a dozen lines of code, and is relatively low impact. Groups kindof does this already here where it loads in all groups for the user and, if the name matches, uses that group's ID as the server group.

    Unfortunately, it has the downside that servers can have the same name within a group, meaning you can end up in a situation where the update is ambiguous as to which server is updated. It also means that you can't move a server between groups by changing its group name. That would just create a new server in the other group.

  2. Store the key that is used to define the server in the servers.json file as a column in the server table.

    This method guarantees that the server the user wanted to update is properly updated (unless they provide invalid json that has 2 keys that are the same).

    The downside is that this requires a database format update, and we need to give a lot of consideration to the exact implementation. Specifically, if we have a user provided key, the method used to store that key needs to be considered

    1. If we store the raw key, we may run into size limitations. Server names are currently limited to 128 characters, so that should be fine for most cases, but important to consider.

    2. If we want to guarantee any potential key used is valid, we could hash the provided key, resulting in a known format for the servers.json file. The downside of this is that exporting the servers.json file could only ever produce a file with new keys for the servers, and you could not import that exported file to update the existing servers.

  3. Allow the user to define the server ID in the servers.json file.

    This method would basically guarintee that any server the user intends to update is exactly the one that gets updated, even allowing moving groups etc. as they see fit.

    Downside is that this would require more work, and it would mean the export function would also have to be changed to export the IDs so that an import of an exported servers.json would update the servers as expected instead of adding new ones. More importantly however, user-set IDs may result in odd interactions/errors with the database when adding new servers through the UI.

    EX: You are working from a fresh install and import the servers.json, importing servers with IDs 1-10. If the server ID colunm is generated auto-incrementing, and starts at 1, you may encounter errors attempting to add new servers through the UI 10 times ntil the ID counter finally hits 11. I'm unsure on the exact eccentricities of pgadmin's chosen database however, and if this would actually be the case.

I lean towards 1 as its the quickest to implement and will be fine for I suspect 99% of cases, and basically all normal use-cases. I can't think of any reason someone would create a servers.json file and have all their servers be the same group & same name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

No branches or pull requests

3 participants