[WIP] Add CentOS/RHEL 8 Support

[nmaludy]

Pull Request (PR) description

This PR adds support for installing RabbitMQ on CentOS 8 / RHEL 8.
Also added back into the README information to the end-user about the need to use an external erlang module on RHEL systems.

This Pull Request (PR) fixes the following issues

Fixes #816

Closes #826

[ekohl]

This looks to be reverting the modulesync 2.12.0. Could you fix that?

[nmaludy]

@ekohl Maybe i'm doing something wrong, but i did the following:

git clone https://github.com/voxpupuli/modulesync_config
export GIT_TAG=$(git tag --list | sort -V | tail -n1)
echo $GIT_TAG
# this printed 2.12.0
git checkout $GIT_TAG
bundle install

Get my module fork:

mkdir -p modules/nmaludy
cd modules/nmaludy
git clone [email protected]:nmaludy/puppet-rabbitmq.git
cd ../..

Run modulesync:

bundle exec msync update -n nmaludy -f puppet-rabbitmq --noop

Is this correct?

[nmaludy]

@ekohl FYI looks like my modulesync changes are consistent with the 2.12.0 tag https://github.com/voxpupuli/modulesync_config/blob/2.12.0/config_defaults.yml#L7-L16

But they are different on the master branch: https://github.com/voxpupuli/modulesync_config/blob/master/config_defaults.yml#L7-L16

I can run modulesync again against master if you prefer?

[wyardley]

@nmaludy yeah, the modulesync PRs should be in a separate commit, at least. I can run one against the repo and then you can rebase / squash your branch

[wyardley]

Hrm, actually, yeah, since it is already synced against 2.12.0, should just leave it out.
If you need something that's more recent than that for this PR to work, we may need to have someone cut a newer modulesync tag.

[nmaludy]

@wyardley i was using modulesync to add in the new beaker acceptance sets, i can just do it manually for now since these modulesync changes are not desired, standby for an updated commit

[ekohl]

Looks like this module is actually on master. We've been making changes to simplify things using voxpupuli-acceptance but looks like we didn't do that via a real tag.

@nmaludy for now, I think you can use master to modulesync (or manual).

@bastelfreak should we do a 2.13.0 where we introduce voxpupuli-acceptance? Then later we can do the big breaking one wiht whitespace.

[wyardley]

I can make that... just thought generally we're only supposed to sync off a tag.

[wyardley]

@nmaludy can you rebase and then squash down into logical commits?

[nmaludy]

Looks like acceptance tests are hanging when trying to start the rabbitmq-server service inside the docker container. This worked perfectly in a CentOS 8 Vagrant VM, so i'm guessing the failure/hang is a docker-ism. Going to look further now.

[nmaludy]

So, this indeed failed for me when using Beaker on my local box. I tried to reproduce with a standard docker setup using Vagrant and this setup worked correctly:

docker_registry = ENV['DOCKER_REGISTRY']
docker_image = ENV['DOCKER_IMAGE'] ? ENV['DOCKER_IMAGE'] : 'centos:8'
docker_image_full = if docker_registry
                      "#{docker_registry}/#{docker_image}"
                    else
                      docker_image
                    end

dockerfile_path = ENV['DOCKERFILE_PATH'] ? ENV['DOCKERFILE_PATH'] : '.'
dockerfile = ENV['DOCKERFILE'] ? ENV['DOCKERFILE'] : 'Dockerfile'
cmd = ['/sbin/init']
volumes = ['/sys/fs/cgroup:/sys/fs/cgroup:ro']

Vagrant.configure('2') do |config|
  config.vm.synced_folder '.', '/ci/'

  config.vm.provider 'docker' do |d|
    if docker_image
      # pull container from registry
      d.image = docker_image_full
    else
      # build container from our Dockerfile
      d.build_dir = dockerfile_path
      d.dockerfile = dockerfile
    end
    # tell the container to stay running, even when /bin/bash exits (default command)
    d.create_args = ['-t', '--privileged']
    # mount volumes needed for systemd
    d.volumes = volumes
    # run as init
    d.cmd = cmd
  end

  config.vm.provision 'docker' do |d|
    d.pull_images docker_image_full
    d.run 'default'
  end
end

Running the commands:

vagrant up
docker ps -a
docker exec -it puppet-rabbitmq_default_1593453936 bash
curl -sSL https://raw.githubusercontent.com/nmaludy/puppet-install-shell/master/install_puppet_6_agent.sh | bash -s
yum -y install git
echo "puppetlabs/stdlib puppetlabs/apt puppet/archive garethr/erlang camptocamp/systemd puppetlabs/yumrepo_core" | xargs -n 1 /opt/puppetlabs/puppet/bin/puppet module install
cp -r /ci /etc/puppetlabs/code/environments/production/modules/rabbitmq
# copy code
cat << "EOF" > ./node.pp
class { 'rabbitmq': }
if $facts['os']['family'] == 'RedHat' {
  class { 'erlang': epel_enable => true}
  Class['erlang'] -> Class['rabbitmq']
}
EOF
/opt/puppetlabs/puppet/bin/puppet apply node.pp

Trying to figure out what is different from the container i created an the one that Beaker created? Maybe it isn't running in privileged mode, or doesn't have the right volumes mounted?

[nmaludy]

Think i found the problem... my container mounts the volume /sys/fs/cgroup:/sys/fs/cgroup:ro but the beaker container does not.

I was able to find this by doing:

$ docker ps -a
CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS              PORTS                  NAMES
81b21fb99204        89c3755d835c        "/sbin/init"        3 minutes ago       Up 2 minutes        0.0.0.0:4088->22/tcp   sweet_lehmann
888c591e1b22        centos:8            "/sbin/init"        20 minutes ago      Up 20 minutes                              puppet-rabbitmq_default_1593455879

$ docker inspect --format='{{.Mounts}}' 888c591e1b22
[{bind  /sys/fs/cgroup /sys/fs/cgroup  ro false rprivate}]

$ docker inspect --format='{{.Mounts}}' 81b21fb99204
[]

[nmaludy]

I've figured out how to add this volume mount to the nodeset in beaker and modified the travis config to match. This test round should get further than last time (at least the service should start)!

[nmaludy]

Volume mounting worked in Travis, got passed the issue with systemd service restarts.

Ran into an issue with plugin idempotency and found a bug where we were only listing Explicitly enabled plugins. Since our test installs rabbitmq_plugin { [ 'rabbitmq_federation_management', 'rabbitmq_federation' ]: and rabbitmq_federation is a dependency of rabbitmq_federation_management it was being implicitly enabled. rabbitmq-plugins CLI doesn't list out implicitly enabled plugins unless you pass another flag. So, without this flag Puppet kept on thinking that the rabbitmq_federation plugin was not installed. I've fixed the bug by passing in the -e flag to list out implicitly enabled plugins within our instances function of the provider.

[wyardley]

I've fixed the bug by passing in the -e flag to list out implicitly enabled plugins within our instances function of the provider.

Thanks for digging into that. That one for sure should be a separate issue / PR, so that it's visible in the changelog etc, if you wouldn't mind cherry-picking that change separately (and maybe filing an issue for it).

Why wouldn't that issue come up with the integration tests for other versions?

[nmaludy]

@wyardley looks like this was introduced in RabbitMQ 3.6.7 rabbitmq/rabbitmq-cli@e087171

I'll add a version switch in the code, it looks like we'll want to pass in -E for older version then just -e for newer versions because -e displays all implicitly and explicitly enabled plugins where -E restricts it to just explicitly enabled plugins (even if both are passed).

[wyardley]

I see why you're doing this here, but I think probably we shouldn't use non-default params for one specific OS version for this.

If you want to switch the default to repos_ensure as a major (breaking) change, I'm onboard with that, though it may be a bit of an involved process.

[wyardley]

@wyardley looks like this was introduced in RabbitMQ 3.6.7 rabbitmq/rabbitmq-cli@e087171

Got it. yeah, the integration tests just run with the default version. I'm Ok with a couple of options:

1. Integration test with both repos_ensure: true and repos_ensure: false for all supported OS versions
2. Assuming there is a vendor package, keep repos_ensure: false (default) for this OS version
3. Make a separate (major / breaking) PR to change the default behavior back to repos_ensure: true for everyone (and only integration test that scenario).

#3 probably makes the most sense, but is also potentially a lot of work.

I take responsibility for the current behavior, introduced back in
#493
see my earlier comments here about this topic.

[wyardley]

I'll add a version switch in the code

If someone ever has the time / inclination, I think hitting the APIs vs. using the CLI would also be better. Someone started down that path, but don't think anything is happening in that regard these days.

[wyardley]

@wyardley I like your suggestions, let me get the build passing then we can figure out how we want to cherry pick and split it up. Does that sound ok?

Absolutely!

[ekohl]

Other than https://github.com/voxpupuli/puppet-rabbitmq/pull/842/files#r447265521 this looks pretty good. Would you mind rebasing on master and squashing the commits?

[nmaludy]

Doing a lot more digging and was having issues trying to do consistent realtime version checks when listing plugins. Checking with older versions of RabbitMQ and the oldest one i could get my hands on was 3.5.1 and rabbitmq-plugin in that version supports the -e option for listing implicitly enabled plugins. I'm not sure how this code has been working so long without running into this problem.

Fighting with this all day and i think the best option is to change -E to -e for listing plugins. Proof is in the pudding, switching from -E to -e for listing plugins allowed all acceptance tests to pass!

[nmaludy]

@wyardley ok, i'm ready to talk about splitting this up into different PRs now!

[wyardley]

Proof is in the pudding, switching from -E to -e for listing plugins allowed all acceptance tests to pass!

if they pass across all supported versions, I think we should be good. You could also run the beaker set for the default RMQ version and manually validate that it works there. As long as it works with our min current supported version, should be Ok.

As far as breaking up, here's what I propose:

• issue / PR for rabbitmq_plugin provider arguments (we could maybe cut a release for this before the breaking changes too). This would seem to be a bug, so we'll want to categorize it as such.
• PR flagged as "backwards-incompatible" / breaking (and "enhancement") to change repos_ensure default to true, with any docs updates needed in README or generated docs (watch out for puppet-strings, though, as you don't want to update the generated docs, just the sources) and any adjustments needed to make current matrix pass for this.
• new enhancement PR (or repurpose this one) adding CentOS / RHEL 8 support

After all that, I can make a release PR with the updated docs, changelog, etc., targeting a major version.

[nmaludy]

rabbitmq_plugin PR is here: #844

[nmaludy]

repos_ensure => true PR is here: #845

[nmaludy]

@wyardley I've squashed this PR down and removed the extra changes caused by repos_ensure. So, this is currently blocked by:

• Fix rabbitmq_plugin to correctly detect implicitly enabled plugins #844
• Change repos_ensure default from false to true #845

Once we merge those two PRs, this is ready to go after a quick rebase.

[vox-pupuli-tasks]

Dear @nmaludy, 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

[vox-pupuli-tasks]

Dear @nmaludy, 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