Skip to content

Conversation

@arhiopterecs
Copy link
Contributor

@arhiopterecs arhiopterecs commented Mar 4, 2020

Description

Added option --es-env-var for docker-compose.yml file generator. Using this option, you can specify the environment variables that will be applied when starting the Elasticsearch container.
This will allow you to change service settings, such as heap size for JVM or Elasticsearch options.
Parameter changes for JVM are available from image version 1.7
Changing Elasticsearch parameters using environment variables is available starting from version 5.2

Fixed Issues (if relevant)

  1. https://magento2.atlassian.net/browse/MAGECLOUD-4164

Manual testing scenarios

  1. Clone [magento-cloud template](https://github.com/magento/magento-cloud).
  2. Run: $ composer update in the magento-cloud template directory:
  3. Generate the docker-compose.yml file with env variables for elasticsearch service:
$ php vendor/bin/ece-docker build:compose --es-env-var=ES_JAVA_OPTS="-Xms512m -Xmx512m" --es-env-var=node.store.allow_mmapfs=false
Configuration was built.

NOTE: By default, an image with Elasticsearch version 6.5 is used. For this version of Elasticsearch, the option node.store.allow_mmapfs exists.
For image varsions 5.2, 2.4, 1.7 ,this option does not exist
Forimage varsions 7.5, this parameter was renamed to node.store.allow_mmap
4. Check the file docker-compose.yml, the following settings must exist:

... 
elasticsearch:
    hostname: elasticsearch.magento2.docker
    image: 'magento/magento-cloud-docker-elasticsearch:6.5-1.1'
    environment:
      - 'ES_JAVA_OPTS=-Xms512m -Xmx512m'
      - node.store.allow_mmapfs=false
    networks:
      magento:
        aliases:
          - elasticsearch.magento2.docker           
....  
  1. Run: $ docker-compose up -d. Containers started successfully
  2. Login in elasticsearch container:
$ docker-compose exec elasticsearch bash
  1. Run the command in the container: $ ps aux | grep -E "(Xms|Xmx)"
    Expected: The last values ​​of the -Xms and -Xmx parameters are 512m

  2. Run the command in the container: $ curl http://localhost:9200/_nodes 2>&1 | grep 'mmapfs'.
    Expected:"allow_mmapfs":"false" should be present

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages

Source\CliSource::OPTION_ES_EVN_VAR,
null,
InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY,
"Environment variable for elasticsearch service"
Copy link
Contributor

Choose a reason for hiding this comment

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

 It would be good to add example for this option.

NadiyaS
NadiyaS previously approved these changes Mar 9, 2020
NadiyaS
NadiyaS previously approved these changes Mar 17, 2020
Source\CliSource::OPTION_ES_EVN_VAR,
null,
InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY,
"Environment variable for elasticsearch service"
Copy link
Member

Choose a reason for hiding this comment

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

Please use single quotes

Copy link
Member

Choose a reason for hiding this comment

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

I believe it's also optional by default

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

/**
* Environment variable for elasticsearch service.
*/
public const OPTION_ES_EVN_VAR = 'es-env-var';
Copy link
Member

Choose a reason for hiding this comment

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

EVN => ENV

Copy link
Member

Choose a reason for hiding this comment

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

Please use full name, OPTION_ES_ENVIRONMENT_VARIABLE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

/**
* ES environment variables
*/
public const SERVICES_ES_ENV_VARS = self::SERVICES_ES.'.'.'env-vars';
Copy link
Member

Choose a reason for hiding this comment

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

I would also suggest dropping env part, variables are already self-explaining

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@arhiopterecs
Copy link
Contributor Author

PR was recreated #174

@arhiopterecs arhiopterecs deleted the MAGECLOUD-4164-2 branch March 23, 2020 20:55
@arhiopterecs arhiopterecs removed the request for review from BaDos March 23, 2020 20:56
magento-devops-reposync-svc pushed a commit that referenced this pull request Oct 8, 2025
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.

5 participants