Skip to content
This repository has been archived by the owner on Sep 13, 2018. It is now read-only.

add INI file for panels plugin #67

Closed
wants to merge 1 commit into from
Closed

add INI file for panels plugin #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions manifests/plugin_panels.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# panels-specific functionality.
# TODO: probably should move this file into its own puppet module
# TODO: need to make config handling more generic, this should work with ANY plugin, and not be panels-specific

class uber::plugin_panels (
$git_repo = "https://github.com/magfest/panels",
$git_branch = "master",
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this interact with the uber::plugins::extra_plugins setting in e.g. event-prime.yaml? Like, what would happen if we had two different values for the Git repos?

Since this just defines a puppet management class, I assume this is NOT required for all deploys which use the ubersystem-puppet repo. Like, an event could install a set of plugins that doesn't include this one? Or is this now required for every deploy using ubersystem-puppet? (I wouldn't have a real problem with that since ubersystem-puppet is pretty much just MAGFest at this point and Docker seems to be the way other events are deploying, but I figured it was worth asking.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK so the answer to this is yes, slightly by design, but easily changable Two main pieces:

  1. our puppet setup isn't elegant enough to handle INI files for other plugins without creating an explicit class for them. this is lame and something that should be fixed as discusssed in Add real support for plugin-specific INI settings #53

  2. there might be situations where a plugin does need to do something more complex than just INI stuff, and does legitimately need its own puppet class like plugin_panels and plugin_hotel which we have now.

how we're handling what puppet classes get included for a "default" install is the following puppet class which contains the definition of a default set of classes which compromise a reasonable starting point for uber deploys.

This starts here in the event-prime.yaml file which is a magfest-specific .yaml that lives in magfest-specific production-config repo:

classes:
- roles::uber_server

roles::uber_server is a class that our puppet module provides which has all the manifests needed to setup a reasonable default uber server. If an event wants to do something slightly different they can include their own specialization of this named hypothetical things like: roles::magfest_prime_uber_server or roles::rams_frontend_uber_server or roles::magfest_replicated_uber_db_only.

Our "roles::uber_server" has the following line in it:

include uber::profile_rams_full_stack

uber::profile_rams_full_stack is a class which defines a reasonably default install of uber, and whose paramaters can be overriden by the .yaml files. it looks like this:

class uber::profile_rams_full_stack (

 ) {
    require ::uber::permissions
    require ::uber::app
    require ::uber::plugin_barcode
   require ::uber::plugin_panels
    require ::uber::db
    require ::uber::nginx
    require ::uber::daemon
   require ::uber::firewall
 }

The idea here being that this is a reasonable default. As we start moving functionality out of core (like panels, barcodes, etc), we could use this to define a reasonable default set of plugins and nginx, db, backend, firewall, info for how to run as a daemon, etc.

If an event wanted to do something different (like, say, run JUST sideboard+plugins on one server, db on another, and nginx on another), they could define stuff like profile_frontend, profile_db, profile_nginx. The config for all 3 of these boxes can still be centrally managed by puppet like we do now, but the various parts would go in different places.

So the answer is: yes, we include panels plugin by default, but someone could also super-easily write their own profile in their own event-specific puppet module that decides not to include it.

From a design standpoint, there's an open question of "which set of plugins should be included in every uber install". From a practical implementation standpoint right now, there's a little more work (not much) to make this a little less hardcoded.

I figured since Magfest is the only one using it though, and it's really easy to point a deploy at a fork of the ubersystem-puppet module by modifying your fabric_settings.ini, this isn't really hurting anything, and anyone can change the default profile in their own fork with zero effort.

How all this works with docker is an open question, this is kind of orthagonal to what docker does and I'd expect a dockerized version of a magfest deploy to probably look similar, but instead of copying uber files onto the server, it's instead copying docker containers around. The config stuff like INI's etc would probably be managed identically to how we're doing it today. Buuuuuut, not actually sure about that yet, just a first guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One other thing: It's also possible to remove panels_plugin from profile_rams_full_stack and include it in a magfest-specific file like event-prime.yaml, by doing the following:

---
classes:
- roles::uber_server
- uber::panels_plugin

The question then really becomes "should panels be a default plugin for all RAMS installations"? If yes, then it's ok to put it in profile_rams_full_stack. If no, then it would be better to put it as I have above in a magfest-specific .yaml file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And, also, we don't have to make this decision if #53 is a thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

That all sounds great, so this all gets a +1 from me. Thanks for such a thorough explanation.

$hide_schedule = true,
) {
uber::repo { "${uber::plugins_dir}/panels":
source => $git_repo,
revision => $git_branch,
require => File["${uber::plugins_dir}"],
}

file { "${uber::plugins_dir}/panels/development.ini":
ensure => present,
mode => 660,
content => template('uber/panels-development.ini.erb'),
require => [ Uber::Repo["${uber::plugins_dir}/panels"] ],
}
}
1 change: 1 addition & 0 deletions manifests/profile_rams_full_stack.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
require ::uber::permissions
require ::uber::app
require ::uber::plugin_barcode
require ::uber::plugin_panels
require ::uber::db
require ::uber::nginx
require ::uber::daemon
Expand Down
4 changes: 4 additions & 0 deletions templates/panels-development.ini.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# THIS FILE IS GENERATED BY PUPPET
# DO NOT EDIT DIRECTLY, INSTEAD EDIT THE .erb template

hide_schedule = "<%= @hide_schedule %>"