Skip to content

fix: Change longhorn-preflight command to longhornctl-local - #552

Open
chattytak wants to merge 1 commit into
longhorn:masterfrom
chattytak:master
Open

fix: Change longhorn-preflight command to longhornctl-local#552
chattytak wants to merge 1 commit into
longhorn:masterfrom
chattytak:master

Conversation

@chattytak

Copy link
Copy Markdown

The command is outdated and not working properly. It needs to be corrected to the current command options.

@chattytak chattytak changed the title Change longhorn-preflight command to longhorn-local fix: Change longhorn-preflight command to longhorn-local Jul 14, 2026
@derekbit

Copy link
Copy Markdown
Member

@chattytak Could you create a BUG ticket in https://github.com/longhorn/longhorn/issues? Thanks.

@chattytak
chattytak force-pushed the master branch 3 times, most recently from 055f6d1 to 0af72f8 Compare July 14, 2026 09:56
@chattytak chattytak changed the title fix: Change longhorn-preflight command to longhorn-local fix: Change longhorn-preflight command to longhornctl-local Jul 14, 2026
@chattytak

Copy link
Copy Markdown
Author

@derekbit
I have created a new issue as instructed.
longhorn/longhorn#13514

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates Longhorn preflight DaemonSet initContainer commands to use the newer longhornctl-local CLI instead of the deprecated longhorn-preflight binary, aligning the manifests with current Longhorn CLI options.

Changes:

  • deploy/install.yaml: Switch initContainer command from longhorn-preflight to longhornctl-local install preflight.
  • deploy/check.yaml: Switch initContainer command to longhornctl-local … with additional subcommands intended to run preflight checks.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
deploy/install.yaml Updates the preflight install initContainer to use longhornctl-local install preflight.
deploy/check.yaml Updates the preflight check initContainer to use longhornctl-local, but currently composes subcommands in a way that appears incorrect.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread deploy/check.yaml

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

deploy/check.yaml:24

  • In this DaemonSet, the initContainer command currently resolves to longhornctl-local install preflight check (because install preflight is followed by the existing check arg). The local CLI subcommands are structured as longhornctl-local check preflight (see cmd/local/subcmd/check.go), while install preflight does not have a check subcommand. This will likely fail at runtime with an unknown command/arg error.

Consider changing the command args to longhornctl-local, check, preflight (and dropping the trailing check).

        command:
        - longhornctl-local
        - install
        - preflight
        - check

@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 3.14%. Comparing base (fba2b75) to head (4aa9396).

Additional details and impacted files
@@          Coverage Diff           @@
##           master    #552   +/-   ##
======================================
  Coverage    3.14%   3.14%           
======================================
  Files          42      42           
  Lines        3629    3629           
======================================
  Hits          114     114           
  Misses       3512    3512           
  Partials        3       3           
Flag Coverage Δ
unittests 3.14% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

deploy/check.yaml:24

  • deploy/check.yaml is intended to run a preflight check, but the updated command runs the installer subcommand (install preflight) and passes check as an extra arg. In cmd/local/subcmd/check.go, the preflight check is implemented as longhornctl-local check preflight, while cmd/local/subcmd/install.go implements longhornctl-local install preflight; passing check after install preflight won’t invoke the checker logic.
        - longhornctl-local
        - install
        - preflight
        - check

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@mergify

mergify Bot commented Aug 28, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@c3y1huang
c3y1huang self-requested a review August 28, 2026 03:05

@c3y1huang c3y1huang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The change LGTM, but should cleanup the commit history before merge. The fix-on-fix shouldn't land. Defering my approval until then to avoid triggering auto-merge.

Signed-off-by: chattytak <94992438+chattytak@users.noreply.github.com>
@chattytak

Copy link
Copy Markdown
Author

The change LGTM, but should cleanup the commit history before merge. The fix-on-fix shouldn't land. Defering my approval until then to avoid triggering auto-merge.

I have summarized the commits.

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.

4 participants