Skip to content

Commit

Permalink
[receiver/vcenter] Fixes Client Virtual App Call Error When No Virtua…
Browse files Browse the repository at this point in the history
…l Apps (#33102)

**Description:** <Describe what has changed.>
Virtual Apps might not exist in an environment. Currently a `NotFound`
error for the internal client is thrown when this condition occurs. I
modified this to simply return an empty array, because this should
simply mean that it did not find any Virtual Apps. I also added more
client unit tests to test for these types of conditions.

**Link to tracking Issue:** <Issue number if applicable>
#33073 (loosely tied to this. At the very least this issue made me
discover this problem)

**Testing:** <Describe what testing was performed and which tests were
added.>
Added new unit tests which showed the error. Fix caused the new unit
test to pass.

**Documentation:** <Describe the documentation added.>
N/A
  • Loading branch information
StefanKurek authored May 16, 2024
1 parent e783923 commit dbdc463
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 4 deletions.
27 changes: 27 additions & 0 deletions .chloggen/fix_vcenter-client-errors-on-empty-vapps.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: vcenterreceiver

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "vcenterreceiver client no longer returns error if no Virtual Apps are found."

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [33073]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [user]
2 changes: 1 addition & 1 deletion .chloggen/fix_vcenter-datastore-attributes.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ subtext: |
If there were multiple Clusters, Datastore metrics were being repeated under Resources differentiated with a
`vcenter.cluster.name` resource attribute. In the same vein, if there were standalone Hosts, in addition to
clusters the metrics would be repeated under a Resource without the `vcenter.cluster.name` attribute. Now there
will only be a single set of metrics for one Datastore (as there should be, as Datastores don't be long to
will only be a single set of metrics for one Datastore (as there should be, as Datastores don't belong to
Clusters).
# If your change doesn't affect end users or the exported elements of any package,
Expand Down
12 changes: 9 additions & 3 deletions receiver/vcenterreceiver/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package vcenterreceiver // import "github.com/open-telemetry/opentelemetry-colle

import (
"context"
"errors"
"fmt"
"net/url"

Expand Down Expand Up @@ -236,11 +237,16 @@ func (vc *vcenterClient) ResourcePoolInventoryListObjects(ctx context.Context) (
// VAppInventoryListObjects returns the vApps (with populated InventoryLists) of the vSphere SDK
func (vc *vcenterClient) VAppInventoryListObjects(ctx context.Context) ([]*object.VirtualApp, error) {
vApps, err := vc.finder.VirtualAppList(ctx, "*")
if err != nil {
return nil, fmt.Errorf("unable to retrieve vApps with InventoryLists: %w", err)
if err == nil {
return vApps, nil
}

var notFoundErr *find.NotFoundError
if errors.As(err, &notFoundErr) {
return []*object.VirtualApp{}, nil
}

return vApps, nil
return nil, fmt.Errorf("unable to retrieve vApps with InventoryLists: %w", err)
}

// PerfMetricsQueryResult contains performance metric related data
Expand Down
91 changes: 91 additions & 0 deletions receiver/vcenterreceiver/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,26 @@ func TestDatastores(t *testing.T) {
})
}

func TestEmptyDatastores(t *testing.T) {
vpx := simulator.VPX()
vpx.Datastore = 0
vpx.Machine = 0
simulator.Test(func(ctx context.Context, c *vim25.Client) {
finder := find.NewFinder(c)
vm := view.NewManager(c)
client := vcenterClient{
vimDriver: c,
finder: finder,
vm: vm,
}
dc, err := finder.DefaultDatacenter(ctx)
require.NoError(t, err)
dss, err := client.Datastores(ctx, dc.Reference())
require.NoError(t, err)
require.Empty(t, dss, 0)
}, vpx)
}

func TestComputeResources(t *testing.T) {
simulator.Test(func(ctx context.Context, c *vim25.Client) {
finder := find.NewFinder(c)
Expand All @@ -68,6 +88,24 @@ func TestComputeResources(t *testing.T) {
})
}

func TestComputeResourcesWithStandalone(t *testing.T) {
esx := simulator.ESX()
simulator.Test(func(ctx context.Context, c *vim25.Client) {
finder := find.NewFinder(c)
vm := view.NewManager(c)
client := vcenterClient{
vimDriver: c,
finder: finder,
vm: vm,
}
dc, err := finder.DefaultDatacenter(ctx)
require.NoError(t, err)
crs, err := client.ComputeResources(ctx, dc.Reference())
require.NoError(t, err)
require.NotEmpty(t, crs, 0)
}, esx)
}

func TestHostSystems(t *testing.T) {
simulator.Test(func(ctx context.Context, c *vim25.Client) {
finder := find.NewFinder(c)
Expand All @@ -85,6 +123,27 @@ func TestHostSystems(t *testing.T) {
})
}

func TestEmptyHostSystems(t *testing.T) {
vpx := simulator.VPX()
vpx.Host = 0
vpx.ClusterHost = 0
vpx.Machine = 0
simulator.Test(func(ctx context.Context, c *vim25.Client) {
finder := find.NewFinder(c)
vm := view.NewManager(c)
client := vcenterClient{
vimDriver: c,
finder: finder,
vm: vm,
}
dc, err := finder.DefaultDatacenter(ctx)
require.NoError(t, err)
hss, err := client.HostSystems(ctx, dc.Reference())
require.NoError(t, err)
require.Empty(t, hss, 0)
}, vpx)
}

func TestResourcePools(t *testing.T) {
simulator.Test(func(ctx context.Context, c *vim25.Client) {
finder := find.NewFinder(c)
Expand Down Expand Up @@ -119,6 +178,25 @@ func TestVMs(t *testing.T) {
})
}

func TestEmptyVMs(t *testing.T) {
vpx := simulator.VPX()
vpx.Machine = 0
simulator.Test(func(ctx context.Context, c *vim25.Client) {
finder := find.NewFinder(c)
vm := view.NewManager(c)
client := vcenterClient{
vimDriver: c,
finder: finder,
vm: vm,
}
dc, err := finder.DefaultDatacenter(ctx)
require.NoError(t, err)
vms, err := client.VMs(ctx, dc.Reference())
require.NoError(t, err)
require.Empty(t, vms, 0)
}, vpx)
}

func TestPerfMetricsQuery(t *testing.T) {
esx := simulator.ESX()
simulator.Test(func(ctx context.Context, c *vim25.Client) {
Expand Down Expand Up @@ -172,6 +250,19 @@ func TestVAppInventoryListObjects(t *testing.T) {
}, vpx)
}

func TestEmptyVAppInventoryListObjects(t *testing.T) {
simulator.Test(func(ctx context.Context, c *vim25.Client) {
finder := find.NewFinder(c)
client := vcenterClient{
vimDriver: c,
finder: finder,
}
vApps, err := client.VAppInventoryListObjects(ctx)
require.NoError(t, err)
require.Empty(t, vApps, 0)
})
}

func TestSessionReestablish(t *testing.T) {
simulator.Test(func(ctx context.Context, c *vim25.Client) {
sm := session.NewManager(c)
Expand Down

0 comments on commit dbdc463

Please sign in to comment.