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

Ensure tomcat native installs and starts correctly #120

Merged
merged 8 commits into from
May 24, 2022
Merged

Ensure tomcat native installs and starts correctly #120

merged 8 commits into from
May 24, 2022

Conversation

guidograzioli
Copy link
Member

Fix #115
Fix #114

@guidograzioli guidograzioli added the bugfixes Fixes that resolve issues. SHOULD not be used for minor enhancements label May 18, 2022
@guidograzioli
Copy link
Member Author

Apache tomcat does not distribute native archives, so we can only test this on JWS

@guidograzioli guidograzioli force-pushed the tomcat_native_starts_correctly branch 2 times, most recently from f326fa5 to cfeca5a Compare May 18, 2022 12:26
csutherl
csutherl requested changes May 18, 2022
View reviewed changes
Copy link
Collaborator

@csutherl csutherl left a comment

Choose a reason for hiding this comment

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

The error check should be modified a bit, but otherwise looks good.

roles/jws_validation/tasks/catalina_out.yml Outdated Show resolved Hide resolved
@csutherl
Copy link
Collaborator

Apache tomcat does not distribute native archives, so we can only test this on JWS

I was thinking about adding a feature to download the sources for tomcat-native from the ASF to build and test it with the Apache Tomcat distribution as well. Then we could test it outside of JWS as well, if we wanted.

@guidograzioli guidograzioli force-pushed the tomcat_native_starts_correctly branch 2 times, most recently from 90125a2 to 52b76aa Compare May 18, 2022 14:55
csutherl
csutherl requested changes May 18, 2022
View reviewed changes
Copy link
Collaborator

@csutherl csutherl left a comment

Choose a reason for hiding this comment

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

Just one minor doc change request, but everything else looks OK.

roles/jws/README.md Outdated Show resolved Hide resolved
@guidograzioli guidograzioli force-pushed the tomcat_native_starts_correctly branch 3 times, most recently from b6f9cd2 to bbcde52 Compare May 18, 2022 17:40
csutherl
csutherl requested changes May 18, 2022
View reviewed changes
Copy link
Collaborator

@csutherl csutherl left a comment

Choose a reason for hiding this comment

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

The molecule test is failing because of an undefined variable, tomcat_zipfile.

molecule/local_with_native/prepare.yml Show resolved Hide resolved
@guidograzioli guidograzioli force-pushed the tomcat_native_starts_correctly branch from bbcde52 to 21c1889 Compare May 19, 2022 07:42
@guidograzioli guidograzioli force-pushed the tomcat_native_starts_correctly branch from 21c1889 to eb9c507 Compare May 19, 2022 14:32
csutherl
csutherl approved these changes May 23, 2022
View reviewed changes
Copy link
Collaborator

@csutherl csutherl left a comment

Choose a reason for hiding this comment

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

LGTM

molecule/local_with_native/prepare.yml Show resolved Hide resolved
@guidograzioli guidograzioli merged commit 8b8202f into ansible-middleware:main May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@csutherl csutherl csutherl approved these changes

Assignees
No one assigned
Labels
bugfixes Fixes that resolve issues. SHOULD not be used for minor enhancements
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

JWS native zip install is missing dependencies Add log file check to jws_validation role
2 participants
@guidograzioli @csutherl