ensure => 'absent' on nginx::resource::server leaves file behind

[dominics]

Affected Puppet, Ruby, OS and module versions/distributions

• Puppet: 5.0.1
• Module version: 0.7.0

How to reproduce

Create a server with something like:

nginx::resource::server { "foo":
  server_name => ['foo.example.com'];
}

Then mark it absent:

nginx::resource::server { "foo":
  ensure => 'absent',
  server_name => ['foo.example.com'];
}

What are you seeing

The locations are removed from the server, and the sites-enabled symlink is removed, but the emptied server definition still lives in sites-available. (As the locations are removed, it's kind-of useless; you couldn't "just" re-symlink it anyway.)

What behaviour did you expect instead

I'd expect the server definition file to be removed from sites-available as well.

Additional information

• I think this can be fixed just by passing the $ensure value to the concat_file definition at https://github.com/voxpupuli/puppet-nginx/blob/v0.7.0/manifests/resource/server.pp#L304 - that way the entire file will be absent, rather than just the locations within it.
• Not a super-important bug - already works functionally because the symlink is removed, which is the important bit. But just a bit annoying to have to define your own file {} to clean up after a server definition is removed.

[wyardley]

I'm curious, if you remove the server resource rather than set ensure => absent (which is how I'd normally get rid of it), with $server_purge enabled, won't that ensure that the file is removed? That's how I normally deal with it vs. doing ensure => absent on something I no longer need, so if you don't have unmanaged files that you need in those directories (or are using $confd_only), that might be an option.

That said, I agree with your assessment of what's happening and your proposed fix seems reasonable. Can you test #1104 and see if that works as you suggested?

[wyardley]

ps - PRs also welcomed if that's something you're interested in / comfortable with.

[wyardley]

Closed via #1104

[bittner]

I'm not 100% sure, but also with nginx::resource::location there seems to be an issue with ensure => absent.

The code doesn't really do a lot to remove locations, does it?

[alexjfisher]

@bittner I think you're right that

puppet-nginx/manifests/resource/location.pp

Lines 246 to 249 in 5f0ed59

	$ensure_real = $ensure ? {
	'absent' => absent,
	default => file,
	}

could be removed. But I think locations will correctly be removed from their servers if you set ensure => absent as there will no longer be any concat::fragments added to the config_file

I don't think setting ensure => absent does anything more than not declaring the location in the first place though. If you'd like to create a PR to clear any of this mess up, it'd be greatly appreciated!