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

docker_image, docker_image_facts, docker_volume: add basic integration tests #47383

Merged
merged 4 commits into from
Oct 26, 2018
Merged

docker_image, docker_image_facts, docker_volume: add basic integration tests #47383

merged 4 commits into from
Oct 26, 2018

Conversation

felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Oct 20, 2018 •
edited

SUMMARY

Adds basic integration tests (in the style as for docker_container and docker_network) to docker_image, docker_image_facts, and docker_volume.

Fixes a bug in docker_volume which caused volumes to only be recreated if force == yes and configuration changed (instead of or). (moved to #47390)

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

docker_image
docker_image_facts
docker_volume

ANSIBLE VERSION
2.8.0

@ansibot
Copy link
Contributor

ansibot commented Oct 20, 2018

Hi @felixfontein, thank you for submitting this pull-request!

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Oct 20, 2018

cc @chouseknecht @softzilla
click here for bot help

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. cloud community_review In order to be merged, this PR must follow the community review workflow. docker module This issue/PR relates to a module. needs_maintainer Ansibot is unable to identify maintainers for this PR. (Check `author` in docs or BOTMETA.yml) needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. labels Oct 20, 2018
@felixfontein
Copy link
Contributor Author

CC "the usual suspects" @akshay196 @danihodovic @dariko @jwitko @kassiansun @pilou- @tbouvet :)

@felixfontein
Copy link
Contributor Author

CC @agronholm as author of docker_volume (no idea why the bot ignored you...)

bot_status

@ansibot
Copy link
Contributor

ansibot commented Oct 20, 2018

Components

changelogs/fragments/docker_volume-force-change-detection.yaml
support: community
maintainers:

lib/ansible/modules/cloud/docker/docker_volume.py
support: community
maintainers:

test/integration/targets/docker_image/aliases
support: community
maintainers: chouseknecht softzilla

test/integration/targets/docker_image/meta/main.yml
support: community
maintainers: chouseknecht softzilla

test/integration/targets/docker_image/tasks/main.yml
support: community
maintainers: chouseknecht softzilla

test/integration/targets/docker_image/tasks/run-test.yml
support: community
maintainers: chouseknecht softzilla

test/integration/targets/docker_image/tasks/tests/basic.yml
support: community
maintainers: chouseknecht softzilla

test/integration/targets/docker_image_facts/aliases
support: community
maintainers: chouseknecht

test/integration/targets/docker_image_facts/meta/main.yml
support: community
maintainers: chouseknecht

test/integration/targets/docker_image_facts/tasks/main.yml
support: community
maintainers: chouseknecht

test/integration/targets/docker_volume/aliases
support: community
maintainers:

test/integration/targets/docker_volume/meta/main.yml
support: community
maintainers:

test/integration/targets/docker_volume/tasks/main.yml
support: community
maintainers:

test/integration/targets/docker_volume/tasks/run-test.yml
support: community
maintainers:

test/integration/targets/docker_volume/tasks/tests/basic.yml
support: community
maintainers:

Metadata

waiting_on: maintainer
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 0
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 1
shipit_actors (maintainers or core team members): felixfontein
shipit_actors_other: []
automerge: automerge shipit test failed

click here for bot help

@felixfontein felixfontein mentioned this pull request Oct 20, 2018
Closed
agronholm
agronholm reviewed Oct 21, 2018
View reviewed changes
lib/ansible/modules/cloud/docker/docker_volume.py Outdated
@@ -223,7 +223,7 @@ def present(self):
if self.existing_volume:
differences = self.has_different_config()

if differences and self.parameters.force:
if differences or self.parameters.force:
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a fix that should come as a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I can move it into a separate PR. I'll then rebase this PR once that PR is merged (as otherwise the test won't work :) ).

@felixfontein felixfontein mentioned this pull request Oct 21, 2018
Merged
@felixfontein
Copy link
Contributor Author

I rebased against current devel (now that #47390 has been merged).

@ansibot ansibot removed the module This issue/PR relates to a module. label Oct 23, 2018
@kassiansun
Copy link
Contributor

shipit

@jwitko
Copy link
Contributor

jwitko commented Oct 23, 2018

shipit

@jborean93 jborean93 removed the needs_triage Needs a first human triage before being processed. label Oct 26, 2018
@jborean93
Copy link
Contributor

Thanks @felixfontein, are you able to backport these tests to stable-2.7 and stable-2.6?

@jborean93 jborean93 merged commit f19ab56 into ansible:devel Oct 26, 2018
@felixfontein
Copy link
Contributor Author

@jborean93 I'll create a 2.7 backport, but 2.6 is more complicated (since it is based on several other PRs which aren't backported). I once tried to backport a similar testsuite (for docker_container), and gave up after cherry-picking several PRs and still not finding out what else needs to be cherry-picked for it... I might try it again later, but for now I'll stick to 2.7 :)

@felixfontein felixfontein deleted the docker-tests branch October 26, 2018 05:39
@jborean93
Copy link
Contributor

No worries, just do what you can.

felixfontein added a commit to felixfontein/ansible that referenced this pull request Oct 27, 2018
@felixfontein
docker_image, docker_image_facts, docker_volume: add basic integratio…
6c3179b 
…n tests (ansible#47383)

* Add docker_image_facts tests.

* Add basic integration test for docker_volume.

* Add basic docker_image tests.

* Only start test registry when tests are actually run (i.e. not on CentOS 6).

(cherry picked from commit f19ab56)
@felixfontein felixfontein mentioned this pull request Oct 27, 2018
Merged
abadger pushed a commit that referenced this pull request Oct 31, 2018
@felixfontein @abadger
docker_image, docker_image_facts, docker_volume: add basic integratio…
37da0b2 
…n tests (#47383)

* Add docker_image_facts tests.

* Add basic integration test for docker_volume.

* Add basic docker_image tests.

* Only start test registry when tests are actually run (i.e. not on CentOS 6).

(cherry picked from commit f19ab56)
Tomorrow9 pushed a commit to Tomorrow9/ansible that referenced this pull request Dec 4, 2018
@felixfontein @Tomorrow9
docker_image, docker_image_facts, docker_volume: add basic integratio…
281b5ca 
…n tests (ansible#47383)

* Add docker_image_facts tests.

* Add basic integration test for docker_volume.

* Add basic docker_image tests.

* Only start test registry when tests are actually run (i.e. not on CentOS 6).
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@agronholm agronholm agronholm left review comments

@jwitko jwitko jwitko approved these changes

@kassiansun kassiansun kassiansun approved these changes

Assignees
No one assigned
Labels
affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. cloud community_review In order to be merged, this PR must follow the community review workflow. docker needs_maintainer Ansibot is unable to identify maintainers for this PR. (Check `author` in docs or BOTMETA.yml) support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@felixfontein @ansibot @kassiansun @jwitko @jborean93 @agronholm