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

Initial support for ostree based systems #2557

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

travier
Copy link

@travier travier commented May 30, 2024

defaults: Add method to check for ostree case

Temporary method to place all ostree specific code paths behind a single
check. May be converted to configuration option later.


bootloader/config/grub2: Handle ostree case for BLS configs

  • Do not replace but append to the existing kernel command line as it is
    correct
  • Do not fix the path to the kernel & initrd as those are also correct

system/kernel: Find kernel in ostree case


kiwi/builder/live: Handle ostree case

Do not rebuild the initrd in the ostree case and use the one from the
ostree commit instead. It is expected that it includes all the needed
modules.


kiwi/bootloader/config/grub2: Handle ostree case


See: #38

@travier
Copy link
Author

travier commented May 30, 2024

This is definitely work in progress. I'm looking for suggestions on how we should implement this properly.

kiwi/system/kernel.py Outdated Show resolved Hide resolved
@schaefi
Copy link
Collaborator

schaefi commented May 31, 2024

This is definitely work in progress. I'm looking for suggestions on how we should implement this properly.

Thanks for looking into this 👍 much appreciated. I made some comments and maybe I understand the ostree better when we meet at oSC :)

@travier
Copy link
Author

travier commented May 31, 2024

... maybe I understand the ostree better when we meet at oSC :)

Unfortunately I'm struggling to get funding for travelling to DevConf.CZ so it's unlikely that I'll be able to go to oSC this year :(

@travier travier force-pushed the main-atomic-ostree-support branch from 8d46a4b to a1f2cb0 Compare June 19, 2024 19:29
@travier
Copy link
Author

travier commented Jun 19, 2024

This is still hackish but a bit less 😅

kiwi/builder/live.py Outdated Show resolved Hide resolved
@Conan-Kudo
Copy link
Member

@travier Couple of things here:

  • Could you rebase this to fix the conflicts?
  • What are you missing to turn this into something that we can review and merge?

@travier
Copy link
Author

travier commented Sep 30, 2024

  • Could you rebase this to fix the conflicts?

Will do.

  • What are you missing to turn this into something that we can review and merge?

I need help figuring out:

@Conan-Kudo
Copy link
Member

@schaefi @travier Is there a time where we could have a dedicated meeting for this? We should try to get this hammered out asap as we want to start moving things for F42 in the spring.

@travier
Copy link
Author

travier commented Sep 30, 2024

Friday is usually a good time for me.

@schaefi
Copy link
Collaborator

schaefi commented Oct 1, 2024

@schaefi @travier Is there a time where we could have a dedicated meeting for this? We should try to get this hammered out asap as we want to start moving things for F42 in the spring.

This week and next week I won't be able to make it. I will be traveling to NUE and next week I'm in Portland for a workshop. The week after we can schedule a meeting and Friday is also a good day for me. That would mean October 18th

@Conan-Kudo
Copy link
Member

That works for me too.

@schaefi
Copy link
Collaborator

schaefi commented Oct 17, 2024

@travier @Conan-Kudo Hi, unfortunately I still don't get my voice back and sound like a mouse. I think we need to move the meeting to another day

Temporary method to place all ostree specific code paths behind a single
check. May be converted to configuration option later.
- Do not replace but append to the existing kernel command line as it is
  correct
- Do not fix the path to the kernel & initrd as those are also correct
Do not rebuild the initrd in the ostree case and use the one from the
ostree commit instead. It is expected that it includes all the needed
modules.
@schaefi
Copy link
Collaborator

schaefi commented Nov 14, 2024

@travier in preparation to the conversation I was so free to rebase your code to the upstream main and fixed the conflict. Hope this is ok ?

Added a small refactor to prevent code duplication and applied
tests for code parts that I believe are good to go. For other
code parts I added a "TODO: ostree:" marker and spared out the
code from being covered by the coverage checker. For those
remaining changes we need to have a conversation how to proceed
@@ -418,6 +420,25 @@ def setup_live_image_config(
has_graphics = True
if 'serial' in self.terminal_output or 'serial' in self.terminal_input:
has_serial = True

# Find the ostree=... parameter from the BLS config and add it to the boot options
# TODO: ostree: This code looks like it is required for any ostree based boot process and should live in its own scope e.g. an OSTree class
Copy link
Collaborator

Choose a reason for hiding this comment

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

Open for the Friday conversation

]
)
else: # pragma: nocover
# TODO: ostree: this code should be part of an OSTree class find the existing initrd in the ostree case
Copy link
Collaborator

Choose a reason for hiding this comment

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

Open for the Friday conversation

@@ -90,6 +91,29 @@ def get_kernel(
filename=kernel_file,
version=version
)

if Defaults.is_ostree(self.root_dir): # pragma: nocover
# TODO: ostree: This code looks similar to the one in builder/live and should imho exist only once in a OSTree class
Copy link
Collaborator

Choose a reason for hiding this comment

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

Open for the Friday conversation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants