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.

Sign up for GitHub

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

Jump to bottom

Make edpm_kernel compliant with ansible-lint production profile #408

Merged
merged 1 commit into from
Oct 6, 2023
Merged

Make edpm_kernel compliant with ansible-lint production profile #408

merged 1 commit into from
Oct 6, 2023

Conversation

rebtoor
Copy link
Contributor

@rebtoor rebtoor commented Oct 6, 2023 •
edited
Loading

  • Added Sysctl reload as handler removing a when clause as for ansible best practice
  • Replaced a command task with slurp module to get remote file content and set_fact task to define the cmdline variable
  • Fixed indentation and spacing for better readability
  • Moved jinja templating at the end of a task name
  • Added changed_when and failed_when in command tasks
  • Added a debug task to whitelist
  • Declared permissions as strings in mode parameter as for ansible best practices

@rebtoor rebtoor requested review from SeanMooney and bogdando October 6, 2023 09:51
@openshift-ci openshift-ci bot requested review from olliewalsh and viroel October 6, 2023 09:51
@openshift-ci openshift-ci bot added the approved label Oct 6, 2023
SeanMooney
SeanMooney requested changes Oct 6, 2023
View reviewed changes
roles/edpm_kernel/tasks/kernelargs.yml Outdated Show resolved Hide resolved
roles/edpm_kernel/tasks/kernelargs.yml Show resolved Hide resolved
roles/edpm_kernel/tasks/main.yml Show resolved Hide resolved
@softwarefactory-project-zuul
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/67c9802d7c0a45a899ec22b10ac968ea

✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 55m 47s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 1h 39m 03s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 38m 43s
✔️ edpm-ansible-molecule-edpm_podman SUCCESS in 5m 51s
✔️ edpm-ansible-molecule-edpm_module_load SUCCESS in 5m 04s
❌ edpm-ansible-molecule-edpm_kernel FAILURE in 4m 25s
✔️ edpm-ansible-molecule-edpm_libvirt SUCCESS in 6m 07s
✔️ edpm-ansible-molecule-edpm_nova SUCCESS in 6m 16s

@rebtoor rebtoor force-pushed the kernel_production_profile branch from 42df36b to 59851b4 Compare October 6, 2023 13:10
@rebtoor rebtoor requested a review from SeanMooney October 6, 2023 13:10
SeanMooney
SeanMooney approved these changes Oct 6, 2023
View reviewed changes
Copy link
Contributor

@SeanMooney SeanMooney left a comment

Choose a reason for hiding this comment

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

this looks good to me now, thanks for addressing my comment :)

@openshift-ci openshift-ci bot added the lgtm label Oct 6, 2023
@SeanMooney
Copy link
Contributor

acctully this is going to fail with

RUNNING HANDLER [osp.edpm.edpm_kernel : Reload sysctl] *************************
fatal: [instance]: FAILED! => changed=false
fatal: [instance]: FAILED! => changed=false
msg: |-
msg: |-
Unable to restart service systemd-sysctl.service: Failed to restart systemd-sysctl.service: Interactive authentication required.
Unable to restart service systemd-sysctl.service: Failed to restart systemd-sysctl.service: Interactive authentication required.
See system logs and 'systemctl status systemd-sysctl.service' for details.
See system logs and 'systemctl status systemd-sysctl.service' for details.

@openshift-ci openshift-ci bot removed the lgtm label Oct 6, 2023
@rebtoor
Make edpm_kernel compliant with ansible-lint production profile
586d566 
- Added `Sysctl reload` as handler removing a `when` clause as for ansible best practice
- Replace a `command` task with `slurp` module to get remote file content and
  `set_fact` task to define the `cmdline` variable
- Fixed indentation and spacing for better readability
- Moved jinja templating at the end of a task name
- Added `changed_when` and `failed_when` in command tasks
- Added a debug task to whitelist
- Declared permissions as strings in `mode` parameter as for ansible best practices

Signed-off-by: Roberto Alfieri <[email protected]>
@rebtoor rebtoor force-pushed the kernel_production_profile branch from 59851b4 to 586d566 Compare October 6, 2023 13:41
@openshift-ci openshift-ci bot added the lgtm label Oct 6, 2023
bogdando
bogdando approved these changes Oct 6, 2023
View reviewed changes
Copy link
Contributor

@bogdando bogdando left a comment

Choose a reason for hiding this comment

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

thanks!

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 6, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bogdando, rebtoor, SeanMooney

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:

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

@openshift-ci openshift-ci bot merged commit 405121d into openstack-k8s-operators:main Oct 6, 2023
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bogdando bogdando bogdando approved these changes

@SeanMooney SeanMooney SeanMooney approved these changes

@olliewalsh olliewalsh Awaiting requested review from olliewalsh

@viroel viroel Awaiting requested review from viroel

Assignees

@SeanMooney SeanMooney

@bogdando bogdando

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rebtoor @SeanMooney @bogdando