Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: avoid possible panic in govc metric commands #2859

Merged
merged 1 commit into from
Jun 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions govc/metric/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,13 @@ func (cmd *info) Run(ctx context.Context, f *flag.FlagSet) error {
seen := make(map[int32]bool)
for i := range all {
id := &all[i]
if seen[id.CounterId] {
info, ok := nc[id.CounterId]
if !ok || seen[id.CounterId] {
continue
}
seen[id.CounterId] = true

names = append(names, nc[id.CounterId].Name())
names = append(names, info.Name())
}
}
}
Expand Down
15 changes: 9 additions & 6 deletions govc/metric/ls.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ func (r *lsResult) Write(w io.Writer) error {

for _, id := range r.MetricList {
if r.cmd.group != "" {
info := r.counters[id.CounterId]
if info.GroupInfo.GetElementDescription().Label != r.cmd.group {
info, ok := r.counters[id.CounterId]
if !ok || info.GroupInfo.GetElementDescription().Label != r.cmd.group {
continue
}
}
Expand All @@ -116,7 +116,10 @@ func (r *lsResult) Write(w io.Writer) error {
}

for _, id := range res {
info := r.counters[id.CounterId]
info, ok := r.counters[id.CounterId]
if !ok {
continue
}

switch {
case r.cmd.long:
Expand Down Expand Up @@ -147,9 +150,9 @@ func (r *lsResult) MarshalJSON() ([]byte, error) {
m := make(map[string]*types.PerfCounterInfo)

for _, id := range r.MetricList {
info := r.counters[id.CounterId]

m[info.Name()] = info
if info, ok := r.counters[id.CounterId]; ok {
m[info.Name()] = info
}
}

return json.Marshal(m)
Expand Down
4 changes: 4 additions & 0 deletions govc/test/metric.bats
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ load test_helper

host=$(govc ls -t HostSystem ./... | head -n 1)
pool=$(govc ls -t ResourcePool ./... | head -n 1)
vm=$(govc ls -t VirtualMachine ./... | head -n 1)

run govc metric.ls "$host"
assert_success
Expand All @@ -22,6 +23,9 @@ load test_helper

run govc metric.ls "$pool"
assert_success

run govc metric.ls "$vm"
assert_success
}

@test "metric.sample" {
Expand Down
22 changes: 17 additions & 5 deletions performance/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,18 @@ func (d groupPerfCounterInfo) Len() int {
func (d groupPerfCounterInfo) Less(i, j int) bool {
ci := d.ids[i].CounterId
cj := d.ids[j].CounterId
gi := d.info[ci].GroupInfo.GetElementDescription()
gj := d.info[cj].GroupInfo.GetElementDescription()

return gi.Key < gj.Key
giKey := "-"
gjKey := "-"

if gi, ok := d.info[ci]; ok {
giKey = gi.GroupInfo.GetElementDescription().Key
}
if gj, ok := d.info[cj]; ok {
gjKey = gj.GroupInfo.GetElementDescription().Key
}

return giKey < gjKey
}

func (d groupPerfCounterInfo) Swap(i, j int) {
Expand Down Expand Up @@ -455,10 +463,14 @@ func (m *Manager) ToMetricSeries(ctx context.Context, series []types.BasePerfEnt

for j := range s.Value {
v := s.Value[j].(*types.PerfMetricIntSeries)
info, ok := counters[v.Id.CounterId]
if !ok {
continue
}

values = append(values, MetricSeries{
Name: counters[v.Id.CounterId].Name(),
unit: counters[v.Id.CounterId].UnitInfo.GetElementDescription().Key,
Name: info.Name(),
unit: info.UnitInfo.GetElementDescription().Key,
Instance: v.Id.Instance,
Value: v.Value,
})
Expand Down
11 changes: 3 additions & 8 deletions simulator/performance_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,7 @@ func (p *PerformanceManager) QueryPerfCounter(ctx *Context, req *types.QueryPerf
body.Res = new(types.QueryPerfCounterResponse)
body.Res.Returnval = make([]types.PerfCounterInfo, len(req.CounterId))
for i, id := range req.CounterId {
if info, ok := p.perfCounterIndex[id]; !ok {
body.Fault_ = Fault("", &types.InvalidArgument{
InvalidProperty: "CounterId",
})
return body
} else {
body.Res.Returnval[i] = info
}
body.Res.Returnval[i] = p.perfCounterIndex[id]
}
return body
}
Expand Down Expand Up @@ -135,6 +128,8 @@ func (p *PerformanceManager) buildAvailablePerfMetricsQueryResponse(ids []types.
r.Returnval = append(r.Returnval, types.PerfMetricId{CounterId: id.CounterId, Instance: id.Instance})
}
}
// Add a CounterId without a corresponding PerfCounterInfo entry. See issue #2835
r.Returnval = append(r.Returnval, types.PerfMetricId{CounterId: 10042})
return r
}

Expand Down