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

Default package_ensure value is not compatible with current stdlib default for ensure_packages() #1522

Closed
varesa opened this issue Nov 30, 2022 · 3 comments · Fixed by #1523

Comments

@varesa
Copy link

varesa commented Nov 30, 2022

Affected Puppet, Ruby, OS and module versions/distributions

How to reproduce (e.g Puppet code you use)

include nginx
ensure_packages(['nginx'])

Note: while the above snippet alone doesn't make much sense to use, it's a reduction of a much more complex setup, where one type of call is used by one module and the other used by some other module.

What are you seeing

Error: Evaluation Error: Error while evaluating a Function Call, Duplicate declaration: Package[nginx] is already declared at (file: /home/varesa/test-modules/nginx/manifests/package/redhat.pp, line: 106); cannot redeclare (file: /home/varesa/test.pp, line: 2) (file: /home/varesa/test.pp, line: 2, column: 1) on node test-node

What behaviour did you expect instead

ensure_packages() does not fail.

Output log

Any additional information you'd like to impart

This is something that used to work, but the stdlib defaults changed somewhere between 7.1.0 and 8.5.0 from { 'ensure' => 'present' } to { 'ensure' => 'installed' }. This means that the default of $package_ensure = present does not deduplicate with ensure_packages() and causes a conflicting resource declaration.

While I believe this would also be a breaking change for the nginx-module for the very same reason, maybe in some future major release it'd be a good idea to align with the current stdlib default, causing less friction when used with ensure_packages()?

Of course, as of now, there is the workaround of either overriding the ensure-parameter, either in the nginx class declaration, or the ensure_packages call. Assuming that at least one of these is not inside some third party / vendor module.

@varesa varesa changed the title Default package_ensure value is not compatible with current stdlib default Default package_ensure value is not compatible with current stdlib default for ensure_packages() Nov 30, 2022
@kenyon
Copy link
Member

kenyon commented Dec 1, 2022

Reference: puppetlabs/puppetlabs-stdlib#1196

@kenyon
Copy link
Member

kenyon commented Dec 1, 2022

Yes, we should change the defaults to installed:

kenyon added a commit to kenyon/puppet-nginx that referenced this issue Dec 1, 2022
@kenyon
Copy link
Member

kenyon commented Dec 1, 2022

Made #1523 for this.

kenyon added a commit to kenyon/puppet-nginx that referenced this issue Dec 1, 2022
kenyon added a commit to kenyon/puppet-nginx that referenced this issue Dec 1, 2022
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 a pull request may close this issue.

2 participants