Crayfish Ansible role #9

ajs6f · 2017-08-30T17:29:59Z

dannylamb · 2017-09-07T13:01:47Z

vagrant up

dannylamb · 2017-09-07T15:14:07Z

crayfish.yml needs to be added to the main playbook.yml or it won't deploy Crayfish when running vagrant up.

dannylamb · 2017-09-07T15:43:12Z

Looks like there's a permissions issue that's stopping the whole show. Requests to Hypercube are giving me this in apache's error log:

[Thu Sep 07 15:40:55.193217 2017] [:error] [pid 30864] [client 10.0.2.2:48650] PHP Fatal error:  Uncaught UnexpectedValueException: The stream or file "../hypercube.log" could not be opened: failed to open stream: Permission denied in /var/www/html/Crayfish/Hypercube/vendor/monolog/monolog/src/Monolog/Handler/StreamHandler.php:107\nStack trace:\n#0 /var/www/html/Crayfish/Hypercube/vendor/monolog/monolog/src/Monolog/Handler/AbstractProcessingHandler.php(37): Monolog\\Handler\\StreamHandler->write(Array)\n#1 /var/www/html/Crayfish/Hypercube/vendor/monolog/monolog/src/Monolog/Handler/GroupHandler.php(67): Monolog\\Handler\\AbstractProcessingHandler->handle(Array)\n#2 /var/www/html/Crayfish/Hypercube/vendor/monolog/monolog/src/Monolog/Logger.php(337): Monolog\\Handler\\GroupHandler->handle(Array)\n#3 /var/www/html/Crayfish/Hypercube/vendor/monolog/monolog/src/Monolog/Logger.php(518): Monolog\\Logger->addRecord(550, 'Fatal Error (E_...', Array)\n#4 /var/www/html/Crayfish/Hypercube/vendor/monolog/monolog/src/Monolog/ErrorHandler.php(180): Monolog\\Logger->log(550, 'Fatal Error (E_...', Array)\n#5 [internal function]: Monolog in /var/www/html/Crayfish/Hypercube/vendor/monolog/monolog/src/Monolog/Handler/StreamHandler.php on line 107

dannylamb · 2017-09-07T15:45:06Z

@ajs6f Further inspection show that root owns all of Crayfish, but ubuntu owns drupal. So hopefully a recursive chown to ubuntu should do the trick.

dannylamb · 2017-09-07T19:51:45Z

roles/internal/crayfish/tasks/install.yml

+    name: "{{item}}"
+    state: present
+  with_items:
+    - tesseract-ocr


You'll need to add imagemagick to this list for Houdini. @jonathangreen Now that we're using grok, is there any need to use Lyrasis's PPA?

@jonathangreen bump

Sorry. missed this one in the flood of islandora release tickets yesterday.

Houdini is just a wrapper on imagemagick. We added grok to Cantaloupe. If we want Houdini to be able to work with grok or kdu, we'll have to do some work on it. That said there is certainly an argument that doing that could be beneficial, or that we should just totally dump Houdini and just use cantaloupe instead... but I think thats another ticket.

That was a long winded way of saying we need imagemagick, and if we want it compiled with the JP2 extensions, installing it from the Lyrasis PPA is probably the thing to do.

@jonathangreen You know me. I'm all about dumping stuff if we don't need to maintain it. I need to check out cantaloupe a little more and see how to replicate Houdini's convert functionality. If we can do that, I say we make a separate ticket and axe it.

@dannylamb I guess the downside would be that cantaloupe won't fit into the API-X model, but it is a image transformation service that we don't have to maintain...

Just to be clear, the landing here is that I need to get Imagemagick from Lyrasis, correct?

https://launchpad.net/~lyrasis/+archive/ubuntu/imagemagick-jp2

jonathangreen · 2017-09-08T15:52:58Z

So I have a few general thoughts on this role. As is it works, and I think that is good, so I don't want to stop it from being merged. Just thoughts and nits for future improvements that we could break out into a separate ticket if we don't want to handle them here:

Config files
- I'd really like to see us use variables for pretty much every value in the templates for every service. Then the default values for the templated variables can be defined in the defaults file. This lets the user of the role flexibly setup the config file. Here is an example of a templated config and a default variables.
- I like to see templated config files say they are managed by ansible at the top, so people are aware that they will get stomped on if ansible is run, and to maybe change it in the playbook instead.
- I like to use .j2 as the extension of template files. This seems to be standard. I can't find a definite reference for this, but it is used in the ansible template module docs. I've also seen it used a lot other roles.
- I'd like to see the .example part of the template filenames removed, it is a bit confusing if you don't realize that its taken from the naming of the example in the crayfish repos.
- I'd like to see the apache configs broken out into a file for each service instead of one single configuration.
Repo structure
- I'd like to see tags included, so we can run this role on its own, see the example below.
- I'd like to see the install.yml broken out into a yml file for each service, perhaps with a base yml file that would install the basics for everything, then have a variable to control if a service is installed, so that we can install some services, or other services. I'm thinking something like:

- include: install-base.yml
  tags: 
    - crayfish
    - crayfish-install-base
- include: install-houdini.yml
  tags: 
    - crayfish
    - crayfish-install-houdini
  when: crayfish_install_houdini
- include: install-gemini.yml
  tags: 
    - crayfish
    - crayfish-install-gemini
  when: crayfish_install_gemini

I'd like to use the handler for restarting apache thats defined in the apache role, since we are using the apache role here. This one is a bit more murky because it creates an implicit dependancy on a handler in another role, but it is better practice to reuse handlers, so that services don't have to restart multiple times. The other hander is defined here: https://github.com/geerlingguy/ansible-role-apache/blob/master/handlers/main.yml#L1-L5

ajs6f · 2017-09-08T21:26:13Z

Just leaving a breadcrumb: per IRC discussion (man, we got to get an IRC logger that leaves anchors in the HTML) I am going to add some commits at the top of next week to meet @jonathangreen 's excellent advice.

…ybook.yml

…icate when config files are Ansible-managed

ajs6f · 2017-09-11T20:25:02Z

Okay, I've worked through many of @jonathangreen's comments. But I'm hesitant about the idea of breaking up the tasks into many tasks, simply because the codebase is still unitary. I can't, for example, install the code of Houdini without installing the code of all the other components as well.

Do we want to break those guys apart? Of them, I understand Gemini to be the only one that is absolutely required. If we want to break them apart, I'm inclined to merge this and then do that, since that will be a bunch of other work.

…JP2) and Gemini template config

jonathangreen · 2017-09-14T12:59:38Z

roles/internal/crayfish/templates/Houdini.config.yaml.j2

+  executable: {{ houdini_executable_config.executable }}
+  formats:
+    valid:
+    {% for type in houdini_executable_config.formats.valid %}


When the template is expanded it messes up the whitespace here, which makes the YAML fail. Its because of the whitespace before the jinja2 for loop. You can use whitespace control operators in the loop to fix it, or you can just remove the whitespace like so:

houdini: # path to the convert executable executable: {{ houdini_executable_config.executable }} formats: valid: {% for type in houdini_executable_config.formats.valid %} - {{ type }} {% endfor %} default: {{ houdini_executable_config.formats.default }}

jonathangreen · 2017-09-14T13:01:53Z

roles/internal/crayfish/tasks/install.yml

+    group: www-data
+    mode: "u+rwx,g+r,o-rwx"
+
+- name: Create Gemini log


Don't think you need to do this, as long as the permissions are okay on the directory.

Right, that was left over from some tinkering.

jonathangreen · 2017-09-14T13:14:44Z

roles/internal/crayfish/tasks/install.yml

+
+- name: Create Gemini db
+  mysql_db:
+    name: gemini


I'd like to see this use the name form the variables, so you can change the DB name if you want to.

- name: Create Gemini db mysql_db: name: "{{ gemini_db_options.dbname }}" state: present

Okay, that will also have to go into the SQL file.

jonathangreen · 2017-09-14T13:16:01Z

roles/internal/crayfish/templates/gemini.sql

@@ -0,0 +1,5 @@
+CREATE TABLE IF NOT EXISTS gemini.Gemini (


I'd like to see this use the variable for the db name as well, also based on the discussion above about idempotency it probably doesn't need the IF NOT EXISTS (although it doesn't hurt to have it there)

CREATE TABLE {{ gemini_db_options.dbname }}.Gemini ( id INT UNSIGNED AUTO_INCREMENT PRIMARY KEY, drupal VARCHAR(2048) NOT NULL UNIQUE, fedora VARCHAR(2048) NOT NULL UNIQUE ) ENGINE=InnoDB;

jonathangreen · 2017-09-14T13:17:24Z

roles/internal/crayfish/defaults/main.yml

@@ -0,0 +1,73 @@
+crayfish_user: www-data


since we are in a global namespace, all the variables in this role should be prefixed with the name of the role.

crayfish_syn_token, etc

Would you include the service-specific ones here? It seems more clear to leave them alone if we expect to break them out into independent roles...

Its definitely best practice to have them all namespaces to the role, so there are no conflicts in the global namespace. I'd like to see them all prefixed with the role name.

jonathangreen · 2017-09-14T13:21:38Z

roles/internal/crayfish/tasks/install.yml

+    dest: "/tmp/gemini.sql"
+
+- name: Install Gemini db schema
+  mysql_db:


So this is idempotent, because it has the IF NOT EXISTS in the sql. However, its idiomatic in ansible to keep ansible from reporting changed on idempotent tasks, so if you are running a bunch of tasks, its easy to see what changed. If you run this task twice, it will always report changed right now.

I think I would rewrite this section this way to keep it from reporting changed:

- name: Create Gemini db mysql_db: name: "{{ gemini_db_options.dbname }}" state: present register: gemini_db_exists - name: Grab Gemini db schema template: src: "gemini.sql" dest: "/tmp/gemini.sql" when: gemini_db_exists.changed - name: Install Gemini db schema mysql_db: state: import name: all target: "/tmp/gemini.sql" when: gemini_db_exists.changed

That way we will only update the schema when the table is first created, and ansible won't report that the tasks have changed.

Ah, I didn't know about something.changed. Is that a universal thing in Ansible?

Yup! its super handy for things like this sometimes.

jonathangreen · 2017-09-14T13:33:12Z

roles/internal/crayfish/defaults/main.yml

+hypercube_executable: tesseract
+
+# Milliner default config
+milliner_log_file: /var/log/islandora/gemini.log


looks like wrong log path, probably should be:

milliner_log_file: /var/log/islandora/milliner.log

one more I just noticed.

Yup, fixing.

ajs6f · 2017-09-14T13:55:31Z

@jonathangreen I think this is ready for more review. I left crayfish_ off of the service-specific default value names, because my understanding is that we're going to break the various services out into their own roles anyway.

ajs6f · 2017-09-14T14:08:15Z

roles/internal/crayfish/tasks/install.yml

+- name: Create Gemini db
+  mysql_db:
+    name: "{{ gemini_db_options.dbname }}"
+    state: present


Got it. That's what creates the ability to check changed after this step, right?

jonathangreen

Couple missed variables.

jonathangreen · 2017-09-14T14:17:21Z

roles/internal/crayfish/tasks/install.yml

+
+- name: Create Gemini db
+  mysql_db:
+    name: "{{ gemini_db_options.dbname }}"


Needs prefix

jonathangreen · 2017-09-14T14:17:38Z

roles/internal/crayfish/templates/gemini.sql

@@ -0,0 +1,5 @@
+CREATE TABLE IF NOT EXISTS {{ gemini_db_options.dbname }}.Gemini (


needs prefix

ajs6f · 2017-09-14T14:23:27Z

Nice catch, fixed!

dannylamb reviewed Sep 7, 2017

View reviewed changes

ajs6f added 4 commits September 11, 2017 11:42

Crayfish Ansible role: first draft

f2e152e

Proper config for Crayfish role

91bf575

Correcting log locations and engaging Crayfish playbook into main pla…

b248b34

…ybook.yml

Breaking up Apache config, better file naming, adding comments to ind…

a57672f

…icate when config files are Ansible-managed

ajs6f force-pushed the 691 branch from 5df44d5 to a57672f Compare September 11, 2017 17:10

ajs6f added 2 commits September 12, 2017 10:44

Adding Lyrasis-sourced Imagemagick package (now with 50 percent more …

54c0769

…JP2) and Gemini template config

Better variable use in Crayfish config templates

7943e66

jonathangreen reviewed Sep 14, 2017

View reviewed changes

ajs6f changed the title ~~Crayfish Ansible role: first draft~~ Crayfish Ansible role Sep 14, 2017

jonathangreen reviewed Sep 14, 2017

View reviewed changes

Further improvements to config templating

e157689

ajs6f commented Sep 14, 2017

View reviewed changes

Prefixing all variables with role name

30e39a4

jonathangreen reviewed Sep 14, 2017

View reviewed changes

Correcting dbname variable name

85a4ebb

jonathangreen merged commit 9cd0d58 into master Sep 14, 2017

jonathangreen deleted the 691 branch September 14, 2017 14:50

		@@ -0,0 +1,5 @@
		CREATE TABLE IF NOT EXISTS {{ gemini_db_options.dbname }}.Gemini (

Crayfish Ansible role #9

Crayfish Ansible role #9

Conversation

ajs6f commented Aug 30, 2017 • edited Loading

dannylamb commented Sep 7, 2017

dannylamb commented Sep 7, 2017

dannylamb commented Sep 7, 2017

dannylamb commented Sep 7, 2017

dannylamb Sep 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonathangreen commented Sep 8, 2017

ajs6f commented Sep 8, 2017

ajs6f commented Sep 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajs6f commented Sep 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonathangreen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajs6f commented Sep 14, 2017

ajs6f commented Aug 30, 2017 •

edited

Loading

dannylamb Sep 7, 2017 •

edited

Loading