ome / infrastructure

A repository containing scripts for managing infrastructure
BSD 2-Clause "Simplified" License
20 stars 19 forks source link

www-dev: improve Jekyll redeployment #283

Closed sbesson closed 7 years ago

sbesson commented 7 years ago

This PR follows up on #281 and addresses the issue captured in https://github.com/openmicroscopy/ansible-role-jekyll-build/issues/3 at the playbook level

The main problem is that bundle is not on the default PATH failing the bundle install handler in the openmicroscopy.jekyll-build role. This PR prepends /usr/local/bin to the PATH and adds the symlink step to the jekyll tag.

With this PR included, there should be no more need for the playbook to be executed twivewhen the Jekyll source code is changed. Additionally, the dependencies can be removed from the playbook and managed solely by the Gemfile of the source code and the bundle install handler.

To test this PR:

manics commented 7 years ago

Is the default PATH on your test VMs different from a standard CentOS 7 system?

sbesson commented 7 years ago
[sbesson@infra-testpr ~]$ sudo -u root -s
[sudo] password for sbesson: 
[root@infra-testpr sbesson]# echo $PATH
/usr/bin:/bin:/usr/sbin:/sbin

is what I have on the VM we use. For comparison

docker run --rm -it[sbesson@ome-c6220-1-4 ~]$ docker run --rm -it centos
[root@82cc0c803c73 /]# echo $PATH
/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
kennethgillen commented 7 years ago

@manics - can you please show us the path from what you would call a standard CentOS 7 system for comparison with what Seb provided?

manics commented 7 years ago

ansible-role-jekyll-build fails on vagrant: molecule test --driver vagrant so it looks like PATH is different on the default CentOS-7 docker image and the vagrant image. I think this indicates this fix should be put in the jekyll role instead?

kennethgillen commented 7 years ago

@manics - I'm not sure what the additional time cost is to fix this in the upstream role rather than fix it here. We need this in and working ASAP/today.

@sbesson - do you need this tested against infra-testpr, or are you happy I merge without re-running the full workflow, considering the time cost of running through the testing? i.e. if you're happy this ran, I'll merge.