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

jboss module: add check mode support #58959

Merged
merged 18 commits into from Jul 15, 2019

Conversation

Projects
None yet
4 participants
@Andersson007
Copy link
Contributor

commented Jul 11, 2019

SUMMARY

jboss module:

  1. add check mode support
  2. add CI tests
  3. add useful info to DOC block
  4. change logic a bit (related to src param)
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

jboss

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

@Andersson007 this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

@Andersson007 Andersson007 force-pushed the Andersson007:jboss_add_check_mode_support branch from 137b3f4 to 538aa2d Jul 11, 2019

@Andersson007

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2019

ready_for_review

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

The test ansible-test sanity --test shebang [explain] failed with 1 error:

test/integration/targets/setup_wildfly_server/files/launch.sh:1:1: unexpected non-module shebang: b'#!/bin/bash'

The test ansible-test sanity --test shellcheck [explain] failed with 4 errors:

test/integration/targets/setup_wildfly_server/files/launch.sh:8:36: SC2086 Double quote to prevent globbing and word splitting.
test/integration/targets/setup_wildfly_server/files/launch.sh:8:42: SC2086 Double quote to prevent globbing and word splitting.
test/integration/targets/setup_wildfly_server/files/launch.sh:10:40: SC2086 Double quote to prevent globbing and word splitting.
test/integration/targets/setup_wildfly_server/files/launch.sh:10:46: SC2086 Double quote to prevent globbing and word splitting.

click here for bot help

@Andersson007

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2019

needs core team help.
the doubtful points are

  1. to implement CI tests for jboss module, it installs wildfly server by download archive (170MB), then extracts it (200MB), etc. is it OK? It seems too heavy.
  2. there are sanity errors related to the bash script that I copy.

If this approach is not OK in general, I could return it to the 2th commit state, but in this case there would be tests for check mode only

@mattclay
Copy link
Member

left a comment

The download sizes aren't an issue. We might want to upload the .war file to our S3 bucket though, so we could avoid adding it to the repo.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2019

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

test/integration/targets/jboss/tasks/main.yml:25:5: key-duplicates duplication of key "when" in mapping

click here for bot help

@ansibot ansibot added the ci_verified label Jul 12, 2019

@ansibot ansibot removed the ci_verified label Jul 12, 2019

@Andersson007

This comment has been minimized.

Copy link
Contributor Author

commented Jul 12, 2019

@mattclay @sdoran thank you for the detailed review!
I added many valuable things to my check list :) Much appreciated!
I started from adding check_mode support and CI tests for them with jboss behavior emulation.
Now jboss module is completely covered by actual tests, has better documentation and logic!

@Andersson007

This comment has been minimized.

Copy link
Contributor Author

commented Jul 12, 2019

ready_for_review

jboss module: add setup_remote_tmp_dir to meta, move post handling fr…
…om always to setup_wildfly_server handler
@Andersson007

This comment has been minimized.

Copy link
Contributor Author

commented Jul 15, 2019

ready_for_review

@samdoran samdoran merged commit 2a07123 into ansible:devel Jul 15, 2019

1 check passed

Shippable Run 132123 status is SUCCESS.
Details
@Andersson007

This comment has been minimized.

Copy link
Contributor Author

commented Jul 15, 2019

@samdoran , @mattclay thank you for reviewing and merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.