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.

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

Handle colon-delimited environmentpath in config_version.sh #64

Open
wants to merge 1 commit into
base: production
Choose a base branch
from

Conversation

mbaynton
Copy link

environment.conf passes the Puppet configuration value $environmentpath as an argument to config_version.sh. This value supports a colon-separated list of paths, but when one is used the config_version scripts currently bomb out.

This fixes them to handle colon-delimited environmentpath.

@natemccurdy
Copy link
Contributor

natemccurdy commented Mar 29, 2018

@mbaynton Can you share an example of the script bombing out?

@@ -1,12 +1,22 @@
#!/bin/bash
if [ -e $1/$2/.r10k-deploy.json ]
RONMENTPATH=$1
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a typo here.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed!

@mbaynton
Copy link
Author

After modifying the compile master's puppet.conf to include several colon-delimited environment paths, I was getting the following on agents during puppet runs:

Info: Loading facts
Info: Caching catalog for adsinternal-pupp4.msi.umn.edu
Info: Applying configuration version 'fatal: Not a git repository: '/etc/puppetlabs/code/environments:/etc/puppetlabs/code/default_environment/production/.git'
1522333495'

@abuxton
Copy link
Contributor

abuxton commented Nov 14, 2019

Apologies this as taken so long to get to @mbaynton can you rebase the pull request please and if it's still required i'll merge it in.

@mbaynton
Copy link
Author

Rebased and merged @abuxton
Full disclosure I don't currently use Puppet anymore so my ability to test is limited, but did try just invoking the merged version on some different inputs.

@natemccurdy
Copy link
Contributor

I just ran into a similar situation where a : delimited environmentpath wasn't working and used this PR for inspiration to fix fork of config_version.sh

It works 👍

Though I do have a minor style nitpick about using upper case variables for things that aren't environment variables.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants