Closes #84 - Add Tika service for ElasticSearch or AzureSearch in Moodle #95

roperto · 2018-05-04T07:26:48Z

No description provided.

hosungsmsft

I think this PR can be accepted as is, but it led me to wonder about a few things and I left comments on those, so I'd appreciate your responses to those comments.

One thing I'd like to repeat here is whether it'd be feasible to run the Tika service on the controller VM, instead of creating/adding a separate VM just for the Tika service. I understand there might be consequences to that approach, but just wondering for now...

hosungsmsft · 2018-05-04T17:55:18Z

scripts/install_moodle.sh

@@ -101,7 +103,7 @@
      pgadminlogin=$dbadminlogin
      pgadminpass=$dbadminpass
    else
-      echo "Invalid dbServerType ($dbServerType) given. Only 'mysql' or 'postgres' is allowed. Exiting"
+      echo "Invalid dbServerType ($dbServerType) given. Only 'mysql' or 'postgres' or 'mssql' is allowed. Exiting"


Thanks for fixing this!

hosungsmsft · 2018-05-04T18:04:34Z

scripts/install_moodle.sh

@@ -828,14 +830,14 @@ EOF

    if [ "$searchType" = "elastic" ]; then
        # Set up elasticsearch plugin
-        sed -i "23 a \$CFG->forced_plugin_settings = ['search_elastic' => ['hostname' => 'http://$elasticVm1IP']];" /moodle/html/moodle/config.php
+        sed -i "23 a \$CFG->forced_plugin_settings = ['search_elastic' => ['hostname' => 'http://$elasticVm1IP', 'fileindexing' => 'true', 'tikahostname' => 'http://$tikaVmIP', 'tikaport' => '9998'],];" /moodle/html/moodle/config.php


Ah... I thought Tika would be already installed on the Elastic Search cluster that's deployed when the Elastic Search is chosen, but apparently I was mistaken... Could you reconfirm that the existing Elastic Search deployment wouldn't install Tika but it requires a separate Tika installation like this? For the Elastic Search option, I'd prefer if we can piggy back on the Elastic Search VMs for Tika as well (why not add another VM, if we can add on to an existing VM...). Sorry if I'm not making much sense, but just wondering!

@hosungsmsft - the existing Elastic Search deployment does not have Tika service included; therefore, the Tika Service is also added for Elastic Search in this pull request.

And sure we can add Tika into one of Elastic Search VMs, but the whole purpose of having Elastic Search cluster (3 nodes) is to make sure if one Elastic Search node dies, the search service can still carry on. That brings some risk of having Tika into one of the Elastic Search nodes...if the node that has Tika dies, there will be no Tika service.

Also we can add Tika on controller VM. The reason to separate Tika from the controller VM is to make sure controller VM is purely serving as a management/jump node as simple as possible but no other mission. As the controller VM can be accessed outside of the private network, the setting of separating Tika from controller VM will allow the search is kept within the private network for better security practice.

Please let me know which preference you wish to go ahead and we can arrange the changes.

Makes sense. Let's keep a separate Tika VM. Thanks.

Same thing as I mentioned above for tikaTemplate's condition clause. Sorry again it didn't occur to me, but the Moodle file indexing and Tika VM installation should be switchable by customers. One idea is that if customer doesn't choose Tika for their Elastic Search or the Azure Search, pass "None" to the $tikaVmIP param and let this script not configure fileindexing if the param's value is "None". Same below for Azure Search.

hosungsmsft · 2018-05-04T18:08:46Z

scripts/install_tika.sh

+#!/bin/sh
+SERVICE_NAME=tika-server
+PATH_TO_JAR=/usr/share/java/tika-server-1.18.jar
+PID_PATH_NAME=/tmp/tika-server-pid


Nitpicking, but I think /tmp may not be a good place for a PID file... Maybe some place in /var/run or /var/tmp? There must be some systemd-stanard location for PID files, I think...

@hosungsmsft - this has been updated to create the PID file in /var/run/

hosungsmsft · 2018-05-04T18:19:24Z

nested/controller.json

+                            "access": "Allow",
+                            "destinationAddressPrefix": "9998",
+                            "destinationPortRange": "9998",
+                            "direction": "Inbound",


Just wondering why you added this inbound security rule on the controller VM's NSG? I don't see you are applying this NSG to the Tika VM's NIC, so this rule wouldn't apply to the deployed Tika VM at all, but apply only to the controller VM, which doesn't run any Tika service at all.

For the Tika VM's case, since it's inside the VNet without any public IP, and the service needs to be accessed only from the web frontend, I'm not sure if we need a security rule for that, but I may be wrong. What do you think?

Overall, this occasion actually led me to wonder whether it'd be OK to run the Tika service on the controller VM, to avoid having to add another VM just for the Tika service. What do you think?

@hosungsmsft - you are right. Tika service does not need the security rule on controller VM, I have removed the rule out from the template.

roperto · 2018-05-04T23:29:50Z

Hi @superzoelu , can you have a look at @hosungsmsft comments? Thanks

hosungsmsft

Sorry I didn't notice earlier that Tika VM is always installed when Elastic Search or Azure Search is selected. I think Tika VM installation should be optional even when Elastic Search or Azure Search is selected. Please make that change. Thanks.

hosungsmsft · 2018-05-07T18:01:25Z

azuredeploy.json

@@ -605,6 +612,27 @@
                }
            }
        },
+        {
+            "condition": "[not(bool(equals(parameters('searchType'), 'none')))]",


Sorry it didn't occur to me earlier, but I think installing a Tika VM should be switchable as well. That is, customer should be able to choose the Elastic Search or the Azure Search, but also be able to choose not to have an additional Tika VM (service). It should be doable by adding another switch (yet another...) to the main template and have it and-ed here.

@hosungsmsft - i am still working on getting tika service optional in the template, will notify you once this has been done.

hosungsmsft · 2018-05-07T18:04:24Z

scripts/install_moodle.sh

@@ -828,14 +830,14 @@ EOF

    if [ "$searchType" = "elastic" ]; then
        # Set up elasticsearch plugin
-        sed -i "23 a \$CFG->forced_plugin_settings = ['search_elastic' => ['hostname' => 'http://$elasticVm1IP']];" /moodle/html/moodle/config.php
+        sed -i "23 a \$CFG->forced_plugin_settings = ['search_elastic' => ['hostname' => 'http://$elasticVm1IP', 'fileindexing' => 'true', 'tikahostname' => 'http://$tikaVmIP', 'tikaport' => '9998'],];" /moodle/html/moodle/config.php


Same thing as I mentioned above for tikaTemplate's condition clause. Sorry again it didn't occur to me, but the Moodle file indexing and Tika VM installation should be switchable by customers. One idea is that if customer doesn't choose Tika for their Elastic Search or the Azure Search, pass "None" to the $tikaVmIP param and let this script not configure fileindexing if the param's value is "None". Same below for Azure Search.

…tion at network template

…tion of moodle, whitelist the port 9998 for tika service

…er supported

…d tika service have been selected other than 'none'

… condition can be added

… tikaVmIP variable

superzoelu · 2018-05-09T06:41:06Z

@hosungsmsft - the template now has tika as an option and the logics to exclude tika vm creation and tika settings in moodle if tikaService is set to 'none'.

The template has been tested with all possible scenarios I can think of.

hosungsmsft

Thanks much for all your hard work! Looks like you resolved the merge conflicts by rebasing on the latest Azure/Moodle:master, which is great! Will merge this PR right away. Thanks again!

hosungsmsft reviewed May 4, 2018

View reviewed changes

hosungsmsft suggested changes May 7, 2018

View reviewed changes

SorraTheOrc mentioned this pull request May 8, 2018

File indexing support for Azure Search (Apache Tika setup) #84

Closed

superzoelu added 14 commits May 9, 2018 10:51

add a dedicated tika service vm if using elastic or azure search

a5719bd

update condition line with right syntax and fix typo

c14f4f8

update tika service subnet range and include tika service subnet crea…

2ed0f1b

…tion at network template

update tika service installation script, include tika in the installa…

226d027

…tion of moodle, whitelist the port 9998 for tika service

update format for search plugin including fileindexing

535d163

fixed Duplicate key 'documentation08' (travis)

56dce1a

remove unnecessary security rule, update the tika PID location

2f20681

update where to get tika server file, as the previous link is no long…

a0c41f9

…er supported

add condition for tika service deployment only if both search type an…

aec48b1

…d tika service have been selected other than 'none'

add condition on addressPrefix for subnetTika

9df4147

move subnets of elasticsearch and tika to its own resource groups, so…

de43ecd

… condition can be added

reduce resource type segments

390ea1e

rollback prior the condition inclusion in network

9c924e7

remove if condition for tiakVmIP on the execution command

e87f54f

superzoelu force-pushed the master branch from bf89418 to 4347b05 Compare May 9, 2018 01:03

fixing the conflicts - applying fix from the git commit: 0c1665a

b0a4ad5

superzoelu force-pushed the master branch from 4347b05 to b0a4ad5 Compare May 9, 2018 01:13

add missing closing bracket in controllerconfig.json and add logic on…

e037db2

… tikaVmIP variable

hosungsmsft approved these changes May 9, 2018

View reviewed changes

hosungsmsft merged commit 713dabf into Azure:master May 9, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Closes #84 - Add Tika service for ElasticSearch or AzureSearch in Moodle #95

Closes #84 - Add Tika service for ElasticSearch or AzureSearch in Moodle #95

roperto commented May 4, 2018

hosungsmsft left a comment

hosungsmsft May 4, 2018

hosungsmsft May 4, 2018

superzoelu May 7, 2018

hosungsmsft May 7, 2018

hosungsmsft May 7, 2018

hosungsmsft May 4, 2018

superzoelu May 7, 2018

hosungsmsft May 7, 2018

hosungsmsft May 4, 2018

superzoelu May 7, 2018

hosungsmsft May 7, 2018

roperto commented May 4, 2018

hosungsmsft left a comment

hosungsmsft May 7, 2018

superzoelu May 8, 2018

hosungsmsft May 7, 2018

superzoelu commented May 9, 2018

hosungsmsft left a comment

Closes #84 - Add Tika service for ElasticSearch or AzureSearch in Moodle #95

Closes #84 - Add Tika service for ElasticSearch or AzureSearch in Moodle #95

Conversation

roperto commented May 4, 2018

hosungsmsft left a 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

roperto commented May 4, 2018

hosungsmsft left a 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

superzoelu commented May 9, 2018

hosungsmsft left a comment

Choose a reason for hiding this comment