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

(PUP-4919) static compiler always sets ensure parameter to 'file' if a #4100

Conversation

tdevelioglu
Copy link

The static_compiler catalog terminus always sets ensure to 'file' on
file resources as long as the resource has a source parameter declared.
It does this even if the ensure parameter is explicitly declared.

This is inconsistent with the behavior of the normal compiler, where a
file's ensure parameter gets enforced regardless of the value of the
source parameter.

This commit skips file resources with ensure set to 'absent' in the static
compiler.

source is declared

The static_compiler catalog terminus always sets ensure to 'file' on
file resources as long as the resource has a source parameter declared.
It does this even if the ensure parameter is explicitly declared.

This is inconsistent with the behavior of the normal compiler, where a
file's ensure parameter gets enforced regardless of the value of the
source parameter.

This commit skips file resources with ensure set to 'absent' in the static
compiler.
@MikaelSmith
Copy link
Contributor

This looks appropriate. @joshcooper or @hlindberg have any background on the static compiler? This seems like a pretty obvious difference in behavior.

@tdevelioglu
Copy link
Author

The alternative would be to not overwrite the ensure parameter with metadata.ftype further along, but I can't really say what unintended other effect that could have as I haven't tested.

Skipping absent files has the added benefit of not fetching file metadata when it's not needed.

@hlindberg
Copy link
Contributor

The change looks sane, but I do not understand if this could have consequences for the recurse case. Need input from the community on this.

@MikaelSmith
Copy link
Contributor

Hi @tdevelioglu, we'd like to see some validation that this also works in the recurse case - i.e. removing a whole directory. Could you also look at adding spec tests to https://github.com/puppetlabs/puppet/blob/master/spec/unit/indirector/catalog/static_compiler_spec.rb?

@HAIL9000 HAIL9000 added Triaged and removed Triaged labels Aug 3, 2015
@HAIL9000 HAIL9000 removed the Triaged label Aug 10, 2015
HAIL9000 added a commit that referenced this pull request Aug 10, 2015
…sent_files_in_static_compiler

(PUP-4919) static compiler always sets ensure parameter to 'file' if a
@HAIL9000 HAIL9000 merged commit fb15a86 into puppetlabs:master Aug 10, 2015
@HAIL9000
Copy link
Contributor

@tdevelioglu Thanks for the contribution!

We decided not to let the issues with the static compiler specs hold this up, I will update PUP-4934 to reflect the fact that the tests (once fixed) will want a test case for this scenario.

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.

None yet

4 participants