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

Update requirements and group_vars to use updated tomcat role #60

Merged
merged 3 commits into from
Apr 6, 2018

Conversation

whikloj
Copy link
Member

@whikloj whikloj commented Feb 26, 2018

Related to: https://github.com/Islandora-Devops/ansible-role-tomcat8/issues/2 and islandora-deprecated/ansible-role-tomcat8#3

This updates the requirements to pull in the updated ansible-role-tomcat8 and defines some needed variables (as the defaults in ansible-role-fcrepo-syn are now incorrect).

@seth-shaw-unlv
Copy link
Contributor

Thanks for doing the PR; I was just getting around to it.

@whikloj
Copy link
Member Author

whikloj commented Feb 26, 2018

@seth-shaw-unlv no worries, I figured you might be busy with...you know...work. 😄

@seth-shaw-unlv
Copy link
Contributor

Doesn't quite work. Running agains Ubuntu fails on the keymaster copy public key out because it wants to drop the key in /opt/tomcat/conf (which doesn't exist). At least the fcrepo_syn_tomcat_home variable should be moved to the OS-based vars files.

@DigitLib
Copy link
Contributor

I suggest that for both distos tomcat_home is /var/lib/tomcat8, this is path to tomcat in other roles, so we don't need to fix to new path on CentOS. I try this symlinked path on CentOS and work well with other roles.

@seth-shaw-unlv
Copy link
Contributor

I suppose that would be fine. I'm just leery of breaking with a distro's usual install locations for the sake of our own consistency. Really, this variable (and others like it) should be built off the tomcat8 variable.

@whikloj
Copy link
Member Author

whikloj commented Feb 28, 2018

I can just make fcrepo_syn_tomcat_home: {{ tomcat8_home }} which should resolve the differences between OSes.

@whikloj
Copy link
Member Author

whikloj commented Feb 28, 2018

@seth-shaw-unlv @DigitLib I tested this with Ubuntu and it seems to work. Could you give it a spin and see if it works for you.

@DigitLib
Copy link
Contributor

@whikloj Tested on CentOS and work.
@seth-shaw-unlv I agree with you.

@seth-shaw-unlv
Copy link
Contributor

Tested ok here on both CentOS and Ubuntu.

@whikloj
Copy link
Member Author

whikloj commented Feb 28, 2018

@Islandora-CLAW/committers I believe this is ready

** Edit** apparently I can't mention the Islandora-CLAW/committers team from this organization. So @dannylamb @DiegoPino @jonathangreen @Natkeeran

@whikloj
Copy link
Member Author

whikloj commented Mar 2, 2018

@seth-shaw-unlv sorry to do this to you, but I'm hoping once the other committers return someone will be able to burn through these quickly. So I merged islandora-deprecated/ansible-role-blazegraph#5, added a tag and this now also updates that requirement.

@DiegoPino
Copy link
Contributor

@whikloj can merge. Any other related to this that requires going together?

@whikloj
Copy link
Member Author

whikloj commented Mar 2, 2018

@DiegoPino if you want to test this and merge it I am fine with that, there is also #59 which is causing problems that @Natkeeran. He might be testing that one though.

@whikloj
Copy link
Member Author

whikloj commented Apr 6, 2018

@dannylamb @jonathangreen @DiegoPino @Natkeeran can this get a review please.

@DiegoPino
Copy link
Contributor

@whikloj looks good. Merging.

@DiegoPino DiegoPino merged commit 6e79913 into Islandora-Devops:master Apr 6, 2018
@whikloj whikloj deleted the tomcat-issue-2 branch September 24, 2021 16:21
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.

4 participants