Add possibility to define watchdog_device resource #454

dlucredativ · 2018-12-13T16:34:32Z

This pull request allows to define a watchdog_device resource for corosync.

In early corosync versions, watchdog usage had to be explicitly turned off via the "off" keyword, while in recent versions its usage needs to be explicitly turned on via the device name e.g. "/dev/watchdog". Hence an Optional[String] seems a better choice than a Boolean.

Related to #451

bastelfreak · 2018-12-15T11:13:52Z

We somehow need to fix #455 before we can merge this :(

bastelfreak · 2018-12-15T11:15:07Z

manifests/init.pp

@@ -348,6 +353,7 @@
  Optional[Enum['yes', 'no']] $clear_node_high_bit                   = undef,
  Optional[Integer] $max_messages                                    = undef,
  Boolean $test_corosync_config                                      = $corosync::params::test_corosync_config,
+  Optional[String] $watchdog_device                                  = undef,


Could you update this to something like:

Suggested change

Optional[String] $watchdog_device = undef,

Optional[Variant[Stdlib::Absolutepath, Enum['off']]] $watchdog_device = undef,

vox-pupuli-tasks · 2020-01-05T13:48:39Z

Dear @dlucredativ, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

vox-pupuli-tasks · 2020-01-05T13:52:57Z

Dear @dlucredativ, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

damluk · 2020-04-09T11:15:16Z

We somehow need to fix #455 before we can merge this :(

#455 is still open, nevertheless there have been other PRs merged, as I had to rebase. Is there a chance this one will be too, anytime soon?

Thanks.

vox-pupuli-tasks · 2020-05-01T11:30:31Z

Dear @dlucredativ, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli Github Bot. I noticed that your pull request has CI failures. Can you please have a look at the failing CI jobs?
If you need any help, you can reach out to us on our IRC channel voxpupuli on Freenode or our Slack channel voxpupuli at slack.puppet.com.
You can find my sourcecode at voxpupuli/vox-pupuli-tasks

ekohl · 2020-05-29T11:14:39Z

I've merged a major update to the tests. CentOS 7 still fails, but if you can rebase this and get everything else green this can be merged I think.

vox-pupuli-tasks · 2020-09-30T23:20:42Z

Dear @dlucredativ, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

dlucredativ · 2020-10-01T12:52:49Z

@timdeluxe do you still have a use case for this MR? It has outlived its purpose on Debian stable.

timdeluxe · 2020-10-03T15:08:03Z

@dlucredativ
I needed to search context first, i think you refer to this: corosync/corosync@b7b318b

It is released with more up to date corosync releases (if i didn't get it wrong, since 2.99.0). Well, we still operate on Ubuntu 18, were i am pretty sure that it did not change there. To confirm it i gathered the sourcecode of the Ubuntu 18 package, and confirmed it:
static char *watchdog_device = "/dev/watchdog";
We did not plan to Ubuntu 20 yet and there is no pressure yet - standard support of Ubuntu 18 goes until April 2023, so that is still a while and i think this corosync module should support it.

It would be nice to get a merge, because then i finally could switch back to the normal upstream release, instead of using a patched fork.

bastelfreak · 2020-10-03T17:33:22Z

looks good to me. @ekohl any objections against merging?

timdeluxe · 2021-03-01T19:09:58Z

@dlucredativ Can you resolve the merge conflict please so this could get finally merged?

timdeluxe · 2021-03-02T08:36:29Z

Thanks @dlucredativ, that was fast!

@ekohl Can this be merged and released now?

manifests/init.pp

spec/classes/corosync_spec.rb

templates/corosync.conf.erb

Co-authored-by: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>

dlucredativ · 2021-03-02T09:48:50Z

@ekohl do you prefer your suggestions to remain in a separate commit or should I fixup them into de92e05?

ekohl

I'm fine with a separate commit here.

ekohl · 2021-03-02T10:16:37Z

Merging now that CI has completed. Thanks!

bastelfreak reviewed Dec 15, 2018

View reviewed changes

dlucredativ force-pushed the master branch from 5be0af2 to a4a4ead Compare December 17, 2018 10:06

timdeluxe mentioned this pull request May 7, 2019

Implement option to disable watchdog device use of corosync #451

Closed

dlucredativ force-pushed the master branch from a4a4ead to 9a2e607 Compare May 7, 2019 12:34

vox-pupuli-tasks bot added the merge-conflicts label Nov 17, 2019

dhoppe removed the merge-conflicts label Jan 5, 2020

vox-pupuli-tasks bot added the merge-conflicts label Jan 5, 2020

dhoppe removed the merge-conflicts label Jan 5, 2020

vox-pupuli-tasks bot added the merge-conflicts label Jan 5, 2020

dlucredativ force-pushed the master branch from 9a2e607 to 43f0440 Compare April 9, 2020 11:11

vox-pupuli-tasks bot removed the merge-conflicts label Apr 9, 2020

vox-pupuli-tasks bot added the tests-fail label May 1, 2020

ekohl added the needs-rebase label May 29, 2020

dlucredativ force-pushed the master branch from 43f0440 to 0ffca92 Compare May 30, 2020 08:04

vox-pupuli-tasks bot added the merge-conflicts label Sep 30, 2020

dlucredativ force-pushed the master branch from 0ffca92 to 784af3d Compare October 1, 2020 12:42

bastelfreak added enhancement New feature or request and removed merge-conflicts needs-rebase tests-fail labels Oct 3, 2020

bastelfreak approved these changes Oct 3, 2020

View reviewed changes

Damian Lukowski added 2 commits March 1, 2021 20:37

Pre-realign parameters

37d0c04

Add possibility to define watchdog_device resource

de92e05

dlucredativ force-pushed the master branch from 784af3d to de92e05 Compare March 1, 2021 19:54

ekohl requested changes Mar 2, 2021

View reviewed changes

manifests/init.pp Outdated Show resolved Hide resolved

spec/classes/corosync_spec.rb Outdated Show resolved Hide resolved

templates/corosync.conf.erb Outdated Show resolved Hide resolved

Apply suggestions from code review

2be23c6

Co-authored-by: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>

ekohl approved these changes Mar 2, 2021

View reviewed changes

ekohl merged commit 9bef543 into voxpupuli:master Mar 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add possibility to define watchdog_device resource #454

Add possibility to define watchdog_device resource #454

dlucredativ commented Dec 13, 2018

bastelfreak commented Dec 15, 2018

bastelfreak Dec 15, 2018

dlucredativ Dec 17, 2018

vox-pupuli-tasks bot commented Jan 5, 2020

vox-pupuli-tasks bot commented Jan 5, 2020

damluk commented Apr 9, 2020

vox-pupuli-tasks bot commented May 1, 2020

ekohl commented May 29, 2020

vox-pupuli-tasks bot commented Sep 30, 2020

dlucredativ commented Oct 1, 2020

timdeluxe commented Oct 3, 2020

bastelfreak commented Oct 3, 2020

timdeluxe commented Mar 1, 2021

timdeluxe commented Mar 2, 2021

dlucredativ commented Mar 2, 2021

ekohl left a comment

ekohl commented Mar 2, 2021

	Optional[String] $watchdog_device = undef,
	Optional[Variant[Stdlib::Absolutepath, Enum['off']]] $watchdog_device = undef,

Add possibility to define watchdog_device resource #454

Add possibility to define watchdog_device resource #454

Conversation

dlucredativ commented Dec 13, 2018

bastelfreak commented Dec 15, 2018

bastelfreak Dec 15, 2018

Choose a reason for hiding this comment

dlucredativ Dec 17, 2018

Choose a reason for hiding this comment

vox-pupuli-tasks bot commented Jan 5, 2020

vox-pupuli-tasks bot commented Jan 5, 2020

damluk commented Apr 9, 2020

vox-pupuli-tasks bot commented May 1, 2020

ekohl commented May 29, 2020

vox-pupuli-tasks bot commented Sep 30, 2020

dlucredativ commented Oct 1, 2020

timdeluxe commented Oct 3, 2020

bastelfreak commented Oct 3, 2020

timdeluxe commented Mar 1, 2021

timdeluxe commented Mar 2, 2021

dlucredativ commented Mar 2, 2021

ekohl left a comment

Choose a reason for hiding this comment

ekohl commented Mar 2, 2021