refactor: update initiator.go#265
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #265 +/- ##
========================================
+ Coverage 0.00% 2.73% +2.73%
========================================
Files 38 38
Lines 4919 5017 +98
========================================
+ Hits 0 137 +137
+ Misses 4919 4870 -49
- Partials 0 10 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors initiator retry behavior to use github.com/avast/retry-go/v4, updates related log messages, and adds test coverage around initiator validation and a few helper behaviors.
Changes:
- Add and vendor
github.com/avast/retry-go/v4and updatego.mod/go.sumaccordingly. - Refactor NVMe/TCP retry loops (connect, disconnect wait, device-info loading, device creation validation) to use
retry-gowith fixed delays and retry logging. - Add/expand initiator unit tests and add additional SPDK client log messages.
Reviewed changes
Copilot reviewed 4 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| .gitignore | Ignore Dockerfile.dapper[0-9]*. |
| go.mod | Add github.com/avast/retry-go/v4 v4.7.0. |
| go.sum | Add checksums for retry-go and update other sums. |
| pkg/initiator/initiator.go | Replace manual retry loops with retry-go; update NVMe/TCP log messages; adjust ublk startup flow and disk creation validation. |
| pkg/initiator/initiator_test.go | Add unit tests for initiator validation and selected helper behaviors. |
| pkg/spdk/client/advanced.go | Add Info-level logs around expose/unexpose steps. |
| vendor/modules.txt | Record new vendored module entry. |
| vendor/github.com/avast/retry-go/v4/retry.go | Vendored retry-go implementation. |
| vendor/github.com/avast/retry-go/v4/options.go | Vendored retry-go options/config implementation. |
| vendor/github.com/avast/retry-go/v4/README.md | Vendored upstream documentation. |
| vendor/github.com/avast/retry-go/v4/LICENSE | Vendored upstream license. |
| vendor/github.com/avast/retry-go/v4/VERSION | Vendored upstream version marker. |
| vendor/github.com/avast/retry-go/v4/Makefile | Vendored upstream build tooling. |
| vendor/github.com/avast/retry-go/v4/.gitignore | Vendored upstream ignore file. |
| vendor/github.com/avast/retry-go/v4/.godocdown.tmpl | Vendored upstream doc template. |
| vendor/github.com/avast/retry-go/v4/generic.txt | Vendored upstream benchmark output. |
| vendor/github.com/avast/retry-go/v4/current.txt | Vendored upstream benchmark output. |
Comments suppressed due to low confidence (1)
pkg/initiator/initiator.go:506
StartUblkInitiatorno longer initializes the ublk target (previously done viaUblkCreateTarget). If the SPDK ublk target hasn't already been created elsewhere, subsequentUblkGetDisks/UblkStartDiskcalls can fail. Consider ensuring the target exists here (create-once semantics, ignoring an "already exists" response if applicable) to preserve prior behavior.
ublkDeviceList, err := spdkClient.UblkGetDisks(0)
if err != nil {
return false, err
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
713508b to
97f2c8c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 17 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Signed-off-by: Derek Su <derek.su@suse.com>
Longhorn 7124 Signed-off-by: Derek Su <derek.su@suse.com>
1. Update log messages 2. Reimplement the retry logics using avast/retry-go Longhorn 7124 Signed-off-by: Derek Su <derek.su@suse.com>
97f2c8c to
ad0f7da
Compare
Persist the discovered SubsystemNQN immediately after a successful discover-and-connect operation. This keeps initiator state aligned with the actual connected NVMe/TCP target even when controllerName validation fails afterwards. Without this update, reload/load/cleanup paths may use a stale SubsystemNQN and fail to locate or disconnect the correct NVMe device. Longhorn 7124 Signed-off-by: Derek Su <derek.su@suse.com>
ad0f7da to
1f212a7
Compare
Which issue(s) this PR fixes:
Issue longhorn/longhorn#7124
What this PR does / why we need it:
Special notes for your reviewer:
Additional documentation or context