Move datasource/host to pkg (#1182)

sean-breen · dhurley · web-flow · commit 013487f40499 · 2025-08-20T12:24:42.000+01:00
* move internal/datasource/host to pkg

* update fakes

* warn when creating grpc connection

* fix lint errors

* Update error handling

* update comment, move methods on Info together

* fix import paths

* fix lint issue

---------

Co-authored-by: Donal Hurley &lt;d.hurley@f5.com&gt;
diff --git a/go.mod b/go.mod
@@ -78,7 +78,6 @@ require (
 	go.uber.org/multierr v1.11.0
 	go.uber.org/zap v1.27.0
 	golang.org/x/mod v0.23.0
-	golang.org/x/sync v0.13.0
 	google.golang.org/protobuf v1.36.6
 )
 
@@ -292,6 +291,7 @@ require (
 	go.opentelemetry.io/proto/otlp v1.5.0 // indirect
 	golang.org/x/arch v0.12.0 // indirect
 	golang.org/x/exp v0.0.0-20250210185358-939b2ce775ac // indirect
+	golang.org/x/sync v0.13.0 // indirect
 	golang.org/x/tools v0.30.0 // indirect
 	gonum.org/v1/gonum v0.16.0 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20250303144028-a0af3efb3deb // indirect
diff --git a/internal/config/config.go b/internal/config/config.go
@@ -20,8 +20,9 @@ import (
 	"strings"
 	"time"
 
+	"github.com/nginx/agent/v3/pkg/host"
+
 	"github.com/nginx/agent/v3/internal/datasource/file"
-	"github.com/nginx/agent/v3/internal/datasource/host"
 	"github.com/nginx/agent/v3/internal/logger"
 
 	"github.com/goccy/go-yaml"
@@ -285,7 +286,11 @@ func addDefaultProcessors(collector *Collector) {
 }
 
 func addDefaultHostMetricsReceiver(collector *Collector) {
-	if host.NewInfo().IsContainer() {
+	isContainer, err := host.NewInfo().IsContainer()
+	if err != nil {
+		slog.Debug("No container information found", "error", err)
+	}
+	if isContainer {
 		addDefaultContainerHostMetricsReceiver(collector)
 	} else {
 		addDefaultVMHostMetricsReceiver(collector)
diff --git a/internal/datasource/nginx/process.go b/internal/datasource/nginx/process.go
@@ -13,8 +13,8 @@ import (
 	"regexp"
 	"strings"
 
-	"github.com/nginx/agent/v3/internal/datasource/host/exec"
 	"github.com/nginx/agent/v3/internal/model"
+	"github.com/nginx/agent/v3/pkg/host/exec"
 	"github.com/nginx/agent/v3/pkg/nginxprocess"
 )
 
diff --git a/internal/datasource/nginx/process_test.go b/internal/datasource/nginx/process_test.go
@@ -11,7 +11,7 @@ import (
 	"errors"
 	"testing"
 
-	"github.com/nginx/agent/v3/internal/datasource/host/exec/execfakes"
+	"github.com/nginx/agent/v3/pkg/host/exec/execfakes"
 	"github.com/stretchr/testify/assert"
 )
 
diff --git a/internal/grpc/grpc.go b/internal/grpc/grpc.go
@@ -16,6 +16,8 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/nginx/agent/v3/pkg/host"
+
 	"github.com/nginx/agent/v3/internal/datasource/file"
 
 	"github.com/cenkalti/backoff/v4"
@@ -27,7 +29,6 @@ import (
 
 	mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1"
 	"github.com/nginx/agent/v3/internal/config"
-	"github.com/nginx/agent/v3/internal/datasource/host"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/keepalive"
@@ -87,10 +88,13 @@ func NewGrpcConnection(ctx context.Context, agentConfig *config.Config,
 
 	slog.InfoContext(ctx, "Dialing grpc server", "server_addr", serverAddr)
 
+	var err error
 	info := host.NewInfo()
-	resourceID := info.ResourceID(ctx)
+	resourceID, err := info.ResourceID(ctx)
+	if err != nil {
+		slog.WarnContext(ctx, "Failed to get ResourceID from host info", "error", err.Error())
+	}
 
-	var err error
 	grpcConnection.mutex.Lock()
 	grpcConnection.conn, err = grpc.NewClient(serverAddr, DialOptions(agentConfig, commandConfig, resourceID)...)
 	grpcConnection.mutex.Unlock()
diff --git a/internal/resource/nginx_instance_operator.go b/internal/resource/nginx_instance_operator.go
@@ -16,9 +16,10 @@ import (
 	"github.com/nginx/agent/v3/internal/backoff"
 	"github.com/nginx/agent/v3/pkg/nginxprocess"
 
+	"github.com/nginx/agent/v3/pkg/host/exec"
+
 	mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1"
 	"github.com/nginx/agent/v3/internal/config"
-	"github.com/nginx/agent/v3/internal/datasource/host/exec"
 )
 
 type NginxInstanceOperator struct {
diff --git a/internal/resource/nginx_instance_operator_test.go b/internal/resource/nginx_instance_operator_test.go
@@ -15,15 +15,13 @@ import (
 	"testing"
 	"time"
 
+	"github.com/nginx/agent/v3/internal/config"
 	"github.com/nginx/agent/v3/internal/resource/resourcefakes"
-
-	"github.com/nginx/agent/v3/test/stub"
-
 	"github.com/nginx/agent/v3/pkg/nginxprocess"
+	"github.com/nginx/agent/v3/test/stub"
 
-	"github.com/nginx/agent/v3/internal/config"
+	"github.com/nginx/agent/v3/pkg/host/exec/execfakes"
 
-	"github.com/nginx/agent/v3/internal/datasource/host/exec/execfakes"
 	"github.com/nginx/agent/v3/test/helpers"
 	"github.com/nginx/agent/v3/test/protos"
 	"github.com/nginx/agent/v3/test/types"
diff --git a/internal/resource/nginx_instance_process_operator.go b/internal/resource/nginx_instance_process_operator.go
@@ -10,8 +10,8 @@ import (
 	"errors"
 	"log/slog"
 
-	"github.com/nginx/agent/v3/internal/datasource/host/exec"
 	"github.com/nginx/agent/v3/internal/datasource/nginx"
+	"github.com/nginx/agent/v3/pkg/host/exec"
 	"github.com/nginx/agent/v3/pkg/id"
 
 	"github.com/nginx/agent/v3/pkg/nginxprocess"
diff --git a/internal/resource/nginx_instance_process_operator_test.go b/internal/resource/nginx_instance_process_operator_test.go
@@ -13,7 +13,7 @@ import (
 	"testing"
 	"time"
 
-	"github.com/nginx/agent/v3/internal/datasource/host/exec/execfakes"
+	"github.com/nginx/agent/v3/pkg/host/exec/execfakes"
 	"github.com/nginx/agent/v3/pkg/nginxprocess"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
diff --git a/internal/resource/resource_service.go b/internal/resource/resource_service.go
@@ -19,10 +19,11 @@ import (
 	"strings"
 	"sync"
 
-	"github.com/nginx/agent/v3/internal/datasource/host/exec"
-
+	"github.com/nginx/agent/v3/pkg/host/exec"
 	"github.com/nginx/agent/v3/pkg/nginxprocess"
 
+	"github.com/nginx/agent/v3/pkg/host"
+
 	parser "github.com/nginx/agent/v3/internal/datasource/config"
 	datasource "github.com/nginx/agent/v3/internal/datasource/proto"
 	"github.com/nginx/agent/v3/internal/model"
@@ -34,8 +35,6 @@ import (
 
 	"github.com/nginx/agent/v3/internal/config"
 
-	"github.com/nginx/agent/v3/internal/datasource/host"
-
 	mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1"
 )
 
@@ -398,12 +397,25 @@ func (r *ResourceService) updateResourceInfo(ctx context.Context) {
 	r.resourceMutex.Lock()
 	defer r.resourceMutex.Unlock()
 
-	if r.info.IsContainer() {
-		r.resource.Info = r.info.ContainerInfo(ctx)
+	isContainer, err := r.info.IsContainer()
+	if err != nil {
+		slog.WarnContext(ctx, "Failed to check if resource is container", "error", err)
+	}
+
+	if isContainer {
+		r.resource.Info, err = r.info.ContainerInfo(ctx)
+		if err != nil {
+			slog.ErrorContext(ctx, "Failed to get container info", "error", err)
+			return
+		}
 		r.resource.ResourceId = r.resource.GetContainerInfo().GetContainerId()
 		r.resource.Instances = []*mpi.Instance{}
 	} else {
-		r.resource.Info = r.info.HostInfo(ctx)
+		r.resource.Info, err = r.info.HostInfo(ctx)
+		if err != nil {
+			slog.ErrorContext(ctx, "Failed to get host info", "error", err)
+			return
+		}
 		r.resource.ResourceId = r.resource.GetHostInfo().GetHostId()
 		r.resource.Instances = []*mpi.Instance{}
 	}
diff --git a/internal/resource/resource_service_test.go b/internal/resource/resource_service_test.go
@@ -13,6 +13,8 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/nginx/agent/v3/pkg/host/hostfakes"
+
 	"github.com/nginx/agent/v3/internal/model"
 	"github.com/nginx/agent/v3/internal/watcher/instance/instancefakes"
 
@@ -23,8 +25,6 @@ import (
 	"github.com/nginx/agent/v3/internal/resource/resourcefakes"
 	"github.com/nginx/agent/v3/test/types"
 
-	"github.com/nginx/agent/v3/internal/datasource/host/hostfakes"
-
 	"github.com/nginx/agent/v3/api/grpc/mpi/v1"
 	"github.com/nginx/agent/v3/test/protos"
 	"github.com/stretchr/testify/assert"
@@ -211,17 +211,17 @@ func TestResourceService_GetResource(t *testing.T) {
 			mockInfo.ContainerInfoReturns(
 				&v1.Resource_ContainerInfo{
 					ContainerInfo: tc.expectedResource.GetContainerInfo(),
-				},
+				}, nil,
 			)
 		} else {
 			mockInfo.HostInfoReturns(
 				&v1.Resource_HostInfo{
 					HostInfo: tc.expectedResource.GetHostInfo(),
-				},
+				}, nil,
 			)
 		}
 
-		mockInfo.IsContainerReturns(tc.isContainer)
+		mockInfo.IsContainerReturns(tc.isContainer, nil)
 
 		resourceService := NewResourceService(ctx, types.AgentConfig())
 		resourceService.info = mockInfo
diff --git a/internal/resource/resourcefakes/fake_process_operator.go b/internal/resource/resourcefakes/fake_process_operator.go
diff --git a/internal/watcher/health/nginx_health_watcher_operator.go b/internal/watcher/health/nginx_health_watcher_operator.go
@@ -9,8 +9,9 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/nginx/agent/v3/pkg/host/exec"
+
 	mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1"
-	"github.com/nginx/agent/v3/internal/datasource/host/exec"
 	processwatcher "github.com/nginx/agent/v3/internal/watcher/process"
 	"github.com/nginx/agent/v3/pkg/nginxprocess"
 )
diff --git a/internal/watcher/instance/instance_watcher_service.go b/internal/watcher/instance/instance_watcher_service.go
@@ -13,6 +13,8 @@ import (
 	"sync/atomic"
 	"time"
 
+	"github.com/nginx/agent/v3/pkg/host/exec"
+
 	"github.com/nginx/agent/v3/internal/datasource/proto"
 
 	parser "github.com/nginx/agent/v3/internal/datasource/config"
@@ -23,7 +25,6 @@ import (
 	"github.com/nginx/agent/v3/internal/watcher/process"
 
 	"github.com/nginx/agent/v3/internal/config"
-	"github.com/nginx/agent/v3/internal/datasource/host/exec"
 	"github.com/nginx/agent/v3/internal/logger"
 	"github.com/nginx/agent/v3/internal/model"
 )
diff --git a/internal/watcher/instance/instance_watcher_service_test.go b/internal/watcher/instance/instance_watcher_service_test.go
@@ -9,11 +9,12 @@ import (
 	"context"
 	"testing"
 
+	"github.com/nginx/agent/v3/pkg/host/exec/execfakes"
+
 	"github.com/nginx/agent/v3/internal/watcher/instance/instancefakes"
 	"github.com/nginx/agent/v3/internal/watcher/process/processfakes"
 
 	mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1"
-	"github.com/nginx/agent/v3/internal/datasource/host/exec/execfakes"
 	"github.com/nginx/agent/v3/internal/model"
 	testModel "github.com/nginx/agent/v3/test/model"
 	"github.com/nginx/agent/v3/test/protos"
diff --git a/internal/watcher/instance/nginx_process_parser.go b/internal/watcher/instance/nginx_process_parser.go
@@ -17,7 +17,7 @@ import (
 	"github.com/nginx/agent/v3/internal/model"
 
 	mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1"
-	"github.com/nginx/agent/v3/internal/datasource/host/exec"
+	"github.com/nginx/agent/v3/pkg/host/exec"
 	"github.com/nginx/agent/v3/pkg/id"
 	"github.com/nginx/agent/v3/pkg/nginxprocess"
 )
diff --git a/internal/watcher/instance/nginx_process_parser_test.go b/internal/watcher/instance/nginx_process_parser_test.go
@@ -14,12 +14,13 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/nginx/agent/v3/pkg/host/exec/execfakes"
+
 	"github.com/nginx/agent/v3/internal/model"
 
 	"google.golang.org/protobuf/proto"
 
 	mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1"
-	"github.com/nginx/agent/v3/internal/datasource/host/exec/execfakes"
 	"github.com/nginx/agent/v3/pkg/nginxprocess"
 	"github.com/nginx/agent/v3/test/helpers"
 	"github.com/nginx/agent/v3/test/protos"
diff --git a/pkg/host/exec/exec.go b/pkg/host/exec/exec.go
@@ -8,7 +8,6 @@ package exec
 import (
 	"bytes"
 	"context"
-	"log/slog"
 	"os"
 	"os/exec"
 	"syscall"
@@ -27,11 +26,14 @@ type ExecInterface interface {
 	KillProcess(pid int32) error
 	Hostname() (string, error)
 	HostID(ctx context.Context) (string, error)
-	ReleaseInfo(ctx context.Context) (releaseInfo *v1.ReleaseInfo)
+	ReleaseInfo(ctx context.Context) (releaseInfo *v1.ReleaseInfo, err error)
 }
 
 type Exec struct{}
 
+// RunCmd executes a command with the given arguments and returns its output.
+// It combines stdout and stderr into a single buffer.
+// If the command fails, the error is returned along with any output that was produced.
 func (*Exec) RunCmd(ctx context.Context, cmd string, args ...string) (*bytes.Buffer, error) {
 	command := exec.CommandContext(ctx, cmd, args...)
 
@@ -43,35 +45,44 @@ func (*Exec) RunCmd(ctx context.Context, cmd string, args ...string) (*bytes.Buf
 	return bytes.NewBuffer(output), nil
 }
 
+// Executable returns the path to the current executable.
 func (*Exec) Executable() (string, error) {
 	return os.Executable()
 }
 
+// FindExecutable searches for an executable named by the given file name in the
+// directories listed in the PATH environment variable.
 func (*Exec) FindExecutable(name string) (string, error) {
 	return exec.LookPath(name)
 }
 
+// ProcessID returns the process ID of the current process.
 func (*Exec) ProcessID() int32 {
 	return int32(os.Getpid())
 }
 
+// KillProcess sends a SIGHUP signal to the process with the given pid.
 func (*Exec) KillProcess(pid int32) error {
 	return syscall.Kill(int(pid), syscall.SIGHUP)
 }
 
+// Hostname returns the host name reported by the kernel.
 func (*Exec) Hostname() (string, error) {
 	return os.Hostname()
 }
 
+// HostID returns a unique ID for the host machine.
+// The context can be used to cancel the operation.
 func (*Exec) HostID(ctx context.Context) (string, error) {
 	return host.HostIDWithContext(ctx)
 }
 
-func (*Exec) ReleaseInfo(ctx context.Context) (releaseInfo *v1.ReleaseInfo) {
+// ReleaseInfo returns operating system release information.
+// It provides details about the platform, version, and other system information.
+func (*Exec) ReleaseInfo(ctx context.Context) (*v1.ReleaseInfo, error) {
 	hostInfo, err := host.InfoWithContext(ctx)
 	if err != nil {
-		slog.ErrorContext(ctx, "Could not read release information for host", "error", err)
-		return &v1.ReleaseInfo{}
+		return &v1.ReleaseInfo{}, err
 	}
 
 	return &v1.ReleaseInfo{
@@ -80,5 +91,5 @@ func (*Exec) ReleaseInfo(ctx context.Context) (releaseInfo *v1.ReleaseInfo) {
 		Codename:  hostInfo.OS,
 		Name:      hostInfo.PlatformFamily,
 		Id:        hostInfo.Platform,
-	}
+	}, nil
 }
diff --git a/pkg/host/exec/exec_test.go b/pkg/host/exec/exec_test.go
diff --git a/pkg/host/exec/execfakes/fake_exec_interface.go b/pkg/host/exec/execfakes/fake_exec_interface.go
diff --git a/pkg/host/hostfakes/fake_info_interface.go b/pkg/host/hostfakes/fake_info_interface.go
diff --git a/pkg/host/info.go b/pkg/host/info.go
diff --git a/pkg/host/info_test.go b/pkg/host/info_test.go

Original file line number	Diff line number	Diff line change
`@@ -13,8 +13,8 @@ import (`
`13`	`13`	`"regexp"`
`14`	`14`	`"strings"`
`15`	`15`
`16`		`- "github.com/nginx/agent/v3/internal/datasource/host/exec"`
`17`	`16`	`"github.com/nginx/agent/v3/internal/model"`
	`17`	`+ "github.com/nginx/agent/v3/pkg/host/exec"`
`18`	`18`	`"github.com/nginx/agent/v3/pkg/nginxprocess"`
`19`	`19`	`)`
`20`	`20`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ import (`
`11`	`11`	`"errors"`
`12`	`12`	`"testing"`
`13`	`13`
`14`		`- "github.com/nginx/agent/v3/internal/datasource/host/exec/execfakes"`
	`14`	`+ "github.com/nginx/agent/v3/pkg/host/exec/execfakes"`
`15`	`15`	`"github.com/stretchr/testify/assert"`
`16`	`16`	`)`
`17`	`17`