PMK-6388: fix prep-node failure with segmentation fault

[mridulgain]

ISSUE(S):

https://platform9.atlassian.net/browse/PMK-6388

SUMMARY

Root cause: Error was not reported back correctly.
2023-09-06T17:49:09.0543Z DEBUG Running command https_proxy=http://qcwebproxylb.juniper.net:3128 bash "-c" "grep host_id /etc/pf9/host_id.conf | cut -d '=' -f2"stdout:stderr:grep: /etc/pf9/host_id.conf: No such file or directory
In scenarios when "file is not found", it's expected to receive a non nil error.
When piping commands, the status code reported by grep is overwritten by that of cut

mridulgain@HXJN3GQY1WFV pf9ctl % grep host_id /etc/pf9/host_id.conf| cut -d '=' -f2
grep: /etc/pf9/host_id.conf: No such file or directory
mridulgain@HXJN3GQY1WFV pf9ctl % echo $?
0

Which leads to a nil err & despite of having error handling it fails to gracefully handle it and later panics with nil pointer dereference due to empty hostID inside AuthorizeHost function.

Suggested fix:

use && instead of | operator to achieve same behavior with correct exit status.

mridulgain@HXJN3GQY1WFV pf9ctl % grep host_id /etc/pf9/host_id.conf && cut -d '=' -f2`
grep: /etc/pf9/host_id.conf: No such file or directory
mridulgain@HXJN3GQY1WFV pf9ctl % echo $?
2

ISSUE TYPE

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that may cause existing functionality to not work as expected)
• This change requires a documentation update

IMPACTED FEATURES/COMPONENTS:

prepNode

RELATED ISSUE(S):

DEPENDS ON:

TESTING DONE

Automated

Manual

Prior to the fix: the prep-node workflow was going into panic, leading to segmentation fault.
After the fix: It gracefully handles & reports the issue.

2025-02-20T06:41:59.2203Z       FATAL
Failed to prepare node. Error: Unable to fetch host ID. exit status 2. See /home/ubuntu/pf9/log/pf9ctl-20250220.log or use --verbose for logs

Reviewers

Summary by Bito

Fixed a bug in command pipeline error handling by replacing the pipe operator (|) with logical AND (&&) when reading host_id from /etc/pf9/host_id.conf. This change ensures proper error reporting from grep commands and prevents potential nil pointer dereference panics.

Unit tests added: False

Estimated effort to review (1-5, lower is better): 1

[bito-code-review]

Code Review Agent Run #a50a7b

Actionable Suggestions - 1

• pkg/pmk/node.go - 1

  • Consider pipe instead of AND operator · Line 147-147

Review Details

• Files reviewed - 1 · Commit Range: c0edddd..c0edddd

  • pkg/pmk/node.go
• Files skipped - 0
• Tools

  • Golangci-lint (Linter) - ✖︎ Failed
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

AI Code Review powered by

[bito-code-review]

Changelist by Bito

This pull request implements the following key changes.

Key Change	Files Impacted
Bug Fix - Fix Command Pipeline Error Handling	- node.go - Modified grep command to use && instead of pipe for proper error status reporting