Skip to content

Conversation

@ricolin
Copy link
Member

@ricolin ricolin commented Dec 19, 2025

This PR introduce:

  • Add common kernal params: Add nofb, console=ttyS0,115200 to ironic kernel_append_params
  • Fix flat ironic network config for multi-tenant network
  • Allow using system scope authentication from OS-CLOUD

@ricolin
Copy link
Member Author

ricolin commented Jan 27, 2026

@claude please review and suggest fix for this pr

@claude
Copy link

claude bot commented Jan 27, 2026

Claude finished @ricolin's task in 1m 45s —— View job


PR Review Complete

  • Gather context and read changed files
  • Analyze changes for bugs, security issues, and best practices
  • Check release notes format and content
  • Provide review feedback with suggestions

Summary

This PR introduces three improvements for Ironic:

  1. Flat network fix: Conditionally omits provider_segmentation_id for flat networks
  2. Kernel parameters: Adds nofb and console=ttyS0,115200 to both PXE and Redfish configurations
  3. System scope authentication: Adds a new system cloud configuration for system-scoped OpenStack operations

Review Findings

1. Flat Network Fix (roles/ironic/tasks/network/create.yml:23-25)

The fix correctly uses Ansible's ternary filter to omit the segmentation ID for flat networks:

provider_segmentation_id: >-
  {{ (ironic_bare_metal_network_provider_network_type == 'flat')
     | ternary(omit, ironic_bare_metal_network_provider_segmentation_id) }}

Assessment: This is the correct approach. Flat networks in Neutron don't use segmentation IDs (unlike VLAN/VXLAN), and passing one would cause an error.

2. Kernel Parameters (roles/ironic/vars/main.yml:69-71)

Added nofb and console=ttyS0,115200 to kernel append params for both PXE and Redfish:

pxe:
  kernel_append_params: "nofb ipa-insecure=true console=ttyS0,115200 systemd.journald.forward_to_console=yes"
redfish:
  kernel_append_params: "nofb ipa-insecure=true console=ttyS0,115200 systemd.journald.forward_to_console=yes"

Assessment: Good additions for bare metal deployments:

  • nofb disables the framebuffer, which is useful for headless servers
  • console=ttyS0,115200 enables serial console output at 115200 baud, essential for BMC/IPMI console access

3. System Scope Authentication (roles/openstacksdk/templates/clouds.yaml.j2:20-36)

Adds a new system cloud configuration with system_scope: all.

Assessment: Useful for Ironic operations that require system-scoped tokens (e.g., openstack --os-cloud system baremetal node list).


Issues Found

Issue 1: Release notes missing kernel parameters change

The release notes mention the flat network fix and system scope authentication, but do not mention the kernel parameters change (nofb, console=ttyS0,115200). This is a user-facing change that should be documented.

Suggested fix - Add to releasenotes/notes/ironic-improve-flat-396db22bb9ecca15.yaml:

other:
  - |
    Add support for system scope in the OpenStack command-line client and SDKs to improve the bare metal management experience.
    Users can enable system scope authentication by setting ``OS_CLOUD=system``.
  - |
    Add common kernel parameters (``nofb``, ``console=ttyS0,115200``) to the bare metal service PXE and Redfish configurations for improved serial console support.

Fix this →

Issue 2: Commit message typo

The commit message "Add common kernal params" has a typo: "kernal" should be "kernel". This is minor since it will be squash-merged, but worth noting.


Optional Improvements

  1. Consider parameterizing console settings: The baud rate 115200 is hardcoded. Some environments may need different settings (e.g., 9600). Consider making this configurable via a variable if flexibility is needed in the future.

  2. DRY principle for kernel params: The kernel_append_params value is duplicated between pxe and redfish sections. Consider using a YAML anchor or variable to avoid duplication:

    _ironic_kernel_append_params: "nofb ipa-insecure=true console=ttyS0,115200 systemd.journald.forward_to_console=yes"

    Then reference it in both places. However, this is a minor improvement and may not be worth changing if the values might diverge in the future.


Verdict

The PR looks good overall. The main actionable item is adding the missing release note entry for the kernel parameters change. The code changes are correct and follow the existing patterns in the codebase.

This is helpful when we operating with ironic
We can trigger system scope authentication when run baremetal commands
like
`openstack --os-cloud system baremetal node list`

Change-Id: I31cbf09bd17d98ef6b9d283a4f155b14b4768237
Signed-off-by: ricolin <rlin@vexxhost.com>
Change-Id: I0d4c3cb97e1c90a9beed4f672c3df08c30b2f93b
Signed-off-by: ricolin <rlin@vexxhost.com>
Add nofb, console=ttyS0,115200 to ironic kernel_append_params

Change-Id: I63fd52c5bea9630b136511e4d6243985304699a4
Signed-off-by: ricolin <rlin@vexxhost.com>
Change-Id: I04e4509d9e20db3e7f1ffb03b57e2ed93dbb6d63
Signed-off-by: ricolin <rlin@vexxhost.com>
Change-Id: I5f308cbb530df80045456119692d344e9956454a
Signed-off-by: ricolin <rlin@vexxhost.com>
Change-Id: I2f8168142c2262488621c4dd3b4860201afb27a2
Signed-off-by: ricolin <rlin@vexxhost.com>
@ricolin ricolin force-pushed the rlin-improve-ironic-fix-flat branch from ade0d36 to 28adeb9 Compare February 11, 2026 01:42
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.

1 participant