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

remove undocumented requirement to include concat::setup in manifest #77

Merged

Conversation

jhoblitt
Copy link
Contributor

Unless the class concat::setup has been manually included into the
manifest before using the concat / concat::fragment defined types,
the puppet master will generate this warning while compiling the catalog.

Tue Oct 15 14:05:06 -0700 2013 Scope(Concat[/etc/exports]) (warning):
Could not look up qualified variable 'concat::setup::root_group'; class
concat::setup has not been evaluated

The need to include concat::setup directly into the manifest has never
been part of the documented API.

@jhoblitt
Copy link
Contributor Author

This patch passed the rspec-system tests with the default box.

$safe_name = regsubst($name, '[/\n]', '_', 'GM')
$safe_target_name = regsubst($target, '[/\n]', '_', 'GM')
$concatdir = $concat::setup::concatdir
$fragdir = "${concatdir}/${safe_target_name}"

$safe_group = $group ? {
undef => $concat::setup::root_group,
default => $safe_group,
Copy link

Choose a reason for hiding this comment

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

Maybe I'm just losing my mind here but.. wouldn't this need to be default => $group, otherwise you're saying "If $group is set, set $safe_group to $safe_group which is nil right now. right? Or am I going crazy here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

/facepalm

Unless the class `concat::setup` has been manually included into the
manifest before using the `concat` / `concat::fragment` defined types,
the puppet master will generate this warning while compiling the catalog.

    Tue Oct 15 14:05:06 -0700 2013 Scope(Concat[/etc/exports]) (warning):
    Could not look up qualified variable 'concat::setup::root_group'; class
    concat::setup has not been evaluated

The need to `include concat::setup` directly into the manifest has never
been part of the documented API.
@jhoblitt
Copy link
Contributor Author

I've fixed that error (note: the logic around setting the gid to 0 when your root seems a bit odd -- are there really that many systems out their without a 'root' group defined in /etc/group?) and added a check for the fragment file resource in the extremely minimal rspec tests I've added.

@jhoblitt
Copy link
Contributor Author

@apenney Is this changeset now acceptable or would you like further iteration?

@apenney
Copy link

apenney commented Oct 21, 2013

Looks OK to me, I meant to merge this but I'm drowning in PRs :)

apenney pushed a commit that referenced this pull request Oct 21, 2013
remove undocumented requirement to include concat::setup in manifest
@apenney apenney merged commit dfcd0ca into puppetlabs:master Oct 21, 2013
@jhoblitt
Copy link
Contributor Author

Thank you!

@jhoblitt jhoblitt mentioned this pull request Oct 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants