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

Fix bug where environment variable contain space. #179

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix bug where environment variable contain space. #179

wants to merge 2 commits into from

Conversation

sandanilenko
Copy link

For example "ES_JAVA_OPTS=-Xms512m -Xmx512m"

For example "ES_JAVA_OPTS=-Xms512m -Xmx512m"
@renskiy
Copy link
Owner

renskiy commented May 21, 2018

Don't you think this changes should be applied only to env Option?

@sandanilenko
Copy link
Author

Bug was found where i used env and added condition when value is string type and containt space symbol. This condition true only for env option, so i don't added it.

@renskiy
Copy link
Owner

renskiy commented May 21, 2018

BTW, you can try following code snippet for your case:

from six.moves import shlex_quote

docker.Container(
    options={'env': 'ES_JAVA_OPTS=%s' % shlex_quote('-Xms512m -Xmx512m')},
)

@sandanilenko
Copy link
Author

sandanilenko commented May 21, 2018

I need to use several environment variables and i use list of variables. If use shlex_quote I get next result --env=''"'"'ES_JAVA_OPTS'='-Xms512m -Xmx512m'"'"''

@renskiy
Copy link
Owner

renskiy commented May 21, 2018

If use shlex_quote I get next result --env=''"'"'ES_JAVA_OPTS'='-Xms512m -Xmx512m'"'"''

it's OK. so does escaping work :) But you need use shlex_quote only for env value, -Xms512m -Xmx512m in this case

@sandanilenko
Copy link
Author

Yes, you are right. May be not need use my decision for all variants. Thanks.

@sandanilenko
Copy link
Author

sandanilenko commented May 21, 2018

Don't work with list of env variables:

@tasks.infrastructure
def master():
    fab.env.master_env.extend([
        "bootstrap.memory_lock=true",
        'ES_JAVA_OPTS={}'.format(shlex_quote('-Xms512m -Xmx512m')),
    ])

    fab.env.update(
        roledefs={
            'web': ['bars@test-ip'],
        },
    )


elasticsearch = tasks.ImageBuildDockerTasks(
    build_path='./elasticsearch',
    service=docker.Container(
        name='elasticsearch',
        image='registry:5000/logging/elasticsearch:latest',
        options=dict(
            publish='9200:9200',
            network='web_bb_logging',
            volume=['esdata:/usr/share/elasticsearch/data', ],
            env=fab.env.master_env
            # ulimit='memlock=-1:-1'
        ),

    ),
    roles=['web']
)

generate docker run command

run: docker run --publish=9200:9200 --env=bootstrap.memory_lock=true --env='ES_JAVA_OPTS='"'"'-Xms512m -Xmx512m'"'"'' --volume=esdata:/usr/share/elasticsearch/data --net=web_bb_logging --name=elasticsearch --detach registry:5000/logging/elasticsearch:latest

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