Skip to content

feat: handle mcc volumes in rest/zapi based on state and svm#3672

Merged
Hardikl merged 4 commits into
mainfrom
hl_vol
May 30, 2025
Merged

feat: handle mcc volumes in rest/zapi based on state and svm#3672
Hardikl merged 4 commits into
mainfrom
hl_vol

Conversation

@Hardikl
Copy link
Copy Markdown
Contributor

@Hardikl Hardikl commented May 29, 2025

No description provided.

Comment thread cmd/collectors/rest/plugins/volume/volume.go
Comment thread cmd/collectors/rest/plugins/volume/volume.go
Comment thread cmd/collectors/zapi/plugins/volume/volume.go
@cgrinds cgrinds requested a review from Copilot May 29, 2025 13:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds logic to exclude offline volumes in MC (Multi-Cluster) SVMs from export and validates it in tests for the REST plugin

  • Filter out volumes with state "offline" in SVMs ending with -mc in both ZAPI and REST collectors
  • Introduce TestMCCVolumes unit test to validate exportable flags for the REST plugin

Reviewed Changes

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

File Description
cmd/collectors/zapi/plugins/volume/volume.go Add offline MC-SVM volume check in processAndUpdateVolume
cmd/collectors/rest/plugins/volume/volume.go Add offline MC-SVM volume check in updateVolumeLabels
cmd/collectors/rest/plugins/volume/volume_test.go Add TestMCCVolumes to verify REST plugin exportable logic
Comments suppressed due to low confidence (4)

cmd/collectors/zapi/plugins/volume/volume.go:158

  • No unit test covers the offline MC-SVM exportable logic in the ZAPI plugin; consider adding tests for processAndUpdateVolume to validate this behavior.
if volState == "offline" && strings.HasSuffix(svm, "-mc") {

cmd/collectors/zapi/plugins/volume/volume.go:123

  • [nitpick] Variable volState could be renamed to volumeState or stateLabel for improved clarity.
volState := volume.GetLabel("state")

cmd/collectors/rest/plugins/volume/volume.go:189

  • Missing import of the "strings" package; this reference to strings.HasSuffix will cause a compile error. Add import "strings" at the top.
if volState == "offline" && strings.HasSuffix(svm, "-mc") {

cmd/collectors/zapi/plugins/volume/volume.go:158

  • [nitpick] This offline MC-SVM filter logic also appears in the REST plugin; consider extracting it into a shared helper to reduce duplication.
if volState == "offline" && strings.HasSuffix(svm, "-mc") {

@Hardikl Hardikl merged commit 88642fd into main May 30, 2025
10 checks passed
@Hardikl Hardikl deleted the hl_vol branch May 30, 2025 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants