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

fix(ansible): Adding bios config job and reboot tasks in bios.yml #40

Closed
wants to merge 8 commits into from

Conversation

rahulsachdev
Copy link
Contributor

No description provided.

@rahulsachdev rahulsachdev requested a review from a team as a code owner July 8, 2024 12:01
ansible/bios.yml Outdated
# TODO: configre BIOS to be always on ( see lab/hardware/dh123) and any virtualization or hyper threading settings we might need

- name: Debug print bios serial number
when: inventory_hostname == 'dh1bmc'
Copy link
Member

Choose a reason for hiding this comment

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

remove when here and rename the task from serial

ansible/bios.yml Outdated
# password: "{{ ansible_password }}"
# when: bios_attribute.changed
- name: Create BIOS configuration job (schedule BIOS setting update)
when: inventory_hostname == 'dh1bmc'
Copy link
Member

Choose a reason for hiding this comment

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

I think we should use another condition here
In previous task or from gather_facts we can already know if we talking to iDRAC or not
let's use that as a condition

ansible/bios.yml Outdated
# password: "{{ ansible_password }}"
# when: bios_attribute.changed
- name: Reboot system to apply new BIOS settings
when: inventory_hostname == 'dh1bmc'
Copy link
Member

Choose a reason for hiding this comment

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

two when statements ? one here and another one below bios_config_job.changed

Copy link
Member

@glimchb glimchb left a comment

Choose a reason for hiding this comment

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

2 when statements

ansible/bios.yml Outdated
# TODO: configre BIOS to be always on ( see lab/hardware/dh123) and any virtualization or hyper threading settings we might need
#- name: Debug print bios attributes
#ansible.builtin.debug: msg={{ result }}
#ansible.builtin.debug: msg={{ result.redfish_facts.bios_attribute.entries[0][1].SystemManufacturer }}
Copy link
Member

Choose a reason for hiding this comment

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

this is a good debug line to keep uncommented
all the rest comments should be removed

ansible/bios.yml Outdated
@@ -43,25 +49,30 @@
username: "{{ ansible_user }}"
password: "{{ ansible_password }}"
register: bios_attribute
tags: ["TEST"]
# tags: ["TEST"]
Copy link
Member

Choose a reason for hiding this comment

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

remove this line

ansible/bios.yml Outdated
# password: "{{ ansible_password }}"
# when: bios_attribute.changed
- name: Create BIOS configuration job (schedule BIOS setting update)
when: result.redfish_facts.bios_attribute.entries[0][1].SystemManufacturer is defined and result.redfish_facts.bios_attribute.entries[0][1].SystemManufacturer == "Dell Inc." and bios_attribute.changed
Copy link
Member

Choose a reason for hiding this comment

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

let's split into lines like

- name: Run telegraf container on others
when:
- inventory_hostname != 'mev'
- inventory_hostname != 'bf2'
community.docker.docker_container:

Copy link
Member

@glimchb glimchb left a comment

Choose a reason for hiding this comment

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

Much better!!!
few small suggestions inside
and please squash all the commits to a single one

@rahulsachdev
Copy link
Contributor Author

Will work on Boris's suggestions and submit a new PR

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

Successfully merging this pull request may close these issues.

2 participants