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 Discovery State and rework Initial power/pxe handling #98

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

defo89
Copy link
Contributor

@defo89 defo89 commented Aug 2, 2024

Proposed Changes

  • add Discovery state where we expect server to PXE boot and post to registry
  • Set PXE boot only once for Initial server and proceed to Discovery state. With this we do not send PXE boot all the time while metal server is booting up
  • On the Initial state we power off the server regardless, hence no need to cover server that is powered on For now, we need to power off the server manually if it's on

Fixes #97

@defo89 defo89 marked this pull request as draft August 2, 2024 12:56
@github-actions github-actions bot added size/M api-change enhancement New feature or request labels Aug 2, 2024
@defo89 defo89 force-pushed the enh/discovery-state branch 3 times, most recently from a7c0788 to 1b8f3b7 Compare August 6, 2024 13:54
@github-actions github-actions bot added size/L and removed size/M labels Aug 6, 2024
@defo89 defo89 marked this pull request as ready for review August 6, 2024 14:09
@defo89 defo89 requested review from afritzler and hardikdr August 6, 2024 14:10
@defo89 defo89 changed the title add discovery state and rework initial power handling add discovery state and rework initial power/pxe handling Aug 6, 2024
@defo89 defo89 requested a review from stefanhipfel August 7, 2024 07:38
@defo89 defo89 force-pushed the enh/discovery-state branch from 1b8f3b7 to aba2ae3 Compare August 7, 2024 12:14
@defo89 defo89 changed the title add discovery state and rework initial power/pxe handling Add Discovery State and rework Initial power/pxe handling Aug 7, 2024
@defo89 defo89 force-pushed the enh/discovery-state branch 2 times, most recently from f09eab1 to 9932ff1 Compare August 7, 2024 13:09
Copy link
Member

@hardikdr hardikdr left a comment

Choose a reason for hiding this comment

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

I suggest we have a quick offline discussion next week before we go ahead here. I have a few points I’d like to cover:

  • Could we leverage the NodeConditions like API instead of introducing a new state? This might streamline the integration without altering the state machine significantly.

  • The rationale for always using PXEBootOnce is to ensure that the Metal Machine consistently fetches the latest PXE script and Ignition during Initial state. Since the OS runs in-memory, especially with Gardenlinux, avoiding PXE could result in the machine booting from disk, which might lead to unexpected behaviors.

  • Regarding the issue with machines that are already powered on, perhaps a hint in the Endpoint object about whether Servers should undergo a Firstboot or be directly moved to Available during onboarding might offer a more holistic solution.

@afritzler
Copy link
Member

@defo89 can you also run make docs to regenerate the API reference documentation?

@defo89 defo89 force-pushed the enh/discovery-state branch from 9932ff1 to c16ccb9 Compare August 9, 2024 13:31
@defo89
Copy link
Contributor Author

defo89 commented Aug 9, 2024

@defo89 can you also run make docs to regenerate the API reference documentation?

thanks for the hint - done

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Aug 9, 2024
@afritzler afritzler merged commit d4ea815 into main Aug 12, 2024
8 checks passed
@afritzler afritzler deleted the enh/discovery-state branch August 12, 2024 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change documentation Improvements or additions to documentation enhancement New feature or request size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve new Server discovery handling
4 participants