fix: avoid possible panic in govc metric commands

Newer versions of vCenter may return a CounterId via QueryAvailablePerfMetric()
which does not have a corresponding PerfCounterInfo entry in PerformanceManager's
perfCounter property. This results in a panic by a few of the govc metric commands
and any application using performance.Manager.AvailableMetric with Sort enabled.

While this may be a bug in vCenter, it is included in some recent release versions.
This workaround avoids the panic by ignoring any CounterId w/o a PerfCounterInfo.

Also updated vcsim's PerformanceManager.QueryCounter to match real vCenter's behavior.
Where null is returned for an unknown CounterId.

Closes vmware#2835

govc/metric/info.go

@@ -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())
 			}
 		}
 	}

govc/metric/ls.go

@@ -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
 			}
 		}
@@ -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:
@@ -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)

govc/test/metric.bats

@@ -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
@@ -22,6 +23,9 @@ load test_helper
 
   run govc metric.ls "$pool"
   assert_success
+
+  run govc metric.ls "$vm"
+  assert_success
 }
 
 @test "metric.sample" {

performance/manager.go

@@ -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) {
@@ -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,
 			})

simulator/performance_manager.go

@@ -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
 }
@@ -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
 }