Skip to content

Add machine serial to repair queue entries #806

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

Merged
merged 1 commit into from
Jul 11, 2025
Merged

Conversation

chez-shanpu
Copy link
Contributor

@chez-shanpu chez-shanpu commented Jul 11, 2025

This PR adds machine serial number to repair queue.
Changes are below:

  • Added Serial field to RepairQueueEntry struct
  • Updated NewRepairQueueEntry() to accept serial parameter
  • Modified CLI command signature: ckecli repair-queue add OPERATION MACHINE_TYPE ADDRESS [SERIAL]
  • Updated Sabakan integration to pass machine serial when creating repair entries
  • Updated all tests and documentation

The ckecli repair-queue add command now accepts an optional SERIAL parameter for backward compatibility.

# Before
$ ckecli repair-queue add unreachable type1 10.0.0.1
# New
$ ckecli repair-queue add unreachable type1 10.0.0.1 SN123456
# Also works
$ ckecli repair-queue add unreachable type1 10.0.0.1

@chez-shanpu chez-shanpu marked this pull request as ready for review July 11, 2025 04:01
@chez-shanpu chez-shanpu force-pushed the repair-queue-serial branch from 84a9e0b to e991ebd Compare July 11, 2025 04:47
@chez-shanpu chez-shanpu removed the request for review from morimoto-cybozu July 11, 2025 04:55
@chez-shanpu chez-shanpu force-pushed the repair-queue-serial branch from e991ebd to 9ab0351 Compare July 11, 2025 05:02
docs/ckecli.md Outdated

Append a repair request to the repair queue.
The repair target is a machine with an IP address `ADDRESS` and a machine type `MACHINE_TYPE`.
The repair target is a machine with an IP address `ADDRESS`, a machine type `MACHINE_TYPE`, and a serial number `SERIAL`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This description seems misleading.
CKE does not use SERIAL to select a target machine.

Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is not a request for change.
The following is purely a matter of preference.

Because we don't necessarily need SERIAL, we could make it an option.
ckecli repair-queue add OPERATION MACHINE_TYPE ADDRESS --serial SERIAL

We could make it an optional argument.
ckecli repair-queue add OPERATION MACHINE_TYPE ADDRESS [SERIAL]

Because we record SERIAL just for displaying target machine's informational details, we could put it in a comment option.
ckecli repair-queue add OPERATION MACHINE_TYPE ADDRESS --comment "Serial=DEADBEEF"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. Making it an option seems simple and good to me.
I'll fix it that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made it an option and fixed PR's description.

@chez-shanpu chez-shanpu merged commit e40a154 into main Jul 11, 2025
7 checks passed
@chez-shanpu chez-shanpu deleted the repair-queue-serial branch July 11, 2025 08:36
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