CLI Environment Fixes #876

bishopbm1 · 2021-03-06T19:37:36Z

Fixes issue where the local encoding is not correct by adding the correct LC_ALL values to the environment.
Error:

warning: the VM is running with native name encoding of latin1 which may cause Elixir to malfunction as it expects utf8. Please ensure your locale is set to UTF-8 (which can be verified by running "locale" in your shell)

…tmq_cli

merging master

…ded comments.

wyardley · 2021-03-06T22:03:43Z

CHANGELOG.md

+
+ - Updated the environment to unclude LANG and LC_ALL vairables to fix issues with trying to run with the incorrect encoding
+ - Fixed issues with when managing repos the Erlang repo was not managed and the required version was not available in EPEL
+


This shouldn’t be edited in a PR - it would be generated with a release (same with metadata)

Ah i didn't realize. I reverted the changes!

wyardley · 2021-03-06T22:05:34Z

manifests/repo/rhel.pp

@@ -14,6 +15,17 @@
    gpgcheck => 1,
  }

+  # This is required because when using the latest version of rabbitmq because the latest version in EPEL
+  # for Erlang is 22.0.7 which is not compatible: https://www.rabbitmq.com/which-erlang.html
+  yumrepo { 'erlang':


Historically, the module is setup in such a way that Erlang configuration is out of the module’s scope. There have been some proposals to fix puppet-erlang, but I think any fix along these lines would have to be made in that module and then update that dependency here.

Normally I would agree except this is a version of Erlang packaged and maintained by RabbitMQ https://www.rabbitmq.com/which-erlang.html#erlang-repositories so i think that it makes sense to have it in here only when managing the repos and downloading from their package cloud.

The issue would be if users also need a competing (but compatible) erlang version on the same machine. Also, adding this here only would be inconsistent with how the module works on other OSes / distros. However, if https://github.com/voxpupuli/puppet-erlang gets released finally, it could be made a dependency or a recommended install, and the issue could be solved that way.

With the current behavior, the module is also not (by default) using the official rabbitmq packages either, tho there have been some proposals to switch that to being the default.

most of them have gotten stuck on similar issues around the Erlang dependency issues.

I can add the same code for the other OS's to make it consistent, that definitely makes sense. We could also add second variable to manage this repo if you think that may be a better option?

When using RHEL 8 with out this addition made the module unusable. So I definitely think having this in here even if it is optional would be a good add. Especially since https://github.com/voxpupuli/puppet-erlang isn't released yet.

It's pretty much always been the case with this module that the user has to satisfy the Erlang dependency themself, I think. It could potentially be documented better, but IMO, it's better if that's done through a separate module vs. a parameter in this one.

Keep in mind that the module doesn't list support for RHEL 8 yet. #842 was @nmaludy's WIP working on adding support for it, which has some good discussion about these points.

I think if we
a) make the module default to using the rabbitmq package vs. OS vendor's default as the default again (as a breaking / major change; essentially toggling the value for repos_ensure)
b) get @nmaludy's puppet-erlang actually building and released and use it in the integration tests + recommend that users use it to manage erlang installation

that can work.

That said, so far, haven't seen anyone follow through with getting all this done in a way that works, passes tests, etc.

Also, could there be issues with the newer erlang if older versions of rabbitmq were chosen?

wyardley · 2021-03-06T23:01:49Z

lib/puppet/provider/rabbitmq_cli.rb

@@ -25,7 +25,7 @@ def self.append_to_path(dir)

  def self.home_tmp_command(name, path)
    has_command name, path do
-      environment HOME: '/tmp'
+      environment HOME: '/tmp', LANG: 'en_US.UTF-8', LC_ALL: 'en_US.UTF-8'


IIRC, this should already be set for most use cases via

puppet-rabbitmq/data/common.yaml

Lines 82 to 83 in dfd7607

rabbitmq::environment_variables:

'LC_ALL': 'en_US.UTF-8'

I'm not 100% sure that that applies to commands run in the provider, but I think it should; I know there have been some previous fixes around this error.

Since environment_variables is a parameter already, maybe the right fix here is to somehow either add HOME to defaults or merge the defaults with the environment for the CLI here?

Either way, I think the environment fix here should probably be a separate PR, as it's a narrower fix that we should be able to merge quickly, and likely affects other OSes besides RHEL.

Did you confirm that both LANG and LC_ALL were necessary? LC_ALL has worked in the past and is the somewhat narrower fix.

bishopbm1 · 2021-03-08T18:18:32Z

I made the requested changes to the variables and the erlang repo.

wyardley

Can you update the PR title / commit message to clarify that it's an environment fix vs. a RHEL fix?

wyardley · 2021-03-12T03:12:45Z

metadata.json

@@ -1,6 +1,6 @@
 {
  "name": "puppet-rabbitmq",
-  "version": "11.0.1-rc0",
+  "version": "11.0.2-rc0",


this shouldn't be adjusted in here. it will get bumped with a release.

bishopbm1 · 2021-03-16T19:48:45Z

Removed metadata tag and fix the title and message.

Fixes issue where the local encoding is not correct by adding the correct LC_ALL values to the environment.

- Add support for oom_score_adj (voxpupuli#877) - CLI Environment Fixes (voxpupuli#876) - make sure the rabbitmq_version method actually returns the version (voxpupuli#874) - Use mocked facts in tests (voxpupuli#873)

- Add support for oom_score_adj (voxpupuli#877) - CLI Environment Fixes (voxpupuli#876) - make sure the rabbitmq_version method actually returns the version (voxpupuli#874) - Use mocked facts in tests (voxpupuli#873) - Enable Puppet 7 support (voxpupuli#881)

- Add support for oom_score_adj (voxpupuli#877) - CLI Environment Fixes (voxpupuli#876) - make sure the rabbitmq_version method actually returns the version (voxpupuli#874) - Use mocked facts in tests (voxpupuli#873) - Enable Puppet 7 support (voxpupuli#881) - Add auto cluster configuration support (voxpupuli#883)

- Add support for oom_score_adj (voxpupuli#877) - CLI Environment Fixes (voxpupuli#876) - make sure the rabbitmq_version method actually returns the version (voxpupuli#874) - Use mocked facts in tests (voxpupuli#873) - Enable Puppet 7 support (voxpupuli#881) - Add auto cluster configuration support (voxpupuli#883) - Compatibility with camptocamp/systemd 3.x (voxpupuli#886)

- Add support for oom_score_adj (voxpupuli#877) - CLI Environment Fixes (voxpupuli#876) - make sure the rabbitmq_version method actually returns the version (voxpupuli#874) - Use mocked facts in tests (voxpupuli#873) - Enable Puppet 7 support (voxpupuli#881) - Add auto cluster configuration support (voxpupuli#883) - Compatibility with camptocamp/systemd 3.x (voxpupuli#886) - puppetlabs/apt: Allow 8.x (voxpupuli#884) - puppet/archive: Allow 5.x (voxpupuli#884) - puppetlabs/stdlib: allow 7.x (voxpupuli#884)

bishopbm2 and others added 6 commits March 1, 2021 13:33

Added the lang and lc_all tags to the environment variables for rabbi…

46dfc45

…tmq_cli

Merge pull request #1 from voxpupuli/master

9b39d7b

merging master

Added erlang repo

200a093

Commenting out code for testing.

a484bb1

Commented out code for testing.

72cded3

Added Changelog and Updated version in metadata. Applied fixes and ad…

a2515d4

…ded comments.

wyardley reviewed Mar 6, 2021

View reviewed changes

Reverted changelog.

e8cb1d5

wyardley reviewed Mar 6, 2021

View reviewed changes

bishopbm2 added 2 commits March 8, 2021 13:11

Removed unneeded environment vaiable.

b7b8ef8

reverting erlang module add

7cf4442

wyardley reviewed Mar 12, 2021

View reviewed changes

Removed new tag from metadata

a0b068c

bishopbm1 changed the title ~~RHEL Fixes~~ CLI Environment Fixes Mar 16, 2021

wyardley approved these changes Mar 17, 2021

View reviewed changes

wyardley merged commit 63fee2c into voxpupuli:master Mar 17, 2021

wyardley added the bug Something isn't working label Mar 17, 2021

cegeka-jenkins pushed a commit to cegeka/puppet-rabbitmq that referenced this pull request Mar 26, 2021

CLI Environment Fixes (voxpupuli#876)

9f9f8da

Fixes issue where the local encoding is not correct by adding the correct LC_ALL values to the environment.

wyardley mentioned this pull request Apr 16, 2021

Release 11.1.0 #879

Merged

bishopbm1 deleted the feature/rhel-8-fix branch July 6, 2021 12:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLI Environment Fixes #876

CLI Environment Fixes #876

bishopbm1 commented Mar 6, 2021 •

edited

Loading

wyardley Mar 6, 2021

bishopbm1 Mar 6, 2021

wyardley Mar 6, 2021

bishopbm1 Mar 6, 2021

wyardley Mar 6, 2021

wyardley Mar 6, 2021 •

edited

Loading

bishopbm1 Mar 6, 2021

wyardley Mar 6, 2021

wyardley Mar 6, 2021

wyardley Mar 6, 2021

bishopbm1 commented Mar 8, 2021

wyardley left a comment

wyardley Mar 12, 2021

bishopbm1 commented Mar 16, 2021


		- Updated the environment to unclude LANG and LC_ALL vairables to fix issues with trying to run with the incorrect encoding
		- Fixed issues with when managing repos the Erlang repo was not managed and the required version was not available in EPEL

CLI Environment Fixes #876

CLI Environment Fixes #876

Conversation

bishopbm1 commented Mar 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wyardley Mar 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bishopbm1 commented Mar 8, 2021

wyardley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bishopbm1 commented Mar 16, 2021

bishopbm1 commented Mar 6, 2021 •

edited

Loading

wyardley Mar 6, 2021 •

edited

Loading