-
Notifications
You must be signed in to change notification settings - Fork 15
install tomcat8 from Apache binaries for centos #3
install tomcat8 from Apache binaries for centos #3
Conversation
So the Also since you added a RedHat vars file, why not add a Debian one and get rid of this conditional? Spit balling here as I haven't tried to run it yet. |
I'll play with it a little bit and probably report back on Monday. Constantly reprovisioning takes time, especially when switching distros. |
Would have had this in on Monday but I was unexpectedly out of the office. Anyway, I took @whikloj 's suggestions for tidying up the role. Let me know if there is anything else I could improve on it. |
👍 nice I'll try to spin up a box shortly. |
Fixing a mistake made while mashing the Debian and RedHat config tasks together.
The PR also addresses this issue on the CLAW repo. |
Changed EOL version 8.0.x to latest 8.5.x
Newest commit changes the Tomcat version to the 8.5.x branch. Partially addressing issue #4 (Tomcat 8.0.x EOL). |
|
@@ -1,11 +1,16 @@ | |||
--- | |||
|
|||
- include: install.yml | |||
# Include variables and define needed variables. | |||
- name: Include OS-specific variables. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably conditionally set the variables here, as we did in Karaf, so they can be overridden.
Maybe some comments in the defaults as well...
I know the Karaf stuff just went in this afternoon, just wanted to make a note here so it doesn't get forgotten.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
templates/setenv.sh.j2
Outdated
@@ -0,0 +1,3 @@ | |||
#!/bin/sh | |||
|
|||
export JAVA_OPTS="$JAVA_OPTS {{tomcat8_java_opts|join(' ')}}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is kind of a nitpick, so feel free to ignore, but I find jinja templates way easier to read with spaces after the opening delimiter and before the closing delimiter like this:
{{ tomcat8_java_opts|join(' ') }}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
I wonder if we should split up (maybe just with comments) the Right now we have variables like Maybe just something like: # ---- Ubuntu ----
# These variables are only used when installing on Ubuntu
# Java compiler to use for translating JavaServer Pages (JSPs). You can use all
# compilers that are accepted by Ant's build.compiler property.
tomcat8_jsp_compiler: javac
# Use the Java security manager? (yes/no, default: no)
tomcat8_tomcat8_security: "no"
# Number of days to keep logfiles in /var/log/tomcat8. Default is 14 days.
tomcat8_logfile_days: 14
# Whether to compress logfiles older than today's
tomcat8_logfile_compress: 1
# Location of the JVM temporary directory
# WARNING: This directory will be destroyed and recreated at every startup !
#tomcat8_jvm_tmp: /tmp/tomcat8-temp
# If you run Tomcat on port numbers that are all higher than 1023, then you
# do not need authbind. It is used for binding Tomcat to lower port numbers.
# (yes/no, default: no)
tomcat8_authbind: no |
* Add CI testing. * Update readme
@seth-shaw-unlv I'm getting this
|
@whikloj Do you have the repo-remi PR applied to your test? |
@seth-shaw-unlv yes I do, it is weird to me. Maybe we need other of the remi-repos installed? This worked when I tested it. But now
I don't think Centos uses libapache2-mod-php7.0 I think this is an Ubuntu thing. So maybe something in pulling this tomcat role overtop of claw-playbook master has messed something up? |
Hmm... BTW, did you include crayfish in this build? The crayfish.yml in claw-playbook also calls geerlingguy.php and that might have caused the problem. Comment out this line and try again. |
Hmmm I think this is due to overlapping, I seem to have the two new php-DISTRO.yml file but still have the original php.yml file too. |
@seth-shaw-unlv @jonathangreen @dannylamb Am I correct? I'm starting to see Ansible as a huge pain for all this tweaking to make 2 OSes work. |
@whikloj yes you are correct, that is what we are doing with the variables and why. As for Ansible, as the old saying goes: familiarity breeds contempt? 🤷♂️ |
@jonathangreen yeah I guess, the decision for variable precendence where the actual inventory are at the low end of priority seems counter-intuitive. The defaults are low, but if you want to include vars (like separate ones for each OS) then suddenly you are unable to override those from your inventory. Long whining story short. I guess we need to make a ticket to unset all the defaults in our roles and add a task to use the loaded vars file with the same |
Overridding the OS-specific variables is done by setting the usual variable name (NOT the one prefixed by underscores).
Actually, the way we have it now, the variables prefixed by an underscore are only set in the vars/* so our defaults and group_vars still work as expected (vars/* with underscored prefix is ignored if we have either defaults or group_vars; group_vars takes precedence over defaults). Adding the underscore prefixes to the defaults/main.yml and README was a mistake now corrected. @whikloj yeah, I've just spent about a month fighting that battle. |
The www-eu URL only keeps the most recent binary, breaking the link with each new version. Using the archive URL so it doesn't keep breaking. Also updated the minor version.
@seth-shaw-unlv can you rebase this to fix this conflict and I'll try it again and see if I can't start clearing out these Centos PRs |
tidy up config and install scripts tomcat-defaults.j2 is a Debian task, not a RH one Fixing a mistake made while mashing the Debian and RedHat config tasks together. Changed tomcat version Changed EOL version 8.0.x to latest 8.5.x allow group_vars and revised defaults to overridde OS-specific variables spacing nit-pick Account for os-specific variables not set corrected default variables and README Overridding the OS-specific variables is done by setting the usual variable name (NOT the one prefixed by underscores). more stable binary url The www-eu URL only keeps the most recent binary, breaking the link with each new version. Using the archive URL so it doesn't keep breaking. Also updated the minor version.
@whikloj Well, I think I did that right. |
@seth-shaw-unlv this is good to merge, but can I get a follow-up PR on claw-playbook to fix this line.
I'll fix up that role later to not need to overwrite two variables, but for now we do need that. |
@seth-shaw-unlv I'm going to merge this and tag it as |
tomcat8_home in /vars/RedHat.yml need to change to /var/lib/tomcat8 because of later roles which have defined that path. |
Addresses issue #2
Integrates DigitLib's work into a single role that supports both Debian and CentOS. Installs Tomcat8 from source on CentOS machines.