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

Rework procedure to enable BMC on Smart Proxies #2825

Merged
merged 3 commits into from
Mar 21, 2024

Conversation

maximiliankolb
Copy link
Contributor

  • Clarifiy enabling BMC Smart Proxy per subnet

  • Simplify enabling BMC on Smart Proxies

  • Sort options in alphabetical order

  • I am familiar with the contributing guidelines.

Please cherry-pick my commits into:

  • Foreman 3.10
  • Foreman 3.9/Katello 4.11 (planned Satellite 6.15)

@maximiliankolb
Copy link
Contributor Author

I have tested both procedures on Foreman Server and Smart Proxy Server 3.9. Does this make sense to you? Can you confirm that ipmitool has to be installed on Foreman Server/Smart Proxy Server (and not on the host)? @knoppi

Copy link
Contributor

@Lennonka Lennonka left a comment

Choose a reason for hiding this comment

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

Nice improvements! Let me get back to you regarding the SmartProxy branding.

guides/common/modules/proc_adding-a-bmc-interface.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_adding-a-bmc-interface.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_adding-a-bmc-interface.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_adding-a-bmc-interface.adoc Outdated Show resolved Hide resolved
@pr-processor pr-processor bot added Waiting on contributor Requires an action from the author and removed Not yet reviewed labels Mar 11, 2024
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Mar 12, 2024
Copy link
Contributor Author

@maximiliankolb maximiliankolb left a comment

Choose a reason for hiding this comment

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

Applied feedback; please let me know about "ipmitool" as prerequisite or part of the procedure.

guides/common/modules/proc_adding-a-bmc-interface.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_adding-a-bmc-interface.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_adding-a-bmc-interface.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_adding-a-bmc-interface.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_adding-a-bmc-interface.adoc Outdated Show resolved Hide resolved
@knoppi
Copy link

knoppi commented Mar 12, 2024

I have tested both procedures on Foreman Server and Smart Proxy Server 3.9. Does this make sense to you? Can you confirm that ipmitool has to be installed on Foreman Server/Smart Proxy Server (and not on the host)? @knoppi

Yes, this has to be installed on Foreman or Smart Proxy respectively.

@pr-processor pr-processor bot added Waiting on contributor Requires an action from the author and removed Needs re-review labels Mar 12, 2024
Copy link
Contributor

@Lennonka Lennonka left a comment

Choose a reason for hiding this comment

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

Looking good!
I'm for including installation of the package in the procedure.

Thanks for fixing the bare-metal stuff :)
I'm not sure if all the procedures are limited to bare metal, but let's keep it like this for now and I'll revisit it later during the Provisioning revamp.

* Clarifiy enabling BMC Smart Proxy per subnet
* Simplify enabling BMC on Smart Proxies
* Sort options in alphabetical order
Do not cherry-pick; only affected nightly.

See 053ba7c75621b85934a649765249b936b5dc8489 in foreman-packaging &
"packages/foreman/rubygem-rubyipmi/rubygem-rubyipmi.spec"
Copy link

Choose a reason for hiding this comment

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

Now, there is no more mention at all if and where ipmitool needs to be installed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The way I understand this: foreman-installer installs a package that has a dependency to "ipmitools"; so we don't have to document it/install it manually. See #2825 (comment)

Is this correct for Foreman nightly? @ekohl

Copy link
Member

Choose a reason for hiding this comment

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

@maximiliankolb it should be fixed for RPMs in nightly. For 3.8, 3.9 and 3.10 it'll need to wait for the next release. I also noticed it's a problem for Debian, so it may not be a bad thing to keep it in for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you; confirmed locally:

# run AlmaLinux 8.9
# add Foreman nightly repo

# dnf install rubygem-rubyipmi
Installing:
 rubygem-rubyipmi                                   noarch                               0.11.1-2.el8                                                           foreman                                  35 k
Installing dependencies:
 ipmitool                                           x86_64                               1.8.18-19.el8                                                          appstream                               394 k
 ruby        

Before, I was searching for "ipmitool" on yum.theforeman.org and couldn't find it. Well d'oh! So yes, we do not need to mention it in the docs for nightly.

So we should rebase on merge and then only cherry-pick the first two commits to "3.10" and "3.9".

Copy link
Contributor

@Lennonka Lennonka left a comment

Choose a reason for hiding this comment

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

LGTM

@maximiliankolb maximiliankolb merged commit 19b38e6 into theforeman:master Mar 21, 2024
8 checks passed
@maximiliankolb maximiliankolb deleted the extend_bmc branch March 21, 2024 11:36
@maximiliankolb
Copy link
Contributor Author

Merged to "master" and cherry-picked the first two commits:
b91ff53..a7df21b 3.10 -> 3.10
6237e73..a006969 3.9 -> 3.9

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.

5 participants