Support bootc #830

bshephar · 2024-11-27T05:00:35Z

This PR adds a number of changes to roles in order to facilitate the use of image mode RHEL.

openshift-ci · 2024-11-27T05:00:39Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

bshephar · 2024-11-27T05:01:57Z

roles/edpm_kernel/defaults/main.yml

@@ -36,3 +36,4 @@ edpm_nova_compute_config_dir: /var/lib/config-data/ansible-generated/nova_libvir

 # KSM control
 edpm_kernel_enable_ksm: false
+edpm_use_bootc: false


We probably need a better way to implement this globally. But at least for testing purposes, this is what I've used to get something that deploys.

This is why I added the edpm_bootc role in #813 so that we had a way to do it consistently across anywhere that needs it.

Also, I created a new bootc branch for edpm-ansible: https://github.com/openstack-k8s-operators/edpm-ansible/tree/bootc

Can you propose this PR to the bootc branch instead?

I'll be reverting #813 from main until we are ready to merge all bootc support into main.

Branch thing is done. But that role would need to be called from each and every playbook to detect and set the bootc variable right? I guess we can just add it as a ansibleVar and avoid calling the role each and every time we start a new service.

So, we can use a custom fact. For example:

cat /etc/ansible/facts.d/bootc.fact #!/usr/bin/env bash is_bootc() { BOOTC_STATUS=$(sudo bootc status --json | jq .status.type) if [[ "$BOOTC_STATUS" == \"bootcHost\" ]]; then BOOTC_SYSTEM="true" else BOOTC_SYSTEM="false" fi } is_bootc echo ${BOOTC_SYSTEM}

This is good from the perspective of not needing the user to manually define that they are using a bootc system, plus it works for our non bootc systems:

[m3@osp-df-3 bootc]$ ansible -i inv.yaml all -m setup -a "filter=ansible_local" edpm-compute-1 | SUCCESS => { "ansible_facts": { "ansible_local": { "bootc": true }, "discovered_interpreter_python": "/usr/bin/python3" }, "changed": false }

So you could combine them in the same NodeSet if you wanted to. But, the down side of this approach is that we need to gather facts from each service. We have thus far tried to limit the amount of fact gathering required, so this approach my not be what we want to do without some more granular control of which facts are being gathered in each service. At the moment, we just define a variable for gather_facts. If that variable is true, then we gather all facts. Obviously, that becomes necessary if we want to allow individual task executions that require facts, but when we want to just gather local facts, then gathering all of them introduces non-trivial time to our executions of each service.

Offering it as a potential solution that we can debate. The alternative is that we require either bootc or non-bootc nodes in each NodeSet.

bshephar · 2024-11-27T05:04:00Z

roles/edpm_logrotate_crond/tasks/configure.yml

    - name: Push script
      ansible.builtin.copy:
-        dest: /usr/local/sbin/containers-tmpwatch
+        dest: /var/lib/openstack/cron/containers-tmpwatch


/usr is immutable with bootc deployments. So I've proposed doing this in two different ways. 1, we bake the scripts into the container file:
https://github.com/openstack-k8s-operators/install_yamls/pull/950/files#diff-f8fb9af5355b45b9ca8936bf0d721c6f0e37e13b637f5598e2be19995dea23e7R45-R46

And 2. Which is this method of writing to /var/lib/openstack. I personally prefer doing it this way if we can agree on a common place for any scripts that we want to use. That saves us baking things into images and then trying to keep them in sync. Better imo to have them in edpm-ansible for now.

bshephar · 2024-11-27T05:04:53Z

roles/edpm_ovn/tasks/run.yml

-  ansible.builtin.include_role:
-    name: osp.edpm.edpm_container_standalone
-  vars:
-    edpm_container_standalone_service: ovn_controller
-    edpm_container_standalone_container_defs:
-      ovn_controller: "{{ lookup('template', 'ovn_controller.yaml.j2') | from_yaml }}"
-    edpm_container_standalone_kolla_config_files:
-      ovn_controller: "{{ lookup('template', 'kolla_ovn_controller.yaml.j2') | from_yaml }}"


All of this needs to stay in order to support both deployment methodologies. It can just be conditional like:
https://github.com/openstack-k8s-operators/edpm-ansible/pull/830/files#diff-34e3323585e197e806d463771e3b5132716048c41818b1318fecb2c0d8e36cd6R45

softwarefactory-project-zuul · 2024-11-27T07:55:22Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/3d020a0278384b7f97de3e8e26403819

softwarefactory-project-zuul · 2024-11-30T12:39:38Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/87ae0cbb50854f54a18570dc772271c1

softwarefactory-project-zuul · 2024-12-02T04:52:03Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/deff54e047964013a0c7461f18cfe415

slagle · 2024-12-06T21:10:54Z

roles/edpm_ovn/tasks/run.yml

+        {{
+          ovn_controller_pod_spec | combine({
+            'spec': {
+              'containers': ovn_controller_pod_spec.spec.containers | zip_longest([], [{'image': edpm_ovn_controller_agent_image}]) | map('combine') | list,


I believe that if we were to customize the image like this then by definition the container is no longer "logically bound". Instead, it would be considered a "floating" container per https://containers.github.io/bootc/logically-bound-images.html#comparison-with-default-podman-systemd-units

and also:

There is no mechanism to inject arbitrary arguments to the podman pull (or equivalent) invocation used by bootc.

which seems to apply additional mounts or other options passed to podman pull are not possible.

The dynamically-injected ConfigMaps[1][2] may provide some customization, but that is still not likely for the app container image itself, b/c once that is changed to some other image, then that no longer fits into how logically bound images should be managed with the lifecycle of the base bootc image itself.

Point being...if we choose to allow the ability to podman run any arbitrary image at runtime, then these really aren't logically bound images at all, but are considered "floating".

The question becomes, should we adopt logically bound images, and require our end users to be building new bootc images before deploying EDPM nodes, depending on if they need to customize any of the container images? We could ship a bootc image that had all the images logically bound, but if a user wanted to run a different one (from a partner, etc) they they would need to rebuild that image.

I do like the quadlet/systemd design, and I think we can still adopt that either way.

[1] https://containers.github.io/bootc/building/guidance.html?highlight=configmap#configuration
[2] containers/bootc#22

softwarefactory-project-zuul · 2024-12-09T08:35:24Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/e201c6428c384df8a92084c2694ba93e

slagle · 2024-12-10T17:01:16Z

I created a new bootc branch for edpm-ansible: https://github.com/openstack-k8s-operators/edpm-ansible/tree/bootc

Can you propose this PR to the bootc branch instead?

I reverted #813 from main in #844 I think that was the only other bootc related PR that has merged.

slagle · 2025-01-17T15:47:34Z

roles/edpm_bootstrap/tasks/bootstrap.yml

+
+- name: Import packages tasks
+  ansible.builtin.import_tasks: packages.yml
+  when: not ansible_local.bootc


You need to update this change based on the commit I made earlier to handle bootc. packages.yml is already included earlier at line 24 using the other variable I had used "bootc". So that needs to be undone so we can go forward with what you're proposing here.

also should we switch this to include_tasks so that the when will skip all the tasks at once instead of individually?

I also see that the only reason ansible_local.bootc is set here is because of the task in bootstrap_command.yml to read local facts. I think we need that to be more explicit. Probably add something directly in bootstrap.yml.

I believe this answers my earlier comment on playbooks/bootstrap.yml on how the fact is initially set. We should make it more explicit.

slagle · 2025-01-17T16:15:40Z

playbooks/bootstrap.yml

@@ -42,6 +42,7 @@
        name: osp.edpm.edpm_kernel
      tags:
        - edpm_kernel
+      when: not ansible_local.bootc


How is the fact ansible_local.bootc initially gathered given that gather_facts defaults to false in the playbook?

We need to figure out what we want this to look like still. I have just set gather_facts: true in my deployment Ansible vars.

Maybe we would need to change the default for it to gather_subset: local at a minimum. It's not ideal that we would gather all facts for every service, but at the moment, that's what my gather_facts: true is doing until I come up with something better.

slagle · 2025-01-17T16:54:33Z

playbooks/bootstrap.yml

we will also need to guard the task Download packages from roles/edpm_download_cache/tasks/main.yml with the fact

to workaround this I dropped download-cache from my NodeSet services

roles/edpm_container_manage/tasks/shutdown.yml

slagle · 2025-01-17T17:32:48Z

roles/edpm_logrotate_crond/tasks/configure.yml

seems like the Insert cronjob in root crontab task requires the cronie rpm. We might need to add that to the image build.

openstack-k8s-operators/edpm-image-builder#43

This change moves the script we're using for the logs cronjob into the /var/lib/openstack/cron directory. This facilitates the bootc immutable filesystem where we can't write to /usr, while also consolidating scripts relevant to our deployment in a common place. Signed-off-by: Brendan Shephard <[email protected]>

Signed-off-by: Brendan Shephard <[email protected]>

slagle · 2025-01-20T16:13:44Z

roles/edpm_logrotate_crond/tasks/run.yml

+    ansible.builtin.systemd_service:
+      name: edpm-compute@logrotate_crond
+      enabled: true
+      state: started


I think we need to separate out this initial bootc support from the logically bound containers PR, openstack-k8s-operators/edpm-image-builder#39

This PR has a strong dep on the logically bound PR, and that complicates things. Let's just get a base bootc working with how we manage containers presently. We can move to logically bound and all the quadlet/systemd stuff as a next step.

Yeah, ok. Let's decouple them. I'll just submit a new PR to edpm-image-builder to change the Quadlet files over to using .container instead of .kube. Then a new one here to work with those instead of the .kube files.

bshephar · 2025-01-22T23:48:11Z

So, we merged a subset of this in the interest of slimming down the number of required changes. Let's backlog this particular piece of work in favor of getting something demo worthy. We'll collect feedback, and then we can loop back on the logically bound images work if we feel it's still worth pursuing.

openshift-merge-robot · 2025-01-22T23:48:19Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

openshift-ci bot added the do-not-merge/work-in-progress label Nov 27, 2024

openshift-ci bot added the approved label Nov 27, 2024

bshephar commented Nov 27, 2024

View reviewed changes

bshephar force-pushed the support-bootc branch from dd3d198 to cde9667 Compare November 30, 2024 09:47

bshephar force-pushed the support-bootc branch 12 times, most recently from ffd86e6 to 54f7101 Compare December 2, 2024 01:52

slagle reviewed Dec 6, 2024

View reviewed changes

bshephar force-pushed the support-bootc branch 2 times, most recently from a0569c4 to b590229 Compare December 9, 2024 05:38

bshephar force-pushed the support-bootc branch 4 times, most recently from 1afffaf to 51a8b34 Compare December 11, 2024 00:34

bshephar changed the base branch from main to bootc December 11, 2024 01:34

slagle reviewed Jan 17, 2025

View reviewed changes

bshephar force-pushed the support-bootc branch 4 times, most recently from 29ca81f to 8d92a63 Compare January 20, 2025 01:05

bshephar force-pushed the support-bootc branch 3 times, most recently from a0efece to 94f062b Compare January 20, 2025 01:43

bshephar added 9 commits January 20, 2025 13:27

Change name of services for bootc

f77e8eb

Signed-off-by: Brendan Shephard <[email protected]>

Rebase on bootc branch

2e6ec67

Signed-off-by: Brendan Shephard <[email protected]>

edpm_tuned support bootc

5323ada

Signed-off-by: Brendan Shephard <[email protected]>

Skip edpm_kernel when using bootc

b0c5c6f

Signed-off-by: Brendan Shephard <[email protected]>

Add Telemetry bootc compatibility

ee7c9a4

Signed-off-by: Brendan Shephard <[email protected]>

Add multipathd bootc support

35944eb

Signed-off-by: Brendan Shephard <[email protected]>

Use local fact for bootc

3fa4be3

Signed-off-by: Brendan Shephard <[email protected]>

Skip swapfile creation for bootc

f1accda

Signed-off-by: Brendan Shephard <[email protected]>

Use quadlet for logrotate_crond

201b673

Signed-off-by: Brendan Shephard <[email protected]>

bshephar force-pushed the support-bootc branch 2 times, most recently from 0260435 to 0d05701 Compare January 20, 2025 04:06

bshephar added 3 commits January 20, 2025 15:38

Nova_compute support bootc

dcbe803

Signed-off-by: Brendan Shephard <[email protected]>

Ensure /var/log/containers exists

e8ce9a6

Signed-off-by: Brendan Shephard <[email protected]>

Render kolla_config files for bootc

d56b311

Signed-off-by: Brendan Shephard <[email protected]>

bshephar force-pushed the support-bootc branch from 0d05701 to d56b311 Compare January 20, 2025 05:38

slagle reviewed Jan 20, 2025

View reviewed changes

slagle mentioned this pull request Jan 21, 2025

Support bootc fixes #858

Merged

openshift-merge-robot added the needs-rebase label Jan 22, 2025

bshephar closed this Jan 22, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support bootc #830

Support bootc #830

bshephar commented Nov 27, 2024

openshift-ci bot commented Nov 27, 2024

bshephar Nov 27, 2024

slagle Dec 10, 2024

slagle Dec 10, 2024

bshephar Dec 11, 2024

bshephar Dec 16, 2024

bshephar Nov 27, 2024

bshephar Nov 27, 2024

softwarefactory-project-zuul bot commented Nov 27, 2024

softwarefactory-project-zuul bot commented Nov 30, 2024

softwarefactory-project-zuul bot commented Dec 2, 2024

slagle Dec 6, 2024 •

edited

Loading

softwarefactory-project-zuul bot commented Dec 9, 2024

slagle commented Dec 10, 2024 •

edited

Loading

slagle Jan 17, 2025

slagle Jan 17, 2025

slagle Jan 17, 2025

slagle Jan 17, 2025

bshephar Jan 17, 2025

slagle Jan 17, 2025

slagle Jan 17, 2025

slagle Jan 17, 2025

slagle Jan 20, 2025

bshephar Jan 21, 2025

bshephar commented Jan 22, 2025

openshift-merge-robot commented Jan 22, 2025

Support bootc #830

Support bootc #830

Conversation

bshephar commented Nov 27, 2024

openshift-ci bot commented Nov 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

softwarefactory-project-zuul bot commented Nov 27, 2024

softwarefactory-project-zuul bot commented Nov 30, 2024

softwarefactory-project-zuul bot commented Dec 2, 2024

slagle Dec 6, 2024 • edited Loading

Choose a reason for hiding this comment

softwarefactory-project-zuul bot commented Dec 9, 2024

slagle commented Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bshephar commented Jan 22, 2025

openshift-merge-robot commented Jan 22, 2025

slagle Dec 6, 2024 •

edited

Loading

slagle commented Dec 10, 2024 •

edited

Loading