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 PXE boot #3304

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 PXE 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 4254ddc is available at theforeman-foreman-documentation-preview-pr-3304.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
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.

Comments for the "Create host in Foreman" part.

One question: Right now, the happy path workflow is described. Do we want to also show paths when, for example, DHCP reservations fail? IMO, it would make the diagram crazy big, but it's something to think about.

guides/image-sources/prov-create-host-pxe.iuml Outdated Show resolved Hide resolved
guides/image-sources/prov-create-host-pxe.iuml Outdated Show resolved Hide resolved
guides/image-sources/prov-create-host-pxe.iuml Outdated Show resolved Hide resolved
guides/image-sources/prov-create-host-pxe.iuml Outdated Show resolved Hide resolved
guides/image-sources/prov-create-host-pxe.iuml Outdated Show resolved Hide resolved
guides/image-sources/prov-create-host-pxe.iuml Outdated Show resolved Hide resolved
guides/image-sources/prov-create-host-pxe.iuml Outdated Show resolved Hide resolved
guides/image-sources/prov-create-host-pxe.iuml 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 Sep 24, 2024
@Lennonka
Copy link
Contributor Author

Lennonka commented Sep 26, 2024

Comments for the "Create host in Foreman" part.

One question: Right now, the happy path workflow is described. Do we want to also show paths when, for example, DHCP reservations fail? IMO, it would make the diagram crazy big, but it's something to think about.

The diagram is already crazy big. I think we should go with the happy path only. We're not writing a design specification, we're trying to familiarize the user with the basic workflow.

@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author Needs re-review labels Sep 26, 2024
@pr-processor pr-processor bot added the Waiting on contributor Requires an action from the author label Oct 2, 2024
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author Needs re-review labels Oct 2, 2024
@Lennonka Lennonka force-pushed the provisioning-diagram-installer-pxe-boot branch from ed26663 to ca9e62d Compare October 8, 2024 13:27
@Lennonka
Copy link
Contributor Author

Lennonka commented Oct 8, 2024

I have enabled the display of Puppet parts because in upstream that's welcome. Also cleaned up the style of the diagram and rebased the PR.

FYI I have kept 3 notes over Host that mark the status of the host at the given times. I think that it's the appropriate use of the notes.

@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 Oct 15, 2024
@Lennonka
Copy link
Contributor Author

@asteflova Can you please review the diagram for style and give it a final ack if okay? I tried to incorporate Avital's comments from #3309 and I have provided a preview inside the docs just to check that the text in the diagram will be readable once included. The module that provides the preview will not be merged at this time.

@asteflova
Copy link
Member

@asteflova Can you please review the diagram for style and give it a final ack if okay?

Hi @Lennonka, to be honest, I wasn't going to review. I can't think of anything that I could review in terms of style in a diagram that is not being used in the docs. I believe feedback from Avital is more than enough.

@Lennonka
Copy link
Contributor Author

@asteflova Does it mean that I can set the style review done label?

@asteflova
Copy link
Member

Ah, now I see that Avital reviewed another PR, not this one.

@asteflova Does it mean that I can set the style review done label?

I was under the impression that your goal was to create the diagrams but not add them to any guides just yet. Has that changed? I can now see it included in the guide builds, including the d/s Satellite build.

@Lennonka
Copy link
Contributor Author

@asteflova Avital has asked me to provide a temporary preview so that we can check that the text in the diagram is readable. This will not be included in the merge.

@asteflova
Copy link
Member

Thanks for explaining! In that case, I'd say there is nothing to review in terms of 'style review'.

@Lennonka Lennonka added style review done No issues from docs style/grammar perspective and removed Needs style review Requires a review from docs style/grammar perspective labels Oct 21, 2024
@Lennonka Lennonka force-pushed the provisioning-diagram-installer-pxe-boot branch from 71e84e8 to ac64275 Compare October 21, 2024 18:32
@Lennonka Lennonka force-pushed the provisioning-diagram-installer-pxe-boot branch from ac64275 to 4254ddc Compare October 21, 2024 18:35
@Lennonka Lennonka merged commit 2fb106d into theforeman:master Oct 22, 2024
8 checks passed
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