accomodate separate-git-dir, consolidate scripts

[tkishel]

An environment's .git 'directory' can be either a directory or a file containing a reference to a directory. When .git is a file, the git rev-parse HEAD command in config_version.sh will fail.

[root@puppet-master-201642 ~]# ls -al /etc/puppetlabs/code/environments/production
drwxr-x---  8 pe-puppet pe-puppet  4096 Nov 24 00:51 .
drwxr-xr-x  3 pe-puppet pe-puppet  4096 Nov 24 00:15 ..
drwxr-x---  2 pe-puppet pe-puppet  4096 Nov 24 00:15 docs
-rw-r-----  1 pe-puppet pe-puppet   131 Nov 24 00:22 environment.conf
-rw-r-----  1 pe-puppet pe-puppet   112 Nov 24 00:15 .git
-rw-r-----  1 pe-puppet pe-puppet    76 Nov 24 00:15 .gitignore
drwxr-x---  4 pe-puppet pe-puppet  4096 Nov 24 00:15 hieradata
-rw-r-----  1 pe-puppet pe-puppet 11358 Nov 24 00:15 LICENSE
drwxr-x---  2 pe-puppet pe-puppet  4096 Nov 24 00:15 manifests
drwxr-x--- 10 pe-puppet pe-puppet  4096 Nov 24 00:15 modules
-rw-r-----  1 pe-puppet pe-puppet  1038 Nov 24 00:15 Puppetfile
-rw-r-----  1 pe-puppet pe-puppet   203 Nov 24 00:51 .r10k-deploy.json
-rw-r-----  1 pe-puppet pe-puppet  8522 Nov 24 00:15 README.md
drwxr-x---  2 pe-puppet pe-puppet  4096 Nov 24 00:32 scripts
drwxr-x---  4 pe-puppet pe-puppet  4096 Nov 24 00:15 site

[root@puppet-master-201642 ~]# cat /etc/puppetlabs/code/environments/production/.git
gitdir: /opt/puppetlabs/server/data/puppetserver/filesync/client/puppet-code.git/modules/environments/production

Possibly as a result of:

https://tickets.puppetlabs.com/browse/PE-4798

This commit accommodates .git being either a directory or a file containing a reference to a directory.

---

The config_version script is implemented as multiple scripts: one bash and two ruby, spreading the code across two languages and three files. That decreases readability and increases maintenance.

[root@puppet-master-201642 ~]# ls -al /etc/puppetlabs/code/environments/production/scripts
total 12
-rwxr-x--- 1 pe-puppet pe-puppet 277 Nov 24 00:29 code_manager_config_version.rb
-rw-r----- 1 pe-puppet pe-puppet 468 Nov 24 00:29 config_version.rb
-rw-r----- 1 pe-puppet pe-puppet 381 Nov 24 00:29 config_version.sh

Like Highlander, "There could be only one!"

This commit consolidates those scripts.

[npwalker]

My thought here is that consistency is more important than having a single script. It's unclear to me that the change to a single script provides more benefit than staying consistent. I could be persuaded though.

I definitely think the changes should be broken into multiple commits though. First commit transfers that same functionality to a single commit, then add functionality in new seperate commits. Before breaking it out though, I'd be interested to hear what others think.

[aharden]

I think logic and readability is actually better with the three scripts; they each have a function. I agree that accommodating the case of .git being a file is a great feature to add.

[tkishel]

My thoughts were: all three scripts take the same parameters and return the same result: a hash or the date. Except, one: config_version (the bash script, not the identically named ruby script) also executes conditional logic. I recently had to support a customer with this, and its awkward to explain, if not defend, for the sake of consistency.

Nevertheless ...

dot_git="$1/$2/.git"
if [ -f "$dot_git" ]
then
  dot_git=`cat "$dot_git" | awk -F ': ' {'print $2'}`
fi
/usr/bin/git --version > /dev/null 2>&1 &&
/usr/bin/git --git-dir $dot_git rev-parse HEAD ||
date +%s

[glarizza]

The issue I have with this PR is that the control-repo is used pretty far and wide by customers, and this introduces an unnecessary change solely for consolidation. The scripts are usually the only thing that change about the control-repo, granted, but we kinda need these scripts to be rock solid or, in certain versions of Puppet, failure to return config_version will be a catalog compilation failure. I'm of the "if it ain't broke" persuasion here, being that it returns a commit hash.

[tkishel]

My expectation is that -all- of our code needs to be rock-solid. The bash script is broken: it doesn't return a commit hash (although it does return the timestamp) when .git is a file, and I was reluctant to add more bash when the majority of the code is ruby. So I merged the scripts, made the correction, and tested the functionality of the entire script. It's not inherently less robust as a single script: I propose the opposite.

Given the feedback, I've provided the additional bash code and will close this request.

[tkishel]

This is still evergreen ... config_version.rb called directly by environment.conf

#!/opt/puppetlabs/puppet/bin/ruby

param_environment_path = ARGV[0]
param_environment      = ARGV[1]

begin
  require 'json'
  require 'rugged'
  require 'socket'
rescue LoadError
  # The struggle is real.
  hostname = %x( /bin/hostname 2>/dev/null ).split('.').first
  compiling_master = ($?.exitstatus == 0) ? hostname : 'unknown'
  environment_version = Time.now.to_i
  puts "#{compiling_master}-#{param_environment}-#{environment_version}"
else

  # Get the hostname of the master compiling the catalog.
  # Sometimes the hostname is the FQDN, so take the first segment.
  compiling_master = Socket.gethostname.split('.').first

  if (param_environment_path && param_environment)
    # Use the parameters, if passed.
    environment_path = File.join(param_environment_path, param_environment)
  else
    # ... or use the parent directory of this script.
    environment_path = File.dirname(File.expand_path(File.dirname(__FILE__)))
  end

  environment_r10k_deploy_file = File.join(environment_path, '.r10k-deploy.json')
  environment_dot_git = File.join(environment_path, '.git')
  pe_version_file = '/opt/puppetlabs/server/pe_version'

  # Use the first 12 characters of the commit ID ...
  if (File.file?(environment_r10k_deploy_file))
    # ... in the r10k deployment file, if using Code Manager.
    environment_version = JSON.parse(File.read(environment_r10k_deploy_file))['signature'][0...11]
  elsif (File.file?(pe_version_file))
    # .. in the latest commit via rugged, if using Puppet Enterprise.
    environment_version = Rugged::Repository.discover(environment_path).head.target_id[0...11]
  elsif (File.exist?(environment_dot_git))
    #  ... in the latest commit via git, just use git.
    if (File.file?(environment_dot_git))
      # ... environment_dot_git is a file, created via --separate-git-dir.
      git_dir = File.read(environment_dot_git).split(': ')[1]
    else
      # ... environment_dot_git is a directory.
      git_dir = environment_dot_git
    end
    git_rev_parse_head = %x( /usr/bin/git --git-dir #{git_dir} rev-parse HEAD 2>/dev/null )
    environment_version = ($?.exitstatus == 0) ? git_rev_parse_head : Time.now.to_i
  else
    environment_version = Time.now.to_i
  end

  # Return the compiling master, environment name, and version.
  puts "#{compiling_master}-#{param_environment}-#{environment_version}"
end