Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"context"
"encoding/json"
"fmt"
"sync"
"time"

"go.uber.org/fx"
Expand All @@ -29,6 +30,10 @@ const (
componentName = "workloadmeta-cloudfoundry-vm"
)

// getClusterAgentClient is overridden in tests to avoid depending on the
// real cluster agent connection singleton.
var getClusterAgentClient = clusteragent.GetClusterAgentClient

type dependencies struct {
fx.In

Expand All @@ -45,8 +50,9 @@ type collector struct {
gardenUtil cloudfoundry.GardenUtilInterface
nodeName string

dcaClient clusteragent.DCAClientInterface
dcaEnabled bool
dcaClientLock sync.Mutex
dcaClient clusteragent.DCAClientInterface
dcaEnabled bool
}

// NewCollector instantiates a CollectorProvider which can provide a CF container collector
Expand Down Expand Up @@ -108,7 +114,7 @@ func (c *collector) Pull(_ context.Context) error {

var allContainersTags map[string][]string
if dcaClient := c.getDCAClient(); dcaClient != nil {
allContainersTags, err = c.dcaClient.GetCFAppsMetadataForNode(c.nodeName)
allContainersTags, err = dcaClient.GetCFAppsMetadataForNode(c.nodeName)
if err != nil {
log.Debugf("Unable to fetch CF tags from cluster agent, CF tags will be missing, err: %v", err)
}
Expand Down Expand Up @@ -223,6 +229,9 @@ func (c *collector) Pull(_ context.Context) error {
}

func (c *collector) getDCAClient() clusteragent.DCAClientInterface {
c.dcaClientLock.Lock()
defer c.dcaClientLock.Unlock()

if !c.dcaEnabled {
return nil
}
Expand All @@ -231,13 +240,13 @@ func (c *collector) getDCAClient() clusteragent.DCAClientInterface {
return c.dcaClient
}

var err error
c.dcaClient, err = clusteragent.GetClusterAgentClient()
client, err := getClusterAgentClient()
if err != nil {
log.Debugf("Could not initialise the communication with the cluster agent, PCF tags may be missing, err: %v", err)
return nil
}

c.dcaClient = client
return c.dcaClient
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,3 +509,40 @@ func TestPullAppNameWithGardenPropertiesWithoutDCA(t *testing.T) {

assert.Contains(t, container.CollectorTags, "container_name:"+"app-name-1")
}

// TestPullDCAUnreachableTwice reproduces the typed-nil interface trap: when
// clusteragent.GetClusterAgentClient returns a concrete (*DCAClient)(nil)
// alongside an error, assigning it into an interface field before the error
// check poisons the field with a non-nil interface wrapping a nil pointer.
// A second Pull would then see c.dcaClient != nil, skip re-init, and call a
// method on a nil *DCAClient receiver.
func TestPullDCAUnreachableTwice(t *testing.T) {
fakeGardenUtil := FakeGardenUtil{containers: nil}

orig := getClusterAgentClient
getClusterAgentClient = func() (*clusteragent.DCAClient, error) {
// Simulate GetClusterAgentClient's real error path: a nil concrete
// pointer alongside a non-nil error.
return nil, fmt.Errorf("cluster agent unreachable")
}
t.Cleanup(func() { getClusterAgentClient = orig })

workloadmetaStore := fxutil.Test[workloadmetamock.Mock](t, fx.Options(
core.MockBundle(),
workloadmetafxmock.MockModule(workloadmeta.NewParams()),
))

c := collector{
gardenUtil: &fakeGardenUtil,
store: workloadmetaStore,
dcaEnabled: true,
}

require.NoError(t, c.Pull(context.TODO()))
require.Nil(t, c.dcaClient, "first Pull must not poison the dcaClient field on error")

require.NotPanics(t, func() {
require.NoError(t, c.Pull(context.TODO()))
})
require.Nil(t, c.dcaClient, "second Pull must not poison the dcaClient field on error")
}
Loading