Skip to content

Commit

Permalink
fix(hnsstats): improve error handling and logging for VFP port counte…
Browse files Browse the repository at this point in the history
…rs (#1002)

# Description

This pull request includes several improvements and error handling
enhancements in the `hnsstats` package for Windows. The changes focus on
better error reporting and wrapping errors with more context.

### Error Handling Improvements:

*
[`pkg/plugin/windows/hnsstats/hnsstats_windows.go`](diffhunk://#diff-f7aff15850299a4daebae31d62fbc55c0085872c1e216801367d51d73d8909aaL127-R131):
Added a check to ensure `portguid` is not empty and logs an error if it
is not found.
*
[`pkg/plugin/windows/hnsstats/hnsstats_windows.go`](diffhunk://#diff-f7aff15850299a4daebae31d62fbc55c0085872c1e216801367d51d73d8909aaL138-R142):
Enhanced the error log message to include `portguid` when unable to find
VFP port counters.

### Dependency and Error Wrapping:

*
[`pkg/plugin/windows/hnsstats/vfp_counters_windows.go`](diffhunk://#diff-ee7db77fb3268bc4192a25ed95de63cfc3be24044a55e739dc59b7e6d63e8095R10-R11):
Imported the `github.com/pkg/errors` package for better error wrapping.
*
[`pkg/plugin/windows/hnsstats/vfp_counters_windows.go`](diffhunk://#diff-ee7db77fb3268bc4192a25ed95de63cfc3be24044a55e739dc59b7e6d63e8095L154-R163):
Wrapped errors with additional context when running `vfpctrl` commands.

## Checklist

- [X] I have read the [contributing
documentation](https://retina.sh/docs/contributing).
- [X] I signed and signed-off the commits (`git commit -S -s ...`). See
[this
documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
on signing commits.
- [X] I have correctly attributed the author(s) of the code.
- [X] I have tested the changes locally.
- [X] I have followed the project's style guidelines.
- [X] I have updated the documentation, if necessary.
- [X] I have added tests, if applicable.

---

Please refer to the [CONTRIBUTING.md](../CONTRIBUTING.md) file for more
information on how to contribute to this project.
  • Loading branch information
ritwikranjan authored Nov 15, 2024
1 parent a255963 commit adcba81
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
8 changes: 6 additions & 2 deletions pkg/plugin/windows/hnsstats/hnsstats_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,11 @@ func pullHnsStats(ctx context.Context, h *hnsstats) error {
// h.l.Info(hnsStatsData.String())

// Get VFP port counters for matching port (MAC address of endpoint as the key)
portguid := kv[mac]
portguid, ok := kv[mac]
if !ok || len(portguid) == 0 {
h.l.Error("port is either empty of not found", zap.String(zapMACField, mac))
continue
}
if countersRaw, err := getVfpPortCountersRaw(portguid); len(portguid) > 0 && err == nil {
if vfpcounters, err := parseVfpPortCounters(countersRaw); err == nil {
// Attach VFP port counters
Expand All @@ -135,7 +139,7 @@ func pullHnsStats(ctx context.Context, h *hnsstats) error {
h.l.Error("Unable to parse VFP port counters", zap.String(zapPortField, portguid), zap.Error(err))
}
} else {
h.l.Error("Unable to find VFP port counters", zap.String(zapMACField, mac), zap.Error(err))
h.l.Error("Unable to find VFP port counters", zap.String(zapMACField, mac), zap.String(zapPortField, portguid), zap.Error(err))
}

notifyHnsStats(h, hnsStatsData)
Expand Down
7 changes: 5 additions & 2 deletions pkg/plugin/windows/hnsstats/vfp_counters_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"os/exec"
"strconv"
"strings"

"github.com/pkg/errors"
)

// IN/OUT Direction of vSwitch VFP port
Expand Down Expand Up @@ -151,13 +153,14 @@ func getVfpPortCountersRaw(portGUID string) (string, error) {
cmd := exec.Command("cmd", "/c", vfpCmd)
out, err := cmd.Output()

return string(out), err
return string(out), errors.Wrap(err, "errored while running vfpctrl /get-port-counter")
}

// TODO: Remove this once Resources.Allocators.EndpointPortGuid gets added to hcsshim Endpoint struct
// Lists all vSwitch ports
func listvPorts() ([]byte, error) {
return exec.Command("cmd", "/c", "vfpctrl /list-vmswitch-port").CombinedOutput()
out, err := exec.Command("cmd", "/c", "vfpctrl /list-vmswitch-port").CombinedOutput()
return out, errors.Wrap(err, "errored while running vfpctrl /list-vmswitch-port")
}

// TODO: Remove this once Resources.Allocators.EndpointPortGuid gets added to hcsshim Endpoint struct
Expand Down

0 comments on commit adcba81

Please sign in to comment.