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.

Sign up for GitHub

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

Jump to bottom

Pass etc_hosts through to docker_container module if set in molecule config #1571

Closed
wants to merge 1 commit into from
Closed

Pass etc_hosts through to docker_container module if set in molecule config #1571

wants to merge 1 commit into from

Conversation

jzafran
Copy link

@jzafran jzafran commented Nov 8, 2018 •
edited

PR Type

  • Feature Pull Request

Set etc_hosts parameter when using docker_container Ansible module in create.yml, which makes it possible to add entries to /etc/hosts in the container (it's not possible to modify that file once the container is started) if etc_hosts is set in the platforms item.

For example:

platforms:
  - name: xenial
    image: ubuntu:xenial
    etc_hosts:
      'test.local': '127.0.0.1'

Results in an entry in /etc/hosts

root@xenial:/# grep test /etc/hosts
127.0.0.1 test.local

@webknjaz
Copy link
Member

webknjaz commented Nov 9, 2018

Hi @jzafran, thanks for your contribution! Please rebase this PR on top of master.

@jzafran
set etc_hosts in docker_container module if set in molecule config
4b961c2 
Signed-off-by: Jeremy Zafran <jzafran@users.noreply.github.com>
@jzafran
Copy link
Author

jzafran commented Nov 9, 2018

@webknjaz all set; I've rebased it onto the latest master.

@decentral1se
Copy link
Contributor

Hey @jzafran, can you try to add a test for this change. I believe you can follow the examples shown in #1535 and #1543 which add new fields to the docker driver. Thanks!

decentral1se
decentral1se suggested changes Jan 31, 2019
View reviewed changes
Copy link
Contributor

@decentral1se decentral1se left a comment

Choose a reason for hiding this comment

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

Just a test needed and then we're good! Thanks for this PR!

molecule/provisioner/ansible/playbooks/docker/create.yml
@@ -75,6 +75,7 @@
networks: "{{ item.networks | default(omit) }}"
network_mode: "{{ item.network_mode | default(omit) }}"
dns_servers: "{{ item.dns_servers | default(omit) }}"
etc_hosts: "{{ item.etc_hosts | default(omit) }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to mark with review interface. Please see #1571 (comment).

@themr0c themr0c added this to the v.2.21 milestone Jan 31, 2019
gundalow
gundalow requested changes Jan 31, 2019
View reviewed changes
Copy link
Contributor

@gundalow gundalow left a comment

Choose a reason for hiding this comment

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

Please could you add some tests?

@gundalow gundalow added bug and removed bug labels Jan 31, 2019
@themr0c themr0c mentioned this pull request Feb 20, 2019
Closed
@decentral1se decentral1se removed this from the v.2.21 milestone Feb 27, 2019
@benjah1 benjah1 mentioned this pull request Apr 14, 2019
Merged
@ssbarnea
Copy link
Member

Closing due to lack of activity.

@ssbarnea ssbarnea closed this Jul 31, 2019
@jzafran jzafran deleted the add-etc_hosts-to-docker branch June 21, 2020 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gundalow gundalow gundalow requested changes

@decentral1se decentral1se decentral1se requested changes

@themr0c themr0c themr0c approved these changes

@webknjaz webknjaz Awaiting requested review from webknjaz

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@jzafran @webknjaz @decentral1se @ssbarnea @themr0c @gundalow