voxpupuli / puppet-keepalived

Puppet Module to manage Keepalived
https://forge.puppet.com/puppet/keepalived
Apache License 2.0
49 stars 151 forks source link

Fix track file #281

Closed normelton closed 1 year ago

normelton commented 1 year ago

Pull Request (PR) description

This PR solves two issues related to keepalive configuration:

This Pull Request (PR) fixes the following issues

Fixes #278

bastelfreak commented 1 year ago

thanks for the PR/email. Please take a look at the inline comments and the other datatypes you added. If you read the code correctly, this is only a bugfix and shouldn't breaking existing installations?

normelton commented 1 year ago

Okay, so I realized a corner-case. This patch depends on knowing the value of the keepalived_version fact. That's fine, as long as keep alive is installed. But if you're deploying keepalive with this module, then the fact doesn't have a value until the package is installed. The code here will complain that the fact value doesn't exist.

I think I need to withdraw this PR and reconsider. A few possibilities:

Thoughts?

bluthg commented 1 year ago

Duh, I had my fix (#282) lying around for ~ as long as this PR is open... maybe we should join forces? ;-)

normelton commented 1 year ago

Duh, I had my fix (#282) lying around for ~ as long as this PR is open... maybe we should join forces? ;-)

Yeah your solution is much simpler. I tried to make it automagically work, but that gets complicated. I’m closing this PR and upvoting yours.