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

Add diagram of installer-based provisioning with HTTP boot #3305

Merged

Conversation

Lennonka
Copy link
Contributor

@Lennonka Lennonka commented Sep 20, 2024

What changes are you introducing?

Adding a sequence diagram of installer-based provisioning with HTTP boot. Using PlantUML to build the diagram.

Why are you introducing these changes? (Explanation, links to references, issues, etc.)

Preparation for improvements of Provisioning hosts. The diagram will be used later to explain the workflow of the provisioning method.

Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)

Checklists

  • I am okay with my commits getting squashed when you merge this PR.
  • I am familiar with the contributing guidelines.

Please cherry-pick my commits into: N/A

Copy link

github-actions bot commented Sep 20, 2024

The PR preview for 562fcca is available at theforeman-foreman-documentation-preview-pr-3305.surge.sh

The following output files are affected by this PR:

show diff

show diff as HTML

@Lennonka Lennonka added Needs tech review Requires a review from the technical perspective Needs style review Requires a review from docs style/grammar perspective labels Sep 20, 2024
@Lennonka
Copy link
Contributor Author

Lennonka commented Oct 8, 2024

Updated based on feedback from #3304

@Lennonka
Copy link
Contributor Author

Lennonka commented Oct 8, 2024

We should tweak the diagram in such way so that it doesn't contain TFTP. Foreman uses the TFTP folder to do some steps, but HTTP boot doesn't use the TFTP protocol.

@Lennonka Lennonka marked this pull request as draft October 8, 2024 15:24
@Lennonka Lennonka force-pushed the provisioning-diagram-installer-http-boot branch from ec1a4b5 to 438b864 Compare October 29, 2024 19:27
@Lennonka Lennonka force-pushed the provisioning-diagram-installer-http-boot branch from 438b864 to 2723843 Compare October 29, 2024 19:28
@Lennonka Lennonka marked this pull request as ready for review October 29, 2024 19:28
@Lennonka
Copy link
Contributor Author

I've made an attempt to tweak the diagram so that it uses Proxy instead of TFTP as a participant, but I'm not sure if it's technically correct. We'll discuss tomorrow during our session.

participant "Provisioned\nHost" as Host
participant "Foreman" as Foreman
participant "Foreman\nProxy" as Proxy
participant DHCP
Copy link
Contributor

Choose a reason for hiding this comment

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

In the #3304 PR, there is a TFTP participant. Should we add the participant HTTP here as well?

Copy link
Contributor Author

@Lennonka Lennonka Nov 4, 2024

Choose a reason for hiding this comment

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

@stejskalleos I believe that we discussed last week that we will not introduce an HTTP participant but treat the Foreman Proxy as the server that provides HTTP.

Copy link
Contributor

Choose a reason for hiding this comment

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

ACK.

guides/image-sources/prov-create-host-http.iuml Outdated Show resolved Hide resolved
@pr-processor pr-processor bot added the Waiting on contributor Requires an action from the author label Nov 4, 2024
@Lennonka Lennonka requested a review from stejskalleos November 4, 2024 21:38
Co-authored-by: Leos Stejskal <[email protected]>
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Nov 7, 2024
@Lennonka Lennonka added tech review done No issues from the technical perspective and removed Needs tech review Requires a review from the technical perspective labels Nov 7, 2024
Copy link
Contributor

@stejskalleos stejskalleos left a comment

Choose a reason for hiding this comment

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

🍏 LGTM

@Lennonka Lennonka removed the Needs style review Requires a review from docs style/grammar perspective label Nov 12, 2024
@Lennonka Lennonka added the style review done No issues from docs style/grammar perspective label Nov 12, 2024
@Lennonka Lennonka merged commit 2474a24 into theforeman:master Nov 13, 2024
8 checks passed
@Lennonka Lennonka deleted the provisioning-diagram-installer-http-boot branch November 13, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
style review done No issues from docs style/grammar perspective tech review done No issues from the technical perspective
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants