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

Fixes for jdbcstore, jdbcconfig & jndi #699

Merged
merged 6 commits into from
Nov 8, 2024
Merged

Conversation

tldev-de
Copy link
Contributor

@tldev-de tldev-de commented Nov 5, 2024

Hi there,

this is my first contribution to this project, so please give me feedback if there is anything I can do better.

This commit fixes two problems, I experienced while using it for a larger deployment:

  1. jdbcstore does not get activated, since the changed configuration is written in jdbcstore.propertiesproperties instead of jdbcstore.properties
  2. jdbcconfig & jdbcstore cannot be used with jndi enabled, since the postgresql driver is not available

The first issue is solved by just fixing the filename. The second issue is fixed by adding the respective line to enable jndi in jdbcconfig / jdbcstore.

if you need more details to understand these problems, please let me know.

Hope this helps :)

Copy link
Collaborator

@NyakudyaA NyakudyaA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tldev-de can we test this change, maybe provide a sample docker-compose with these settings and I will make a test in a separate PR

scripts/functions.sh Outdated Show resolved Hide resolved
scripts/functions.sh Outdated Show resolved Hide resolved
@tldev-de
Copy link
Contributor Author

tldev-de commented Nov 8, 2024

Sorry for the delay - I was really busy the last few days. I actually deliberately left the sed command outside the if condition, because that makes it possible to switch between jndi and direct connection by just changing the ENV-variable. But of course the comment makes more sense inside the if condition. I can change that again if you like. :)

Regarding the docker compose file - this should be a good example:

volumes:
  geoserver-data-dir:
  postgis-data:


services:
  db:
    image: kartoza/postgis:16-3.4
    volumes:
      - postgis-data:/var/lib/postgresql
    environment:
      - POSTGRES_DB=gis
      - POSTGRES_USER=docker
      - POSTGRES_PASS=docker
      - ALLOW_IP_RANGE=0.0.0.0/0
      - POSTGRES_MULTIPLE_EXTENSIONS=postgis,hstore,postgis_topology,postgis_raster,pgrouting
      - RUN_AS_ROOT=true
    restart: on-failure
    healthcheck:
      test: "PGPASSWORD=docker pg_isready -h 127.0.0.1 -U docker -d gis"

  geoserver:
    image: 'kartoza/geoserver:2.26.0'
    restart: 'always'
    volumes:
      - geoserver-data-dir:/opt/geoserver/data_dir
    environment:
      GEOSERVER_ADMIN_PASSWORD: myawesomegeoserver
      GEOSERVER_ADMIN_USER: admin
      INITIAL_MEMORY: 2G
      MAXIMUM_MEMORY: 4G
      POSTGRES_JNDI: TRUE # this is important to enable jndi
      HOST: db
      POSTGRES_DB: gis
      POSTGRES_USER: docker
      POSTGRES_PASS: docker
      COMMUNITY_EXTENSIONS: "jdbcconfig-plugin, jdbcstore-plugin" # this is important to enable the plugins
    depends_on:
      db:
        condition: service_healthy
    ports:
      - "8080:8080"
    healthcheck:
      test: ["CMD-SHELL", "curl --fail --silent --write-out 'HTTP CODE : %{http_code}\n' --output /dev/null -u $${GEOSERVER_ADMIN_USER}:$${GEOSERVER_ADMIN_PASSWORD} http://localhost:8080/geoserver/rest/about/version.xml"]
      interval: 1m30s
      timeout: 10s
      retries: 3

@NyakudyaA
Copy link
Collaborator

@tldev-de if you know it's better to have it outside then move it there. Also note that I moved the logic to be after setting the default configs. The extra configs already catered for this as you could add any king of configuration you could.

@NyakudyaA NyakudyaA merged commit aee4db9 into kartoza:develop Nov 8, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants