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.

Sign up for GitHub

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

Jump to bottom

Improve Discovery in provisioning #2890

Merged
merged 31 commits into from
Jun 13, 2024
Merged

Improve Discovery in provisioning #2890

merged 31 commits into from
Jun 13, 2024

Conversation

Lennonka
Copy link
Contributor

@Lennonka Lennonka commented Mar 15, 2024 •
edited
Loading

Goals

  • Restructuring of original content
  • Re-modularization
  • Terminology improvements
  • Beginner-friendliness
  • Reduction of bloat (hopefully) and "de-spaghettization" :)
  • Renamed images and image sources

For review, you can either go commit by commit or treat it as an entirely new chapter (latter recommended).

Please cherry-pick my commits into: for Foreman 3.12

  • Foreman 3.11
  • Foreman 3.10/Katello 4.12
  • Foreman 3.9/Katello 4.11 (planned Satellite 6.15)
  • Foreman 3.8/Katello 4.10
  • Foreman 3.7/Katello 4.9 (Satellite 6.14)
  • Foreman 3.6/Katello 4.8
  • Foreman 3.5/Katello 4.7 (Satellite 6.13; orcharhino 6.6/6.7)
  • Foreman 3.4/Katello 4.6 (EL8 only)
  • Foreman 3.3/Katello 4.5 on EL7 & EL8 (Satellite 6.12 on EL8 only; orcharhino 6.4/6.5 on EL8 only)
  • We do not accept PRs for Foreman older than 3.3.

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 15, 2024 •
edited
Loading

The PR preview for 05d2952 is available at theforeman-foreman-documentation-preview-pr-2890.surge.sh

The following output files are affected by this PR:

show diff

show diff as HTML

@Lennonka Lennonka mentioned this pull request Mar 20, 2024
Merged
9 tasks
@Lennonka Lennonka force-pushed the improve-discovery branch 4 times, most recently from 2c1d3b5 to 146ba84 Compare April 16, 2024 16:55
@Lennonka Lennonka linked an issue Apr 29, 2024 that may be closed by this pull request
Integrate 5.4 Running arbitrary commands after boot #43
Open
@Lennonka Lennonka added the Needs tech review Requires a review from the technical perspective label Apr 30, 2024
@Lennonka

This comment was marked as outdated.

Sign in to view
@Lennonka Lennonka marked this pull request as ready for review April 30, 2024 20:15
@Lennonka Lennonka requested a review from ekohl April 30, 2024 20:26
@Lennonka

This comment was marked as outdated.

Sign in to view
@apinnick

This comment was marked as outdated.

Sign in to view
@Lennonka

This comment was marked as outdated.

Sign in to view
@apinnick

This comment was marked as outdated.

Sign in to view
@Lennonka

This comment was marked as outdated.

Sign in to view
@Lennonka Lennonka removed a link to an issue May 2, 2024
Integrate 5.4 Running arbitrary commands after boot #43
Open
@Lennonka Lennonka mentioned this pull request May 2, 2024
Open
@apinnick

This comment was marked as outdated.

Sign in to view
maximiliankolb
maximiliankolb reviewed May 6, 2024
View reviewed changes
guides/common/modules/proc_installing-the-discovery-service.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_discovery-in-pxe-mode.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_discovery-in-pxeless-mode.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_discovery-in-pxeless-mode.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_discovery-in-pxe-mode.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_performing-discovery-in-pxe-mode.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_performing-discovery-in-pxeless-mode.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_performing-discovery-in-pxeless-mode.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_preparing-discovery-image-for-pxe-boot.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_setting-discovery-as-the-default-PXE-boot-option.adoc Outdated Show resolved Hide resolved
apinnick
apinnick reviewed May 7, 2024
View reviewed changes
guides/common/modules/con_prerequisites-for-using-discovery.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_prerequisites-for-using-discovery.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_installing-the-discovery-service.adoc Show resolved Hide resolved
guides/common/modules/proc_installing-the-discovery-service.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_installing-the-discovery-service.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_performing-discovery-in-pxeless-mode.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_performing-discovery-in-pxeless-mode.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_performing-discovery-in-pxeless-mode.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_performing-discovery-in-pxeless-mode.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_performing-discovery-in-pxeless-mode.adoc Outdated Show resolved Hide resolved
@Lennonka
Copy link
Contributor Author

Lennonka commented Jun 5, 2024

Rebased.

bangelic
bangelic suggested changes Jun 5, 2024
View reviewed changes
guides/common/modules/proc_creating-discovery-rules.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_creating-discovery-rules.adoc Show resolved Hide resolved
guides/common/modules/proc_creating-hosts-from-discovered-hosts.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_customizing-the-discovery-pxe-boot.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_customizing-the-discovery-pxe-boot.adoc Outdated Show resolved Hide resolved
guides/common/modules/proc_performing-discovery-in-pxeless-mode.adoc Show resolved Hide resolved
guides/common/modules/proc_performing-discovery-in-pxeless-mode.adoc Show resolved Hide resolved
guides/common/modules/proc_performing-discovery-in-pxeless-mode.adoc Show resolved Hide resolved
guides/common/modules/proc_setting-discovery-as-the-default-PXE-boot-option.adoc Show resolved Hide resolved
guides/common/modules/proc_setting-discovery-as-the-default-PXE-boot-option.adoc Outdated Show resolved Hide resolved
@pr-processor pr-processor bot added Waiting on contributor Requires an action from the author and removed Needs re-review labels Jun 5, 2024
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Jun 6, 2024
@Lennonka
Copy link
Contributor Author

Lennonka commented Jun 12, 2024 •
edited
Loading

Unless there are further comments within the next 24 hours, I'm going to merge it as it is.
After that, you can add more suggestions to the issue #3041 for the next round of improvements.

maximiliankolb
maximiliankolb approved these changes Jun 13, 2024
View reviewed changes
Copy link
Contributor

@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.

Thank you Lena!

guides/common/modules/proc_building-a-custom-discovery-image.adoc
Use either a dedicated environment or copy the repositories and Kickstart file to a separate server.
====

The following procedure demonstrates the building process on {EL} 8 and uses CentOS Stream 8 repositories.
Copy link
Contributor

Choose a reason for hiding this comment

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

CentOS Stream 8 is EoL: https://blog.centos.org/2023/04/end-dates-are-coming-for-centos-stream-8-and-centos-linux-7/

Copy link
Contributor Author

@Lennonka Lennonka Jun 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

TBH I would prefer to drop this procedure altogether. I don't see why anybody would need it when the built FDI is already provided by Foreman as well as downstreams. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't know, will ask internally. Either we drop the procedure in a follow-up PR or we need to raise an issue for FDI.

guides/common/modules/proc_customizing-the-discovery-pxe-boot.adoc
The `APPEND` option contains kernel parameters.

* `pxegrub_discovery`: This snippet is included in the `PXEGrub global default` template.
However, Discovery is http://projects.theforeman.org/issues/15997[not implemented for GRUB 1.x].
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
However, Discovery is http://projects.theforeman.org/issues/15997[not implemented for GRUB 1.x].
ifndef::orcharhino[]
However, Discovery is http://projects.theforeman.org/issues/15997[not implemented for GRUB 1.x].
endif::[]

Copy link
Contributor

Choose a reason for hiding this comment

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

Grub2 is over ten years old: GNU GRUB version 2.00 was officially released on June 26, 2012.[17][18] maybe we can drop this altogether? Nobody worked on this for eight years according to the issue, so it's probably not too important?

see https://en.wikipedia.org/wiki/GNU_GRUB#Development

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 real question is how likely the user is to need to use Grub 1 instead of Grub 2. I'm not sure in what circumstances they would need it. I would even consider not documenting any templates/snippets for Grub 1 if it's unlikely.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know, but to me, this sounds unlikely that someone still uses grub1.

Copy link
Contributor

Choose a reason for hiding this comment

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

asked internally: 👍 to dropping the note for grub1.

@Lennonka Lennonka merged commit b19a420 into theforeman:master Jun 13, 2024
8 checks passed
@Lennonka Lennonka deleted the improve-discovery branch June 13, 2024 14:19
@Lennonka
Copy link
Contributor Author

Phew. Hoooray! 🥳 Thanks a lot to everyone who gave feedback and your patience!

@asteflova asteflova mentioned this pull request Sep 3, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jloeser jloeser jloeser left review comments

@maximiliankolb maximiliankolb maximiliankolb approved these changes

@stejskalleos stejskalleos stejskalleos left review comments

@bangelic bangelic bangelic requested changes

@ekohl ekohl Awaiting requested review from ekohl

@apinnick apinnick Awaiting requested review from apinnick

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Beginner-friendly provisioning guide
Development

Successfully merging this pull request may close these issues.
6 participants
@Lennonka @apinnick @jloeser @maximiliankolb @stejskalleos @bangelic