-
Notifications
You must be signed in to change notification settings - Fork 139
Add FDP update automation for EDPM deployments #3432
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 FDP update automation for EDPM deployments #3432
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
19e68bb to
55d1b16
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/7c305864d102485fbe9dafd0ff85da35 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 47m 25s |
55d1b16 to
cfe3b16
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/30c74ff1ec854ebd94ca1da1b1070b64 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 41m 57s |
6a356e7 to
d6d0563
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/15ccfc3015ed498c8bec52bc522be2c3 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 59m 47s |
e0f9071 to
f46652f
Compare
playbooks/fdp_update.yml
Outdated
| changed_when: _fdp_update_nat_rule.rc == 0 | ||
|
|
||
| - name: Persist firewall rules | ||
| ansible.builtin.shell: # noqa: command-instead-of-shell |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think is raising any error, here is the same situation: https://github.com/openstack-k8s-operators/ci-framework/pull/3432/files#diff-a961c65eda0f40d05092b70d7ecb9015ee9d2236e273248178dc0c2406268deeR23
| changed_when: false | ||
|
|
||
| - name: Authenticate podman with TLS verification | ||
| ansible.builtin.shell: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use here: podman_login https://docs.ansible.com/ansible/latest/collections/containers/podman/podman_login_module.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
78e14cb to
063c2a8
Compare
f290329 to
0ac0413
Compare
| cifmw_fdp_update_container_images_repo_name: "custom-repo" | ||
| cifmw_fdp_update_container_images_repo_baseurl: "" # REQUIRED - must be set by user | ||
| cifmw_fdp_update_container_images_repo_enabled: 1 | ||
| cifmw_fdp_update_container_images_repo_gpgcheck: 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a nit: do we really want to have so much parameters available?
| # DNF update arguments | ||
| cifmw_fdp_update_container_images_update_dnf_args: "--disablerepo='*' --enablerepo={{ cifmw_fdp_update_container_images_repo_name }}" | ||
|
|
||
| # Internal variables (do not override) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if later you set fact with empty vars, why in defaults you put same? Please remove, otherwise we will be confused why e.g. this var is available where set_fact is setting it.
|
|
||
| - name: Extract registry host from route | ||
| ansible.builtin.set_fact: | ||
| _cifmw_fdp_update_container_images_route: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why you do dict here with {'stdout': {{ cifmw_fdp_update_container_images_route_info.resources[0].spec.host if _cifmw_fdp_update_container_images_route_info.resources | length > 0 else '' }}"} ? No need to make dict.
| _cifmw_fdp_update_container_images_route: | ||
| stdout: "{{ _cifmw_fdp_update_container_images_route_info.resources[0].spec.host if _cifmw_fdp_update_container_images_route_info.resources | length > 0 else '' }}" | ||
|
|
||
| - name: Set registry URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we really want to make var reassign here?
| - name: Verify registry URL | ||
| ansible.builtin.fail: | ||
| msg: "Failed to determine registry URL. Set cifmw_fdp_update_container_images_image_registry manually." | ||
| when: cifmw_fdp_update_container_images_image_registry is not defined or cifmw_fdp_update_container_images_image_registry | length == 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so if you verify here length because other condition fail, why not move the condition here instead of verify length?
| ansible.builtin.debug: | ||
| msg: | ||
| - "==========================================" | ||
| - "✓ Container image update complete" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove the non-ascii chars
| # under the License. | ||
|
|
||
| - name: Create repository file | ||
| ansible.builtin.template: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: consider to use module, just a hint, not something important.
|
|
||
| - name: Display progress | ||
| ansible.builtin.debug: | ||
| msg: "✓ Updated {{ image_entry.key }} ({{ _cifmw_fdp_update_container_images_processed_images }}/{{ _cifmw_fdp_update_container_images_image_entries | length }})" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same with non ascii chars
| @@ -0,0 +1,3 @@ | |||
| FROM {{ base_image }} | |||
| COPY ./{{ cifmw_fdp_update_container_images_repo_name }}.repo /etc/yum.repos.d/ | |||
| RUN dnf update -y {{ cifmw_fdp_update_container_images_update_dnf_args }} {{ cifmw_fdp_update_container_images_target_package }}* | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: dnf clean all can save few MB
| 4. Pushing updated images to OpenShift internal registry | ||
| 5. Patching OpenStackVersion CR to use the new images | ||
|
|
||
| ## Privilege escalation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can remove the chapter
Implement comprehensive FDP update workflow including: - New playbook fdp_update.yml orchestrating the update process - Role fdp_edpm_update_host_packages: Updates packages on EDPM hosts - Role fdp_update_container_images: Rebuilds container images with updated packages * Includes Molecule tests for validation * Jinja2 templates for Dockerfile and repo configuration - Role fdp_update_edpm_containers: Updates running EDPM containers This automation streamlines the process of updating Fast Data Path components across OpenStack EDPM (External Data Plane Management) deployments by coordinating host package updates, container image rebuilds, and container deployment updates. Assisted-By: Claude <[email protected]> Signed-off-by: Miguel Angel Nieto Jimenez <[email protected]>
0ac0413 to
0796edc
Compare
|
@mnietoji: The following test failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/a7d3f702a34f4e228d124d8d2e863162 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 35m 50s |
|
so now I need to check 3466 and compare with my comments that I left here. Might take 2x more time to review it. |
I'm reviewing it, I'm moving you'r reviews there. |
Uh oh!
There was an error while loading. Please reload this page.