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

[edpm_update] Added edpm_update role and update playbook #623

Merged
merged 5 commits into from
May 22, 2024

Conversation

rebtoor
Copy link
Contributor

@rebtoor rebtoor commented Apr 12, 2024

The edpm_update role is added to allow the update of packages and containers on EDPM nodes.

The related update.yml playbook has been added.

Jira: https://issues.redhat.com/browse/OSPRH-6219

Copy link
Contributor

openshift-ci bot commented Apr 12, 2024

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

@rebtoor rebtoor requested a review from slagle April 12, 2024 15:30
@rebtoor rebtoor changed the title [edpm_update] Added edpm_update role and added update.yml tasks file to impacted role. [edpm_update] [edpm_update] Added edpm_update role and and update playbook Apr 12, 2024
Copy link
Contributor

@slagle slagle left a comment

Choose a reason for hiding this comment

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

Thanks, this is what we discussed and looks good from that perspective.

However, it occurred to me that since openstack-operator will be providing the image url using sha256 instead of tags, that the simplest case of pull the image and restart the container won't work for that, we'll also need to use the tasks from run.yml.

Copy link
Contributor

@slagle slagle Apr 12, 2024

Choose a reason for hiding this comment

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

for all the update.yml, I think this works for the simplest case when the image is using tags, such as :latest.

since we plan to use sha256 as well, we'll also need to run the task to render the container definition files since those contain the image. Then, we need to use the edpm_container_standalone role to restart the container since that will write out the updated container definition file (/var/lib/edpm-config/container-startup-config/ovn_controller/ovn_controller.json) which contains the image with sha256 (hardcoded).

This effectively what we already have in run.yml for roles such as these (edpm_ovn for example).

Do you think we should just use run.yml for both initial deploy and updates? Or use a dedicated update.yml, which will look pretty similar to run.yml?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@slagle imho we can reuse run.yml and avoid a new task file, also for the issue raised by @jpodivin below.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is trade-off between having separate update playbook/service with a lot of repetition from run.yaml and using the same deployment playbooks/services for update too. From user experience point of view one playbook for starting update is better, but it might be hard to maintain run.yaml/update.yaml files in sync. I would favor to use run.yaml for update too and update.yaml tasks just for extra steps that need to happen during update. This would follow the same pattern we had in tripleo.

@jpodivin
Copy link
Contributor

The update images and restart containers tasks repeat a lot with modest variation. Would it be possible to factor them out into their own role, and invoke that with proper arguments in the respective plays?

@rebtoor rebtoor changed the title [edpm_update] [edpm_update] Added edpm_update role and and update playbook [edpm_update] [edpm_update] Added edpm_update role and update playbook Apr 15, 2024
@rebtoor rebtoor force-pushed the OSPRH-3890 branch 2 times, most recently from 06ea57c to f3e602a Compare April 15, 2024 13:27
@rebtoor rebtoor requested a review from slagle April 15, 2024 13:28
Copy link

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://review.rdoproject.org/zuul/buildset/b126122738884af880095a7c01fb3cec

✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 08m 36s
podified-multinode-edpm-deployment-crc FAILURE in 53m 10s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 50m 50s
✔️ edpm-ansible-molecule-edpm_bootstrap SUCCESS in 6m 04s
✔️ edpm-ansible-molecule-edpm_podman SUCCESS in 5m 11s
✔️ edpm-ansible-molecule-edpm_module_load SUCCESS in 4m 11s
✔️ edpm-ansible-molecule-edpm_kernel SUCCESS in 9m 47s
✔️ edpm-ansible-molecule-edpm_libvirt SUCCESS in 9m 59s
✔️ edpm-ansible-molecule-edpm_nova SUCCESS in 10m 41s
✔️ edpm-ansible-molecule-edpm_frr SUCCESS in 7m 10s
✔️ edpm-ansible-molecule-edpm_iscsid SUCCESS in 5m 29s
✔️ edpm-ansible-molecule-edpm_ovn_bgp_agent SUCCESS in 8m 09s
✔️ edpm-ansible-molecule-edpm_ovs SUCCESS in 5m 37s

@rebtoor rebtoor marked this pull request as ready for review April 15, 2024 16:51
@openshift-ci openshift-ci bot requested review from abays and olliewalsh April 15, 2024 16:51
@rebtoor rebtoor force-pushed the OSPRH-3890 branch 2 times, most recently from a9b435d to b7aed71 Compare April 15, 2024 20:37
@bshephar bshephar self-requested a review April 16, 2024 09:50
@rebtoor rebtoor changed the title [edpm_update] [edpm_update] Added edpm_update role and update playbook [edpm_update] Added edpm_update role and update playbook Apr 16, 2024
Copy link
Contributor

@jpodivin jpodivin left a comment

Choose a reason for hiding this comment

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

LGTM. But let's wait for the conversation to finish.

@@ -0,0 +1,10 @@
---

Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -0,0 +1,10 @@
---

- name: Apply packages updates
Copy link
Contributor

Choose a reason for hiding this comment

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

this needs become:true

@@ -0,0 +1,105 @@
---

- name: Login to container registries if needed
Copy link
Contributor

Choose a reason for hiding this comment

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

i think all of these also need become:true

Copy link
Contributor

Choose a reason for hiding this comment

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

actually you can't use become:true on include_role, so maybe just set it in the update.yml playbook. Either that or we'll need to go through and add it on every task that needs it.

Copy link
Contributor

Choose a reason for hiding this comment

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

when I tested this, I used become:true in the playbook, so let's just go with that for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, we can!

https://docs.ansible.com/ansible/latest/collections/ansible/builtin/include_role_module.html#parameter-apply
e.g.

- name: Login to container registries if needed
  ansible.builtin.include_role:
    name: osp.edpm.edpm_podman
    tasks_from: login.yml
    apply:
      become: true
  tags:
    - edpm_podman
    - edpm_update

rebtoor and others added 4 commits May 22, 2024 18:36
The `edpm_update` role is added to allow the update of packages and
containers on EDPM nodes.

A file called `update.yml` has been added into all impacted roles.

Jira: https://issues.redhat.com/browse/OSPRH-6219
Signed-off-by: Roberto Alfieri <[email protected]>
openvswitch requires special handling when updating. It should be
excluded from the generic package update.

Signed-off-by: James Slagle <[email protected]>
The edpm_update containers.yml tasks should only be executed when the
respective services are actually installed on the node.

Signed-off-by: James Slagle <[email protected]>
In order to proper test the update of all the packages,
a custom Containerfile without `dnf upgrade -y` task was provided.

Signed-off-by: Roberto Alfieri <[email protected]>
Copy link

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://review.rdoproject.org/zuul/buildset/481aba23b18e4b33ac9a3fa13ef48edc

openstack-k8s-operators-content-provider FAILURE in 17m 26s
⚠️ podified-multinode-edpm-deployment-crc SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ cifmw-crc-podified-edpm-baremetal SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
✔️ edpm-ansible-molecule-edpm_bootstrap SUCCESS in 6m 20s
✔️ edpm-ansible-molecule-edpm_podman SUCCESS in 5m 20s
✔️ edpm-ansible-molecule-edpm_module_load SUCCESS in 4m 52s
✔️ edpm-ansible-molecule-edpm_kernel SUCCESS in 9m 16s
✔️ edpm-ansible-molecule-edpm_libvirt SUCCESS in 8m 32s
✔️ edpm-ansible-molecule-edpm_nova SUCCESS in 8m 49s
✔️ edpm-ansible-molecule-edpm_frr SUCCESS in 6m 53s
✔️ edpm-ansible-molecule-edpm_iscsid SUCCESS in 4m 30s
✔️ edpm-ansible-molecule-edpm_ovn_bgp_agent SUCCESS in 6m 59s
✔️ edpm-ansible-molecule-edpm_ovs SUCCESS in 5m 06s

Copy link
Contributor

openshift-ci bot commented May 22, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fao89, rebtoor, slagle

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [fao89,rebtoor,slagle]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fao89
Copy link
Contributor

fao89 commented May 22, 2024

recheck

Copy link

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://review.rdoproject.org/zuul/buildset/d571b8a761c54062b77b811e777991ca

openstack-k8s-operators-content-provider FAILURE in 9m 56s
⚠️ podified-multinode-edpm-deployment-crc SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ cifmw-crc-podified-edpm-baremetal SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
✔️ edpm-ansible-molecule-edpm_bootstrap SUCCESS in 8m 10s
✔️ edpm-ansible-molecule-edpm_podman SUCCESS in 5m 10s
✔️ edpm-ansible-molecule-edpm_module_load SUCCESS in 4m 41s
✔️ edpm-ansible-molecule-edpm_kernel SUCCESS in 9m 25s
✔️ edpm-ansible-molecule-edpm_libvirt SUCCESS in 10m 18s
✔️ edpm-ansible-molecule-edpm_nova SUCCESS in 10m 57s
✔️ edpm-ansible-molecule-edpm_frr SUCCESS in 7m 44s
✔️ edpm-ansible-molecule-edpm_iscsid SUCCESS in 5m 13s
✔️ edpm-ansible-molecule-edpm_ovn_bgp_agent SUCCESS in 8m 13s
✔️ edpm-ansible-molecule-edpm_ovs SUCCESS in 6m 08s

@slagle
Copy link
Contributor

slagle commented May 22, 2024

recheck

Copy link

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://review.rdoproject.org/zuul/buildset/193c85b614fe40db9bcbf5b5f5bddc39

openstack-k8s-operators-content-provider FAILURE in 18m 37s
⚠️ podified-multinode-edpm-deployment-crc SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ cifmw-crc-podified-edpm-baremetal SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
✔️ edpm-ansible-molecule-edpm_bootstrap SUCCESS in 7m 57s
✔️ edpm-ansible-molecule-edpm_podman SUCCESS in 6m 54s
✔️ edpm-ansible-molecule-edpm_module_load SUCCESS in 5m 47s
✔️ edpm-ansible-molecule-edpm_kernel SUCCESS in 13m 27s
✔️ edpm-ansible-molecule-edpm_libvirt SUCCESS in 11m 05s
✔️ edpm-ansible-molecule-edpm_nova SUCCESS in 11m 01s
✔️ edpm-ansible-molecule-edpm_frr SUCCESS in 7m 47s
✔️ edpm-ansible-molecule-edpm_iscsid SUCCESS in 5m 55s
✔️ edpm-ansible-molecule-edpm_ovn_bgp_agent SUCCESS in 8m 32s
✔️ edpm-ansible-molecule-edpm_ovs SUCCESS in 6m 05s

@slagle
Copy link
Contributor

slagle commented May 22, 2024

recheck

don't think the failure is related

2024-05-22 14:10:49.955459 | controller | Wednesday 22 May 2024 14:10:49 -0400 (0:01:52.098) 0:11:55.742 ********* 2024-05-22 14:11:43.793441 | controller | FAILED - RETRYING: [localhost]: openstack-operator - Push bundle image (5 retries left). 2024-05-22 14:11:43.793512 | controller | FAILED - RETRYING: [localhost]: openstack-operator - Push bundle image (4 retries left). 2024-05-22 14:11:43.793525 | controller | FAILED - RETRYING: [localhost]: openstack-operator - Push bundle image (3 retries left). 2024-05-22 14:11:43.793535 | controller | FAILED - RETRYING: [localhost]: openstack-operator - Push bundle image (2 retries left). 2024-05-22 14:11:43.793551 | controller | FAILED - RETRYING: [localhost]: openstack-operator - Push bundle image (1 retries left). 2024-05-22 14:11:43.793561 | controller | An exception occurred during task execution. To see the full traceback, use -vvv. The error was: AttributeError: 'NoneType' object has no attribute 'split'

@openshift-merge-bot openshift-merge-bot bot merged commit cf29422 into openstack-k8s-operators:main May 22, 2024
35 checks passed
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.

7 participants