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

Add extra-settings script #1016

Merged
merged 9 commits into from
Mar 7, 2024

Conversation

YevhenZvieriev
Copy link
Contributor

@YevhenZvieriev YevhenZvieriev commented Dec 4, 2023

This script is designed to simplify the configuration of the Docker environment and related settings. The primary tasks it addresses are:

  1. Docker Container IP Address:

    • Adds the container's IP address to the /etc/hosts file for convenient access.
  2. Opening Port 9003 for Xdebug:

    • Provides the option to open port 9003 for Xdebug.
  3. Increasing Virtual Memory Map Count for Elasticsearch:

    • Allows increasing the virtual memory map count for Elasticsearch if needed.

User Instructions:

  1. Execute the script to perform the necessary configurations.

  2. Follow the prompts and choose options according to your preferences.

  3. Verify that all tasks have been completed successfully.

Please feel free to reach out if you have any questions or suggestions for improving the script.

Screenshot_2023-12-04_17-00-45

Copy link

what-the-diff bot commented Dec 4, 2023

PR Summary

  • New Configuration Script for Linux Users
    A fresh script has been included, specifically designed for Linux users. This script performs several tasks, including verifying if the user is operating on a Linux-based system, acquiring the IP address automatically from any running Docker container, and updating the '/etc/hosts' file appropriately. It further guides the user to unlock port 9003 for Xdebug operations and ultimately confirms successful completion with a message.

  • Enhanced Virtual Memory Allocation
    In the compose/compose.yaml file, a few adjustments have been made to boost the virtual memory map count. By unlocking the max_map_count=262144 line, we've expanded the virtual memory allocation for both opensearch and elasticsearch systems, which will potentially improve performance.

@markshust
Copy link
Owner

I like the ideas in this PR. However, I don't really like how this is in a non-descript "extra-settings" file. It feels a bit out of place & random.

Does it make sense to rename this to bin/configure-linux? I think the host.docker.internal ip is automatically taken care of for you in macOS, and the iptables command doesn't exist (or is needed) on mac. This would also open up this script for possible future enhancements that are specific to Linux.

And I don't think we need the elasticsearch command at all, because I think we can set this memory param as an environment variable at https://github.com/markshust/docker-magento/blob/master/compose/compose.yaml#L63. If so, it would make sense to just add this max_map_count prop as a commented-out setting under that environment property right in the compose.yaml file, along with a comment-out description for what it does.

@YevhenZvieriev
Copy link
Contributor Author

Hi, Mark!

Thank you for your feedback.
Good idea!

  1. I've renamed the script name from bin/extra-settings to bin/configure-linux, and modified the script.

  2. I've added the max_map_count parameter for Elasticsearch and OpenSearch with comments to the compose.yaml file.

Make shebang more portable, capitalized X in Xdebug.
Standardized OS check
Switched response logic around
@markshust
Copy link
Owner

I made a few updates to make the script more portable, consistent with the logic within other scripts, and simplify the logic.

@markshust markshust merged commit 69f29de into markshust:master Mar 7, 2024
1 check 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