Conversation
Reviewer's GuideAdds SLE16+ (and openSUSE 16+) support to the kdump Ansible role by introducing SUSE-specific variables and using kdumptool instead of grubby for crashkernel updates, while skipping kdump service start when a reboot is required. Sequence diagram for SLES16+ crashkernel update using kdumptoolsequenceDiagram
actor Admin
participant AnsibleController
participant KdumpRole
participant SLES16Host
participant Kdumptool
Admin->>AnsibleController: run kdump role on SLES16Host
AnsibleController->>KdumpRole: execute tasks/main.yml
KdumpRole->>SLES16Host: gather ansible_facts
KdumpRole->>SLES16Host: evaluate kdump_reboot_required
alt os_family is Suse and distribution_major_version >= 16 and kdump_reboot_required is true
KdumpRole->>Kdumptool: /usr/sbin/kdumptool commandline -u
Kdumptool-->>KdumpRole: update bootloader crashkernel setting
else other systems or versions
KdumpRole-->>SLES16Host: skip kdumptool task
end
alt kdump_reboot_required is false
KdumpRole->>SLES16Host: start __kdump_service
SLES16Host-->>KdumpRole: kdump service started
else kdump_reboot_required is true
KdumpRole-->>SLES16Host: skip __kdump_service start
end
KdumpRole-->>AnsibleController: role execution complete
AnsibleController-->>Admin: report kdump configuration status
Flow diagram for kdump role logic with Suse-specific handlingflowchart TD
A["Start kdump role"] --> B["Gather ansible_facts"]
B --> C{os_family == 'Suse'?}
C -- Yes --> D{distribution_major_version >= 16?}
C -- No --> H["Use non-Suse crashkernel update path"]
D -- Yes --> E{kdump_reboot_required?}
D -- No --> H
E -- true --> F["Run /usr/sbin/kdumptool commandline -u"]
E -- false --> G["Skip kdumptool crashkernel update"]
F --> I["Evaluate kdump_reboot_required for service start"]
G --> I
H --> I
I --> J{kdump_reboot_required is false?}
J -- Yes --> K["Start __kdump_service (e.g. kdump)"]
J -- No --> L["Skip __kdump_service start (reboot required)"]
K --> M["End kdump role"]
L --> M
Flow diagram for Suse-specific kdump variables in vars/Suse.ymlflowchart LR
A["Host with os_family == 'Suse'"] --> B["Ansible loads vars/Suse.yml"]
B --> C["Set __kdump_packages = [iproute, kexec-tools, kdump, openssh-clients]"]
C --> D["Set __kdump_conf_file = /etc/kdump.conf"]
D --> E["Set __kdump_service = kdump"]
E --> F["Tasks use Suse-specific variables for installation and service management"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The Suse-specific crashkernel update block currently only checks
os_family == 'Suse'and version >= 16; consider also restricting bydistribution(e.g. SLES vs openSUSE variants) so this doesn’t attempt to runkdumptoolon Suse-family systems that may not provide it. - The
kdumptool commandline -utask is forced tochanged_when: true, which will always report a change; if possible, make this idempotent (e.g. by inspecting the current crashkernel configuration or using command output/return codes) to avoid unnecessary handler runs and confusing change reports.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The Suse-specific crashkernel update block currently only checks `os_family == 'Suse'` and version >= 16; consider also restricting by `distribution` (e.g. SLES vs openSUSE variants) so this doesn’t attempt to run `kdumptool` on Suse-family systems that may not provide it.
- The `kdumptool commandline -u` task is forced to `changed_when: true`, which will always report a change; if possible, make this idempotent (e.g. by inspecting the current crashkernel configuration or using command output/return codes) to avoid unnecessary handler runs and confusing change reports.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
tasks/main.yml
Outdated
| - ansible_facts['os_family'] == 'Suse' | ||
| block: | ||
| - name: Use kdumptool commandline -u (SLES 16+/openSUSE) | ||
| command: /usr/sbin/kdumptool commandline -u |
There was a problem hiding this comment.
What package provides kdumptool?
tasks/main.yml
Outdated
| when: | ||
| - kdump_reboot_required | bool | ||
| - ansible_facts['os_family'] == 'Suse' | ||
| block: |
There was a problem hiding this comment.
This block is not needed - you could just have the single task name: Use kdumptool commandline -u (SLES 16+/openSUSE) without the block and add the when conditions above to that task
|
Looks like sle is not using /etc/kdump.conf and puts everything kernel parameters and service config in /etc/sysconfig/kdump, working on it. |
|
@Spectro34 ping? |
|
@Spectro34 ping? @HVSharma12 are you interested in this? |
|
@richm Yes, will take it up on monday +1 |
Enhancement: Add SLE16+ support to kdump role
Reason: SLES systems require different tools for crashkernel configuration. The role currently only supports RedHat family systems using
grubby, which is not available on SLES. SLES 16+ useskdumptool commandline -uto update bootloader crashkernel settings.Result:
vars/Suse.ymlfor packages, config file, and service namekdumptool commandline -utask for SLES 16+ crashkernel updatesIssue Tracker Tickets (Jira or BZ if any): NA
Summary by Sourcery
Add SLES 16+ and openSUSE support to the kdump role and tighten kdump service management around reboot requirements.
New Features:
Enhancements: