Add BMC feature to foreman-proxy

[arvind4501]

Why are you introducing these changes? (Problem description, related links)

Enable BMC feature for proxy

What are the changes introduced in this pull request?

• BMC feature for foreman-proxy
• Default bmc provider ipmi

How to test this pull request

foremanctl deploy --add-feature bmc

Steps to reproduce:

Checklist

• Tests added/updated (if applicable)
• Documentation updated (if applicable)

[arvind4501]

To test this with foremanctl we need theforeman/foreman-packaging#13417

[evgeni]

needs a rebase please

[evgeni]

While the parameter is called "default provider", the overall behavior is a bit different.

The smart proxy code takes the provider from the request, and falls back to the default if unset (honoring the "default" definition):
https://github.com/theforeman/smart-proxy/blob/a5780bc6e0132d7e91fa3074845b2a3d1cc30f01/modules/bmc/bmc_api.rb#L392-L393

But then Foreman comes (who is the only consumer of this API) and always sets the provider unless the selected provider in Foreman is "IPMI":
https://github.com/theforeman/foreman/blob/f2d892f7585c61d71f671298c8158712daace8e8/app/models/nic/bmc.rb#L27

Which basically makes the setting "used IPMI implementation" (as one can't pick one in Foreman), not "fallback provider".

To add insult to injury, everyone disagrees what a provider is. Foreman defines IPMI, Redfish and SSH, but the Proxy knows about freeipmi, ipmitool, redfish, shell and ssh 🤯

I don't think we can (or even should) fix all of that mess here and now, but maybe calling this ipmi_implementation instead of default_provider and dropping redfish would slightly reduce the user confusion?

👀 @stejskalleos

PS: I couldn't find any users of the "Shell" provider, which is basically giving power control over the machine running the smart proxy. First I thought that'd be used in Discovery, but there we have an own implementation. It was added in https://projects.theforeman.org/issues/2387 but the explanation is rather… slim

[arvind4501]

But then Foreman comes (who is the only consumer of this API) and always sets the provider unless the selected provider in Foreman is "IPMI":
https://github.com/theforeman/foreman/blob/f2d892f7585c61d71f671298c8158712daace8e8/app/models/nic/bmc.rb#L27

In this case of user selected IPMI in foreman UI, the smart proxy does not get any bmc_provider in request and falls back to Proxy::BMC::Plugin.settings.bmc_default_provider , which is ipmitool if its installed via foreman-installer.

To add insult to injury, everyone disagrees what a provider is. Foreman defines IPMI, Redfish and SSH, but the Proxy knows about freeipmi, ipmitool, redfish, shell and ssh 🤯

Yes and also with ssh provider, i am not sure if its supported as provider by foreman, our doc does not say that and even i can't see any ssh key deployed at https://github.com/theforeman/puppet-foreman_proxy/blob/master/manifests/init.pp#L401 path

[evgeni]

Should we expose the "foreman_proxy" part to the user? Or should this be --bmc-… given the feature is "bmc"?

[arvind4501]

i think it should be --bmc-*, as we have done with foreman related params such as --initial-organization and --initial-location, will update it

[evgeni]

There is a similar thing for IOP in

foremanctl/tests/conftest.py

Lines 195 to 208 in 44c09ec

	def is_iop_enabled():
	test_dir = os.path.dirname(os.path.abspath(__file__))
	foremanctl_dir = os.path.dirname(test_dir)
	params_file = os.path.join(foremanctl_dir, '.var', 'lib', 'foremanctl', 'parameters.yaml')
	if os.path.exists(params_file):
	with open(params_file, 'r') as f:
	params = yaml.safe_load(f)
	features = params.get('features', [])
	if isinstance(features, str):
	features = features.split()
	return 'iop' in features
	return False

Should we have an enabled_features helper that then can be inspected further?

[evgeni]

any reason this is an own file and not in foreman_proxy_test.py?

[arvind4501]

TBH no strong reason, foreman_proxy_test.py is a general place to test foreman proxy and its feature, the features we are asserting in foreman_proxy_test are added in CI too and works fine, but for BMC i need to implement is_bmc_enabled and foreman_proxy_test.py does not look right place for that.
as features grow for foreman-proxy i thought of having tests per feature

[evgeni]

BMC is a proxy feature, so all helpers for that are IMHO welcome in the proxy test file, but as noted above I actually think these helpers should be made more generic and moved to conftest.py

[arvind4501]

fair, moved to proxy test

[stejskalleos]

In the bmc.yaml.example are more providers:

# Available providers:
# - freeipmi / ipmitool - requires the appropriate package installed, and the rubyipmi gem
# - redfish - requires the redfish_client gem
# - shell - for local reboot control (requires sudo access to /sbin/shutdown for the proxy user)
# - ssh - limited remote control (status, reboot, turn off)
:bmc_default_provider: freeipmi