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

IBX-522: Introduced Docker image for Solr standalone #214

Draft
wants to merge 3 commits into
base: 3.3
Choose a base branch
from

Conversation

Steveb-p
Copy link
Contributor

@Steveb-p Steveb-p commented Jun 8, 2021

Question Answer
JIRA issue IBX-522
Type improvement
Target Ibexa DXP version v4.0, v3.3
BC breaks no

Note: Target version is irrelevant. It's a docker image for use in CI / local dev.

Note: (:exclamation:) Most of the configuration files are actually copies of existing files (:exclamation:), that are present in lib. They have been copied to reduce the Docker context that is copied into Docker engine when doing the build.

❗ Warning ❗

Current version of this PR builds an invalid image. Somehow files copied during build time from the extended context differ from the ones that were present when they were copied into docker/solr directory.

One file has been renamed: schema.xml -> managed-schema, as that's the file that is actually loaded in the Solr docker image, apparently. schema.xml had no effect if left as is.

@Steveb-p Steveb-p requested review from adamwojs, ibexa-yuna and a team June 8, 2021 08:40
Comment on lines +7 to +8
RUN sed --in-place '/<updateRequestProcessorChain name="add-unknown-fields-to-the-schema".*/,/<\/updateRequestProcessorChain>/d' server/solr/configsets/_default/conf/solrconfig.xml
RUN sed --in-place 's/${solr.autoSoftCommit.maxTime:-1}/${solr.autoSoftCommit.maxTime:20}/' server/solr/configsets/_default/conf/solrconfig.xml
Copy link
Contributor

Choose a reason for hiding this comment

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

Why and what are those?

Copy link
Contributor Author

@Steveb-p Steveb-p Jun 8, 2021

Choose a reason for hiding this comment

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

They come from:

# Adapt autoSoftCommit to have a recommended value, and remove add-unknown-fields-to-the-schema
sed -i.bak '/<updateRequestProcessorChain name="add-unknown-fields-to-the-schema".*/,/<\/updateRequestProcessorChain>/d' $DESTINATION_DIR/solrconfig.xml
sed -i.bak2 's/${solr.autoSoftCommit.maxTime:-1}/${solr.autoSoftCommit.maxTime:20}/' $DESTINATION_DIR/solrconfig.xml

They are the only commands (other than straight up copying configuration files to the service) that are required to mimic the behavior of host-based Solr (created when init_solr.sh is executed).

From what I understand first disables automatic field registration as schema, and the second makes Solr commit more often.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are they really needed in a test environment i.e. not a long-running instance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are they really needed in a test environment i.e. not a long-running instance?

As described by @adamwojs in a personal conversation with me, they are needed because sometimes we test against field being empty in search engine, and this apparently is required for it to work. I didn't dig deeper tbh.

Copy link
Member

Choose a reason for hiding this comment

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

Are they really needed in a test environment i.e. not a long-running instance?

Yes, these config changes are required for test environment.

@Steveb-p Steveb-p changed the title IBX-519: Introduced Docker image for Solr standalone IBX-522: Introduced Docker image for Solr standalone Jun 8, 2021
@ViniTou
Copy link
Contributor

ViniTou commented Jun 8, 2021

Please take a look on:
https://github.com/ibexa/installer/pull/20

@Steveb-p
Copy link
Contributor Author

Steveb-p commented Jun 8, 2021

Please take a look on:
ibexa/installer#20

Looks good, but the solution ultimately different, on a different level.

@alongosz provided a docker-compose.yaml file that sets up containers using images provided.
My solution creates an image to be used in such an orchestration. I actually can solve a few things he's asking in his PR:

  • container name conflict
  • port conflict
  • creating multiple collections
    and I'm not using volume for container configuration - it's baked into the image instead.

I feel that images and containers are confused with each other, and the docker-compose is not really understood.

Here is an example of a docker-compose.yaml file that I used for local tests:

version: '3'

services:
    elasticsearch:
        image: elasticsearch:7.12.1
        ports:
            - '127.0.0.1::9200/tcp'
        # use --compatibility flag on non-swarm to prevent docker from claiming all resources, especially on startup
        deploy:
            resources:
                limits:
                    cpus: '1'
                    memory: 500M
        healthcheck:
            test: ["CMD", "curl", "http://localhost:9200/_cluster/health"]
            interval: 10s
            timeout: 5s
            retries: 10
        environment:
            discovery.type: single-node

    elasticsearch-index:
        depends_on:
            - elasticsearch
        build: ./docker/elasticsearch-index
        command:
            - "elasticsearch:9200"
            - "--strict"
            - '--timeout=0'
            - "--"
            - "curl"
            - "-X"
            - "PUT"
            - "-H"
            - "Content-Type: application/json"
            - "-d"
            - "@./es-index-template.json"
            - "http://elasticsearch:9200/_template/repository"

    solr:
        image: ghcr.io/ibexa/product-catalog/solr:latest
        healthcheck:
            test: [ "CMD", "solr", "status" ]
            interval: 10s
            timeout: 5s
            retries: 10
        ports:
            - '127.0.0.1::8983/tcp'

Note that:

  1. I don't specify container names (preventing container name conflict). Container name in this case is a concatenation of COMPOSE_PROJECT_NAME (directory of the first docker-compose.yaml file by default) with service name, for example product-catalog_solr (where product-catalog is directory I'm working in).
  2. Ports are not specified explicitly. I allow docker to assign a free port by itself instead. I can get the port name by running docker-compose port solr 8983 (sample output: 127.0.0.1:41720)
  3. Healthchecks can be omitted or added into docker image (I actually should do that instead of declaring it here)
  4. deploy configuration is only for myself, as elasticsearch caused my private pc to freeze, since I have less cpu / ram.

@Steveb-p
Copy link
Contributor Author

Steveb-p commented Jun 9, 2021

Configuration file duplication can be avoided by using project directory as docker build context (and passing Dockerfile location, not using default):

docker build -f docker/solr/Dockerfile .

EDIT: Well, it almost works...

When copying in the most recent version there seems to be an issue with

"SolrCore 'collection1' is not available due to init failure: Could not load conf for core collection1: Error while parsing currency configuration file currency.xml"

@Steveb-p Steveb-p marked this pull request as draft June 10, 2021 21:24
@Steveb-p Steveb-p self-assigned this Jun 10, 2021
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 1, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@ibexa-yuna ibexa-yuna changed the base branch from master to 3.3 November 19, 2021 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants