Update to stable/1.7.x #52

scollazo · 2018-01-31T01:57:58Z

This pr brings updates needed for stable/1.7.x deployment

After artefactual/deploy-pub#52 is merged, we won't be using a a shared folder for code deployment in virtualbox.

scollazo · 2018-01-31T02:12:06Z

Related to #51

scollazo · 2018-01-31T02:18:52Z

I think that changing the path to the archivematica clone in order to avoid the virtualbox shared folder, will fix #22 , and help with windows support ( #40)

hakamine · 2018-01-31T16:49:44Z

I think the main reason to put the archivematica code in a synced folder was to allow developers to easily modify the code from the host (which is a big plus for development). If the code is now deployed to /opt/archivematica and this folder is not synced this feature will be lost.

scollazo · 2018-02-06T15:50:35Z

The problem is that the npm package have problems to install/build. I think this happens because they need a lot of different files, and the vbox driver is not able to handle that in a timely fashion, and that affects all users, developers or not.

We should look for other ways to share the source between the host and the vm

sevein

I'd suggest to address the changes around synced folders separately. This would mean giving up on deploy-pub as a development workflow - is that what we want? Should we file an issue and consult the team/community beforehand? Perhaps we should have a playbooks/archivematica-development playbook?

This PR could remain as the changeset suggested to update the branch requirements of the roles and the changes around the indexless feature.

jrwdunham · 2018-02-06T17:29:45Z

Should the archivematica_src_search_enabled var be in the vars-singlenode-x.yml files with a default "yes" value? Also, as with the ansible role PR, should we document this new variable and the indexless option in the README here too?

jhsimpson · 2018-02-06T18:07:30Z

This might not be a relevant comment, but does part of the problem come from the need to build the javascript apps with node on the server Archivematica is being deployed to? In the past, the appraisal tab and transfer browser components were built manually using webpack, and so there were fewer build dependencies required during deployment.

If the node apps where built and stored in an registry or artifact repository somewhere, would that make the deploy faster and work with fewer dependencies?

sevein · 2018-02-06T20:09:18Z

How much faster do you want it to be? I can manually 1) clone the repo, 2) build appraisal-tab and 3) build transfer-browser in about a minute. I've uploaded a little asciinema so you have an idea how fast this stuff got in the last year: https://asciinema.org/a/ibpqExMCCJiQKPwBXWR5kMMJD. Building with yarn seems a tiny bit faster but it's almost unnoticeable. And if we merged transfer-browser and appraisal-tab as a single front-end project then we would cut the build time in half.

For some perspective, lxml alone takes 1 minute and 20 seconds to build. We build it four times (MCPServer/MCPClient/Dashboard/StorageService).

The problem with the Dashboard front-end build that Santi is trying to fix is only reproducible when you're using VirtualBox + vboxsf shared folders. We could arrange things in this repo so vboxsf is only used by a new playbooks/archivematica-dev environment where we also ask the developer to run npm install manually from the host.

For deployments, deb/rpm packages and our Docker image already bundles the front-end prebuilts. I think that it'd be ok if we continue letting Ansible build all the things from scratch, it makes things simpler for us.

sevein · 2018-02-06T20:19:06Z

Does this work on the vagrant environment, using a shared folder for archivematica?

No. But I think that this could be considered a minor issue iff we used vboxsf in development environments (e.g. playbooks/archivematica-dev?). We would also have to tell developers to avoid running npm install inside the virtual machine but run it from the host instead.

scollazo · 2018-02-06T20:20:00Z

Should the archivematica_src_search_enabled var be in the vars-singlenode-x.yml files with a default "yes" value? Also, as with the ansible role PR, should we document this new variable and the indexless option in the README here too?

The current docs point to the defaults/main.yml, not sure why this case requires to be added in a explicit way

scollazo · 2018-02-06T20:22:36Z

@sevein we have archivematica_src_environment_type that defaults to production, but is overrided in the vagrantfile with development

I think we should use the default value when building from vagrant, and set to "development" when needed. When this "development" mode is enabled, we won't build npm dependencies.

What do you think?

sevein · 2018-02-06T20:23:45Z

Sounds like a good plan.

sevein · 2018-02-07T16:45:27Z

playbooks/archivematica-xenial/Vagrantfile

@@ -31,7 +31,7 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
  end

  # Make the project root available to the guest VM
-  config.vm.synced_folder '.', '/vagrant'
+  config.vm.synced_folder '.', '/home/vagrant-sync'


IIRC /vagrant is a default synced folder configured by Vagrant and the original L34 wasn't really needed. I think that what this means is that with your change you would have a new /home/vagrant-sync but /vagrant would still be configured automatically. I haven't confirmed it but I have a strong feeling that this is the case.

If you wanted to disable /vagrant I think that you'd need to add the additional line:

config.vm.synced_folder ".", "/vagrant", disabled: true

In any case, I'd still recommend to submit this as a separate issue/pull-request.

scollazo · 2018-02-08T18:21:17Z

Created artefactual-labs/ansible-archivematica-src#173 to circunvent the issue with vagrant and shared folders

jrwdunham · 2018-02-11T23:52:39Z

Should the archivematica_src_search_enabled var be in the vars-singlenode-x.yml files with a default "yes" value? Also, as with the ansible role PR, should we document this new variable and the indexless option in the README here too?

The current docs point to the defaults/main.yml, not sure why this case requires to be added in a explicit way

But you're talking about https://github.com/artefactual-labs/ansible-archivematica-src/tree/qa/1.7.x, right? I mean, in this repo and this PR we only mention the archivematica_src_search_enabled var twice, once in playbooks/archivematica/singlenode.yml and the other time in playbooks/archivematica-xenial/singlenode.yml.

It seems to me, that in order to make it easy for an end user of this repo to deploy AM without indexing, we should have archivematica_src_search_enabled: "no" in the following:

playbooks/archivematica-xenial/vars-singlenode-1.7.yml
playbooks/archivematica/vars-singlenode-1.7.yml

sevein

Can you update the description of the PR? I think that the current description is not describing all the changes that you're making. I just want to make sure that when someone lands into this PR it's easier to see what changes are happening and why.

Also, the development workflow changes considerably, i.e.:

The developer needs to change the environment type variable (archivematica_src_environment_type) to development,
The developer needs to assign the /vagrant/src/ value to archivematica_src_dir,
The developer needs to run npm install manually from the host.

Should this be added to the README?

sevein · 2018-02-14T17:01:31Z

playbooks/archivematica-centos7/singlenode.yml

-    - name: "Set SELinux to Permissive"
-      command: "setenforce Permissive"      
-      become: "yes"
+   - name: "Selinux boolean 1"


Could this be more descriptive?
e.g. SELinux: allow HTTPD scripts and modules to connect to the network.

Same with the others.

sevein · 2018-02-14T17:02:30Z

playbooks/archivematica-centos7/singlenode.yml

+   - name: "Selinux seport"
+     become: "yes"
+     seport:
+       ports: 8000,8001


Do you want to quote this so it's consistent with our style of writing YAML files? It's definitely a string.

sevein · 2018-02-14T17:14:19Z

playbooks/archivematica-centos7/singlenode.yml

+       state: "present"
+     when: ansible_selinux.status == "enabled"
+
+   - name: "Enable epel repository"


s/epel/EPEL

sevein

What do you think about these notes for the README in regards to the development workflow? https://gist.github.com/sevein/74f26cf6638ff19276e2808c152faf71

scollazo · 2018-02-15T15:14:34Z

I'm testing a fix for the root cause, adding config.vm.synced_folder '.', '/vagrant',mount_options: ["uid=333", "gid=333"] to the vagrant file configuration may be the key to the npm problem.

If it works, I'll push
https://github.com/artefactual/deploy-pub/compare/dev/update-to-1.7-take3 to this pr

scollazo · 2018-02-15T15:54:42Z

Tested with trusty, and npm works.

Also, a pull request removing the workaround in the archivematica ansible role that should be merged for this to work, has been created at artefactual-labs/ansible-archivematica-src#174

sevein · 2018-02-15T16:31:30Z

playbooks/archivematica-centos7/Vagrantfile

@@ -32,7 +32,7 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
  end

  # Make the project root available to the guest VM
-  config.vm.synced_folder '.', '/vagrant'
+  config.vm.synced_folder '.', '/vagrant',mount_options: ["uid=333", "gid=333"]


Whitespace after the comma?

sevein

The description of the PR is outdated.

My comments have not been addressed yet.

scollazo · 2018-02-15T17:53:08Z

@sevein your feedback has been addresed. Regarding your gist, I would prefer to merge this "as is", and make the doc update changes as part of a fix to #53

sevein · 2018-02-15T18:05:19Z

playbooks/archivematica/vars-singlenode-1.7.yml

+archivematica_src_am_multi_venvs: "true"
+archivematica_src_dir: "/opt/archivematica"
+archivematica_src_search_enabled: "yes"
+# elasticsearch role


Cosmetic change: missing blank line before L13.

sevein · 2018-02-15T18:07:24Z

playbooks/archivematica-centos7/Vagrantfile

@@ -32,7 +32,7 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
  end

  # Make the project root available to the guest VM
-  config.vm.synced_folder '.', '/vagrant'
+  config.vm.synced_folder '.', '/vagrant',mount_options: ["uid=333","gid=333"]


Whitespace after comma.

Cosmetic: config.vm.synced_folder '.', '/vagrant', mount_options: ["uid=333", "gid=333"]

sevein · 2018-02-15T18:07:51Z

playbooks/archivematica-centos7/singlenode.yml

-    - name: "Set SELinux to Permissive"
-      command: "setenforce Permissive"      
-      become: "yes"
+   - name: "SELinux: Allow nginx connections to gunicorn"


s/gunicorn/Gunicorn

sevein · 2018-02-15T18:08:15Z

playbooks/archivematica-centos7/singlenode.yml


-    - name: "Enable epel repository"
-      yum:
+   - name: "SELinux: Allow nginx to connect to mysql "


s/mysql/MySQL

whitespace before "

sevein · 2018-02-15T18:08:42Z

playbooks/archivematica/Vagrantfile

@@ -31,7 +31,7 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
  end

  # Make the project root available to the guest VM
-  config.vm.synced_folder '.', '/vagrant'
+  config.vm.synced_folder '.', '/vagrant',mount_options: ["uid=333","gid=333"]


Missing whitespace after comma.

This fixes the problems when building npm modules

jrwdunham · 2018-02-15T20:50:04Z

@sevein I like your proposed additions to the README. I don't want to have to re-remember these things if I have to create a new ansible/vagrant dev deploy again some day.

Oops, I didn't realize this had already been merged. Nevermind.

sevein · 2018-02-15T20:51:09Z

@sevein I like your proposed additions to the README. I don't want to have to re-remember these things if I have to create a new ansible/vagrant dev deploy again some day.

I'll submit a PR soon!

scollazo added the Status: refining The issue needs additional details to ensure that requirements are clear. Waffle label. label Jan 31, 2018

scollazo added a commit to artefactual-labs/ansible-archivematica-src that referenced this pull request Jan 31, 2018

Build npm dependencies also in vagrant

e4fd3a0

After artefactual/deploy-pub#52 is merged, we won't be using a a shared folder for code deployment in virtualbox.

scollazo mentioned this pull request Jan 31, 2018

Build npm dependencies also in vagrant artefactual-labs/ansible-archivematica-src#166

Closed

scollazo added Columbia University Library CUL: phase 1 and removed CUL: phase 1 Columbia University Library labels Jan 31, 2018

scollazo mentioned this pull request Feb 6, 2018

Allow for indexless installs artefactual-labs/ansible-archivematica-src#171

Merged

sevein reviewed Feb 6, 2018

View reviewed changes

sevein reviewed Feb 7, 2018

View reviewed changes

jrwdunham assigned scollazo and jrwdunham Feb 11, 2018

jrwdunham added Columbia University Library CUL: phase 1 In Progress and removed Status: refining The issue needs additional details to ensure that requirements are clear. Waffle label. labels Feb 11, 2018

jrwdunham mentioned this pull request Feb 12, 2018

Ansible deployment for Columbia #51

Closed

sevein requested changes Feb 14, 2018

View reviewed changes

qubot force-pushed the dev/update-to-1.7 branch from 501f909 to 5654b00 Compare February 14, 2018 17:42

sevein requested changes Feb 14, 2018

View reviewed changes

qubot force-pushed the dev/update-to-1.7 branch from ed6de69 to cf642be Compare February 15, 2018 15:52

scollazo mentioned this pull request Feb 15, 2018

Build npm dependencies always artefactual-labs/ansible-archivematica-src#174

Merged

sevein requested changes Feb 15, 2018

View reviewed changes

qubot force-pushed the dev/update-to-1.7 branch 2 times, most recently from 98842f7 to 9f54fb9 Compare February 15, 2018 17:52

sevein requested changes Feb 15, 2018

View reviewed changes

qubot force-pushed the dev/update-to-1.7 branch 4 times, most recently from ac3a55e to eb77dab Compare February 15, 2018 18:31

scollazo added 6 commits February 15, 2018 19:32

Update playbooks for stable/1.7.x

bf3c4d6

Update centos base box and use official one

d8221eb

Improve SELinux support

1480be0

Add tip regarding shared folders issues

cadb91f

Change owner of shared folder to archivematica uid

60f1141

This fixes the problems when building npm modules

Fix typo with Extra Packages for Enterprise Linux

86d70f4

qubot force-pushed the dev/update-to-1.7 branch from eb77dab to 86d70f4 Compare February 15, 2018 18:33

sevein approved these changes Feb 15, 2018

View reviewed changes

qubot merged commit 86d70f4 into master Feb 15, 2018

helenst removed the In Progress label Feb 15, 2018

qubot deleted the dev/update-to-1.7 branch May 4, 2018 09:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to stable/1.7.x #52

Update to stable/1.7.x #52

scollazo commented Jan 31, 2018 •

edited

Loading

scollazo commented Jan 31, 2018 •

edited

Loading

scollazo commented Jan 31, 2018

hakamine commented Jan 31, 2018

scollazo commented Feb 6, 2018

sevein left a comment

jrwdunham commented Feb 6, 2018

jhsimpson commented Feb 6, 2018

sevein commented Feb 6, 2018 •

edited

Loading

sevein commented Feb 6, 2018 •

edited

Loading

scollazo commented Feb 6, 2018

scollazo commented Feb 6, 2018

sevein commented Feb 6, 2018

sevein Feb 7, 2018

scollazo commented Feb 8, 2018

jrwdunham commented Feb 11, 2018

sevein left a comment •

edited

Loading

sevein Feb 14, 2018

sevein Feb 14, 2018

sevein Feb 14, 2018

sevein left a comment

scollazo commented Feb 15, 2018

scollazo commented Feb 15, 2018

sevein Feb 15, 2018

sevein left a comment

scollazo commented Feb 15, 2018

sevein Feb 15, 2018

sevein Feb 15, 2018

sevein Feb 15, 2018

sevein Feb 15, 2018

sevein Feb 15, 2018

sevein Feb 15, 2018

jrwdunham commented Feb 15, 2018 •

edited

Loading

sevein commented Feb 15, 2018

Update to stable/1.7.x #52

Update to stable/1.7.x #52

Conversation

scollazo commented Jan 31, 2018 • edited Loading

scollazo commented Jan 31, 2018 • edited Loading

scollazo commented Jan 31, 2018

hakamine commented Jan 31, 2018

scollazo commented Feb 6, 2018

sevein left a comment

Choose a reason for hiding this comment

jrwdunham commented Feb 6, 2018

jhsimpson commented Feb 6, 2018

sevein commented Feb 6, 2018 • edited Loading

sevein commented Feb 6, 2018 • edited Loading

scollazo commented Feb 6, 2018

scollazo commented Feb 6, 2018

sevein commented Feb 6, 2018

Choose a reason for hiding this comment

scollazo commented Feb 8, 2018

jrwdunham commented Feb 11, 2018

sevein left a comment • 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

sevein left a comment

Choose a reason for hiding this comment

scollazo commented Feb 15, 2018

scollazo commented Feb 15, 2018

Choose a reason for hiding this comment

sevein left a comment

Choose a reason for hiding this comment

scollazo commented Feb 15, 2018

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

jrwdunham commented Feb 15, 2018 • edited Loading

sevein commented Feb 15, 2018

scollazo commented Jan 31, 2018 •

edited

Loading

scollazo commented Jan 31, 2018 •

edited

Loading

sevein commented Feb 6, 2018 •

edited

Loading

sevein commented Feb 6, 2018 •

edited

Loading

sevein left a comment •

edited

Loading

jrwdunham commented Feb 15, 2018 •

edited

Loading