Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Scottx611x/docker puppet install #1664

Merged
merged 11 commits into from
Apr 5, 2017
Merged

Scottx611x/docker puppet install #1664

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0c001e7
32 bit selenium on Linux doesn't exist for release `v.0.15.0`
scottx611x Apr 4, 2017
dde2154
I don't believe this code is necessary. Caused Docker trouble, and we…
scottx611x Apr 4, 2017
aeacd84
Require docker before starting `supervisord`
scottx611x Apr 4, 2017
d160d8c
Add `garethr-docker` dep.
scottx611x Apr 4, 2017
3b2953e
Include `docker` locally and on AWS. `docker_` to avoid naming conflicts
scottx611x Apr 4, 2017
76e96ba
Update `stankevich/python` to next minor version that supports fetchi…
scottx611x Apr 4, 2017
c23057d
Use `puppetlabs/docker_platform`
scottx611x Apr 4, 2017
3095a51
Rename `docker_` to `docker`
scottx611x Apr 4, 2017
f0bac99
Explicitly indicate global namespace
scottx611x Apr 4, 2017
fb90605
Supervisor shouldn't care about docker
scottx611x Apr 4, 2017
645365b
Reference global `app_user`
scottx611x Apr 5, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions deployment/Puppetfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
forge "https://forgeapi.puppetlabs.com"

mod 'garethr-docker', '5.3.0'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use the puppetlabs/docker_platform?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, it looks exactly the same as: garethr-docker
Actually it depends on it: https://forge.puppet.com/puppetlabs/docker_platform/dependencies

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The benefit of the puppetlabs modules is that they are provided by the Puppet team. Also, garethr-docker was last updated about a year ago and it supports only up to Ubuntu 14.04. The puppetlabs version was last updated a week ago and supports Ubuntu 16.04. Basically, it looks like it is probably going to replace garethr-docker.

mod 'puppet/nodejs', '2.1.0'
mod 'puppetlabs-apache', '1.11.0'
mod 'puppetlabs/apt', '2.3.0'
Expand Down
1 change: 1 addition & 0 deletions deployment/manifests/aws.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
include refinery
include refinery::apache2
include refinery::aws
include refinery::docker_
include refinery::neo4j
include refinery::pg
include refinery::solr
Expand Down
1 change: 1 addition & 0 deletions deployment/manifests/default.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
# See code in refinery-modules/refinery/...
include refinery
include refinery::apache2
include refinery::docker_
include refinery::neo4j
include refinery::pg
include refinery::selenium
Expand Down
5 changes: 5 additions & 0 deletions deployment/refinery-modules/refinery/manifests/docker_.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class refinery::docker_ {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not refinery::docker?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because there is already a classdocker and Puppet complains about a naming conflict

class { 'docker':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be class { '::docker': to indicate global namespace explicitly

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, should this be changed throughout our manifests? I was just following the syntax that we had used throughout

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this answer is from 2012, is it still relevant?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's the reason you got the naming error when trying to use refinery::docker on the line above. Here's the source in Puppet docs (still relevant as of Puppet v3.5 and we are using v3.4.3): https://docs.puppet.com/puppet/3.5/lang_namespaces.html#workaround

Copy link
Member

@hackdna hackdna Apr 4, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And yes, this should be changed in the rest of the manifests but in a separate branch.

docker_users => [$app_user]
}
}
4 changes: 1 addition & 3 deletions deployment/refinery-modules/refinery/manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@
# for better performance
sysctl { 'vm.swappiness': value => '10' }

# to avoid empty ident name not allowed error when using git
user { $app_user: comment => $app_user }

file { "/home/${app_user}/.ssh/config":
ensure => file,
source => "${deployment_root}/ssh-config",
Expand Down Expand Up @@ -229,6 +226,7 @@
Class["ui"],
Class["solr"],
Class["neo4j"],
Class["docker"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does supervisor require docker?

Copy link
Member Author

@scottx611x scottx611x Apr 4, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think supervisor explicitly needs docker, but i don't think we want to start up everything without it since our app will depend on docker working.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd remove that since there is no explicit dependency and docker installation will still happen by the time the Refinery manifest is applied.

Class["::rabbitmq"],
Service["memcached"],
],
Expand Down
2 changes: 1 addition & 1 deletion deployment/refinery-modules/refinery/manifests/selenium.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class refinery::selenium {
$geckodriver_version = 'v0.15.0'
$filename = "geckodriver-${geckodriver_version}-linux32.tar.gz"
$filename = "geckodriver-${geckodriver_version}-linux64.tar.gz"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because Geckodriver is not providing 32 bit builds in its newer releases anymore

$install_path = "/opt/geckodriver"

package { "firefox":}
Expand Down