Add deploy task #400
Add deploy task #400
Conversation
tasks/deploy.rb
Outdated
| if modules | ||
| list = modules.join(',') | ||
| messages << "Deploying modules #{list}." | ||
| `/opt/puppetlabs/bin/r10k deploy module #{list}` |
There was a problem hiding this comment.
can we rely on a proper PATH variable here? Otherwise we need a solution to set the path or something similar. This will fail on systems where r10k isn't installed into puppet.
There was a problem hiding this comment.
I don't have a solid answer for that, but relying on $PATH seemed to work for me, so I took out the fully qualified path.
|
Puppet tasks \o/ |
| "description": "Trigger an r10k deployment.", | ||
| "puppet_task_version": 1, | ||
| "parameters": { | ||
| "environment": { |
There was a problem hiding this comment.
It kind of seems stupid to have singular and plural forms of these arguments.
tasks/deploy.json
Outdated
| "parameters": { | ||
| "environment": { | ||
| "description": "The environment to deploy.", | ||
| "type": "String" |
There was a problem hiding this comment.
Since there are simple regexes for module and environment names we should probably use them to validate either in the metadata types or the task script itself to prevent shell injections.
Since there are two options here these should probably be optional.
something like
Optional[Pattern[/\A[a-z0-9_]+\Z/]]
There was a problem hiding this comment.
If you're going to do that, I am pretty sure you cannot start or end with an underscore, not sure about leading digits. I don't know where the official restriction description/regex is located, though.
tasks/deploy.rb
Outdated
| if environments | ||
| environments.each do |env| | ||
| messages << "Deploying environment #{env}." | ||
| `/opt/puppetlabs/bin/r10k deploy environment #{env} --puppetfile` |
There was a problem hiding this comment.
My recollection of r10k is that it will be hard to debug failures without capturing stderr. I could be wrong though
There was a problem hiding this comment.
r10k is hard to debug anyway. My thought was that if it failed, you'd attempt a manual deploy for troubleshooting.
tasks/deploy.rb
Outdated
| if environments | ||
| environments.each do |env| | ||
| messages << "Deploying environment #{env}." | ||
| `/opt/puppetlabs/bin/r10k deploy environment #{env} --puppetfile` |
There was a problem hiding this comment.
It might be worthwhile to allow an optional path parameter, for those who do things their own way.
|
👍 for the last commit message |
This is a pretty basic and not very clever task to orchestrate r10k deployments. I'd like to add more at some point, but this seems like an ok MVP. #puppethack
tasks/deploy.rb
Outdated
| exitstatus += status.exitstatus | ||
| end | ||
|
|
||
| puts({ 'messages' => messages }.to_json) |
There was a problem hiding this comment.
Should the exit status be returned in the json as well?
There was a problem hiding this comment.
hmm. There's not a precedent for it, but I do think that's probably a good idea. Maybe even a more structured output instead of just a list of messages and a status. Lemme think on it.
| 'status' => exitstatus.zero?, | ||
| 'messages' => messages, | ||
| 'warnings' => warnings, | ||
| 'errors' => errors |
There was a problem hiding this comment.
I hate rubocop and it's wrong. https://github.com/bbatsov/ruby-style-guide#no-trailing-array-commas
#puppethack This is a pretty basic and not very clever task to orchestrate r10k
deployments. I'd like to add more at some point, but this seems like an
ok MVP.