Added chef-server-ctl version functionality #1485
Conversation
|
|
||
| # detect if running as a habitat service | ||
| if File.exist?('/hab/svc/chef-server-ctl/PID') | ||
| #code for populating version through /version endpoint |
There was a problem hiding this comment.
left over comment artifact perhaps?
| # detect if running as a habitat service | ||
| if File.exist?('/hab/svc/chef-server-ctl/PID') | ||
| #code for populating version through /version endpoint | ||
| puts "running in hab" |
There was a problem hiding this comment.
nit: I don't really think this is necessary - seems like a left over debugging statement perhaps?
| if File.exist?('/hab/svc/chef-server-ctl/PID') | ||
| #code for populating version through /version endpoint | ||
| puts "running in hab" | ||
| ident_file = File.read('../IDENT') |
There was a problem hiding this comment.
nit: I always like to use the File.expand_path('../../IDENT', __FILE__) pattern to ensure a more deterministic file path, otherwise the relative path thing can bite sometimes depending on the situation.
| end | ||
|
|
||
| puts version | ||
|
|
There was a problem hiding this comment.
nit: perhaps put an error handler after line 29 to handle any Errno::ENOENT?
rescue Errno::ENOENT => ex
puts "Error determining version! #{ex.message}"
The rescue would line up with the add_command_under_category and end
There was a problem hiding this comment.
adding error checking, I'm going to use e as the var and log instead of puts, to match how errors are handled in other parts of chef-server-ctl. For example rotate_credentials.rb
|
Having this simple yet important feature would be nice! |
| ident_file = File.read('../IDENT') | ||
| version = "chef-server #{ident_file.split('/')[2]}" | ||
| else | ||
| version = File.readlines('/opt/opscode/version-manifest.txt')[0] |
There was a problem hiding this comment.
I might do JSON.parse(File.read('/opt/opscode/version-manifest.json'))['build_version'] instead. I suspect the json file might be more future proof that relying on the first line of the txt file.
There was a problem hiding this comment.
I was on the fence as to which file to use, I'll switch over to the json.
| puts version | ||
| rescue Errno::ENOENT => e | ||
| puts "Error determining version!" | ||
| log(e.message, :error) |
There was a problem hiding this comment.
[minor note] I think this depends on the glob-order loading of ctl-commands to work. The log command in omnibus-ctl takes 1 argument; however that function is overwritten by the definitions in these two commands:
omnibus/files/private-chef-ctl-commands/backup.rb
omnibus/files/private-chef-ctl-commands/rotate_credentials.rb
However, I don't think we should block a useful command on cleaning that up.
|
Once you have all the changes, please squash/rebase and make sure you have signed off on the commit (git commit -s --amend) will fixup missing signoffs. |
thomascate
force-pushed
the
tc/add-version-command
branch
2 times, most recently
from
fa44e11 to
53d2d3d
Compare
Signed-off-by: Thomas Cate <[email protected]>
thomascate
force-pushed
the
tc/add-version-command
branch
from
53d2d3d to
eb063c0
Compare
Matches the one in Chef Server[1] [1] chef/chef-server#1485 Signed-off-by: Robb Kidd <[email protected]>
Matches the one in Chef Server[1] [1] chef/chef-server#1485 Signed-off-by: Robb Kidd <[email protected]>
Matches the one in Chef Server[1] [1] chef/chef-server#1485 Signed-off-by: Robb Kidd <[email protected]>
Added basic functionality for chef-server-ctl to report the version from the version-manifest.txt file. If it detects it's running in Habitat it will derive the version from the IDENT file in the hab package.
Signed-off-by: Thomas Cate [email protected]