Skip to content

Optionally create dependency on EPEL module#12

Open
dleske wants to merge 5 commits into
bodgit:mainfrom
dleske:master
Open

Optionally create dependency on EPEL module#12
dleske wants to merge 5 commits into
bodgit:mainfrom
dleske:master

Conversation

@dleske
Copy link
Copy Markdown

@dleske dleske commented Oct 28, 2020

These changes create, for systems in the Red Hat OS family, a dependency on the EPEL module. While the EPEL repository is a prerequisite for Red Hat variants to use this module, if the repository is configured manually or otherwise already exists then the dependency can be overridden via a parameter: $assume_epel = true.

Context

We have been using this module to manage etckeeper on CentOS 7 systems without any issue. However on a new CentOS 8 build this module initializes before the EPEL repository is configured.

@bodgit
Copy link
Copy Markdown
Owner

bodgit commented Oct 28, 2020

Does adding something like:

Class['epel'] -> Class['etckeeper']

not work?

@dleske
Copy link
Copy Markdown
Author

dleske commented Oct 28, 2020

@bodgit that would work except we are using an ENC (Foreman) and have no way of describing the relationship. Puppet classes are associated to nodes and there is no control about which fires first. The two options we have so far as I understand it are to code a separate mechanism for forcing the dependency or for the module to handle the dependency, and the latter seems cleaner to me.

@dleske dleske marked this pull request as draft October 28, 2020 17:01
@dleske
Copy link
Copy Markdown
Author

dleske commented Oct 28, 2020

Took a closer look at the failing CI and the tests are failing now for a different reason than before, so I've converted this to a Draft PR while I work that out.

@coveralls
Copy link
Copy Markdown

coveralls commented Oct 28, 2020

Coverage Status

Coverage remained the same at 0.0% when pulling 29bc15f on dleske:master into 91d5581 on bodgit:master.

@dleske
Copy link
Copy Markdown
Author

dleske commented Oct 30, 2020

I've improved my fix so that it passes existing tests. I also checked on the linting errors and apparently the use of absolute addressing for classes and resources was last necessary in Puppet 3 and since this module supports 4.4.0 on it seemed reasonable to remove them. On my testing branch this then passed all the tests.

I've only some old Ruby experience and none in testing but I'll look at adding tests for the new code.

@dleske
Copy link
Copy Markdown
Author

dleske commented Oct 30, 2020

Rspec 3.10.0 was released three hours ago and is being installed by Bundler, and is breaking some tests.

@dleske
Copy link
Copy Markdown
Author

dleske commented Oct 31, 2020

@bodgit Sorry for the mess of a PR, I don't do this often.

The last test run had one failed job with the output rake aborted! and little else I could see as an obvious issue. Tests run in Travis from my fork on the last commit are green. I have reproduced the Ruby 2.1.9/Puppet 4.5.0 case locally (using this rickety silliness) and it passed. Unfortunately I don't have the option with Travis to rerun the one test case and so without creating a spurious commit I don't know how I can retest for this PR.

I spent some time looking at creating test cases for the addition but I am not experienced with rspec, there are no similar Vagrant boxes in the cloud for Centos8, and the coverage tests were not succeeding before I came along, and I need to get back to other things. Having said that I'd be willing to work with someone on this to get it going.

Given all that, I believe this to be a decent addition and it allows me to use the module in CentOS 8 as I have been in CentOS 7. Also I fixed the puppetlint warnings for absolute addressing, and then locked rspec-core to 3.9.3 as 3.10.0 appears to introduce an API incompatibility somewhere in the stack.

@dleske dleske marked this pull request as ready for review October 31, 2020 15:29
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.

3 participants