Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #28352: Enable tuning config for foreman scenario #498

Merged
merged 1 commit into from
Apr 29, 2020

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Apr 15, 2020

No description provided.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the tests are failing.

@ehelms
Copy link
Member Author

ehelms commented Apr 15, 2020

Err, this is why it fails locally for me. @ekohl any idea? I've not encountered this before.

[ERROR 2020-04-15T10:01:19 main] connect: No such file or directory
Please make sure that the zfs-fuse daemon is running.
internal error: failed to initialize ZFS library
Error: Evaluation Error: Operator '[]' is not applicable to an Undef Value. (file: /home/ehelms/workspace/upstream/foreman-installer/tmp/share/foreman-installer/modules/puppet/manifests/params.pp, line: 39, column: 19) on node war.eagle

@ekohl
Copy link
Member

ekohl commented Apr 15, 2020

That looks like a facter issue. We probably have Facter 2 in our tests and started to use modern facts in our modules. It stems from something like $facts['networking']['fqdn'] instead of $facts['fqdn'] which Facter 2 doesn't support. Now that Facter 4 is out (which bundler can install), I think that should be a decent workaround. There's probably some gem pinned so it doesn't get considered by bundler.

@wbclark this is why I've been very hesitant in using modern facts

@ehelms
Copy link
Member Author

ehelms commented Apr 28, 2020

Passing tests now if we'd like this included in 2.1

Copy link
Contributor

@wbclark wbclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and I support including this in 2.1 since imo it's a significant improvement to the user experience to implement these tuning profiles via the installer without having to add custom hiera.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Untested, but 👍 for the general idea. Even though some parts (passenger, candlepin) aren't that important anymore, the postgresql tuning is useful.

It'd be good to add this to the manual and possibly a headline feature. You might be able to copy/move the parts from the Katello manual.

@ehelms
Copy link
Member Author

ehelms commented Apr 29, 2020

Opened here -- theforeman/theforeman.org#1576

@ehelms ehelms merged commit e3807b8 into theforeman:develop Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants