Skip to content
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
7c01b9d
Add test before the fix
pjanotti Nov 15, 2025
0f84444
Use sudo when setting the dockerd config file
pjanotti Nov 15, 2025
48cca92
Use system V CLI commands
pjanotti Nov 15, 2025
75fd6c3
run a container so there are metrics to be collected
pjanotti Nov 15, 2025
5d3677a
Keep container running
pjanotti Nov 15, 2025
76a00a5
Fix run/rm docker container location
pjanotti Nov 15, 2025
a380cd3
do not error on dockerd restart
pjanotti Nov 16, 2025
36fe762
change function to get service state
pjanotti Nov 16, 2025
a11a44f
Remove unused import
pjanotti Nov 16, 2025
eac105f
Missed update
pjanotti Nov 16, 2025
f0fba40
Fix mv command
pjanotti Nov 16, 2025
002a755
Upgrade to latest dockerd
pjanotti Nov 16, 2025
2e840b2
Re-org CI workflow
pjanotti Nov 17, 2025
963fcd0
Remove duplicated code
pjanotti Nov 17, 2025
649a3cf
Negotiate client API version
pjanotti Nov 17, 2025
95d5d9f
Re-add container to ensure metrics are generated
pjanotti Nov 17, 2025
9a96ec7
Remove test w/ 1.44
pjanotti Nov 17, 2025
27e8162
Expect test failure for default
pjanotti Nov 17, 2025
b01366c
Reinstate the fix
pjanotti Nov 17, 2025
9a60ba2
Add changelog
pjanotti Nov 17, 2025
71b0ed3
Update internal/signalfx-agent/pkg/monitors/docker/docker_test.go
pjanotti Nov 17, 2025
e32045d
Update internal/signalfx-agent/pkg/monitors/docker/testdata/upgrade-d…
pjanotti Nov 17, 2025
ce711ba
Update internal/signalfx-agent/pkg/monitors/docker/testdata/upgrade-d…
pjanotti Nov 17, 2025
6c2dd1f
Update internal/signalfx-agent/pkg/monitors/docker/docker_test.go
pjanotti Nov 17, 2025
52c7e80
Run go test with `-v`
pjanotti Nov 18, 2025
178cad2
Code review feedback
pjanotti Nov 18, 2025
6e59502
Merge branch 'main' into fix-docker-monitor-min-client-ver
pjanotti Nov 18, 2025
caec134
Copilot suggestion
pjanotti Nov 18, 2025
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
21 changes: 21 additions & 0 deletions .chloggen/fix-docker-monitor-min-client-ver.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# 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. crosslink)
component: smartagent/docker-container-stats


# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Use client API version negotiation instead of fixed version

# One or more tracking issues related to the change
issues: [6941]

# (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: |
The `smartagent/docker-container-stats` monitor was using a fixed API version of `v1.24`,
which is not compatible with version `v29` of the Docker engine. This change updates the
monitor to use API version negotiation, allowing it to work with a wider range of Docker
engine versions.
12 changes: 12 additions & 0 deletions .github/workflows/build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -129,3 +129,15 @@ jobs:
uses: ./.github/workflows/reusable-compile.yml
with:
SYS_BINARY: ${{ matrix.SYS_BINARIES }}

latest-dockerd-test:
# Run this as a separate test to avoid affecting other tests with the dockerd upgrade.
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@v5
- uses: ./.github/actions/setup-environment

- name: dockerd test
working-directory: ./internal/signalfx-agent/pkg/monitors/docker
run: |
go test -v -tags=dockerd -timeout 3m ./...
4 changes: 1 addition & 3 deletions internal/signalfx-agent/pkg/monitors/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ import (
"github.com/signalfx/signalfx-agent/pkg/utils/timeutil"
)

const dockerAPIVersion = "v1.24"

func init() {
monitors.Register(&monitorMetadata, func() interface{} { return &Monitor{} }, &Config{})
}
Expand Down Expand Up @@ -97,7 +95,7 @@ func (m *Monitor) Configure(conf *Config) error {
defaultHeaders := map[string]string{"User-Agent": "signalfx-agent"}

var err error
m.client, err = docker.NewClientWithOpts(docker.WithHost(conf.DockerURL), docker.WithVersion(dockerAPIVersion), docker.WithHTTPHeaders(defaultHeaders))
m.client, err = docker.NewClientWithOpts(docker.WithHost(conf.DockerURL), docker.WithAPIVersionNegotiation(), docker.WithHTTPHeaders(defaultHeaders))
if err != nil {
return fmt.Errorf("could not create docker client: %w", err)
}
Expand Down
232 changes: 232 additions & 0 deletions internal/signalfx-agent/pkg/monitors/docker/docker_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,232 @@
//go:build dockerd

package docker

import (
"encoding/json"
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"
"sync"
"testing"
"time"

"github.com/signalfx/defaults"
"github.com/signalfx/golib/v3/datapoint" //nolint:staticcheck // SA1019: deprecated package still in use
"github.com/signalfx/golib/v3/event" //nolint:staticcheck // SA1019: deprecated package still in use
"github.com/signalfx/golib/v3/trace" //nolint:staticcheck // SA1019: deprecated package still in use
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/pdata/pmetric"

"github.com/signalfx/signalfx-agent/pkg/core/config"
"github.com/signalfx/signalfx-agent/pkg/core/dpfilters"
"github.com/signalfx/signalfx-agent/pkg/monitors/types"
)

func TestMinimumRequiredClientVersion(t *testing.T) {
// Skip this test if not running on Linux GitHub runner
if runtime.GOOS != "linux" {
t.Skip("Skipping test on non-Linux OS")
}
if os.Getenv("GITHUB_ACTIONS") != "true" {
t.Skip("Skipping test outside of GitHub Actions")
}

// Execute the dockerd upgrade script and fail the test if it fails.
// This test is not rolling back the dockerd upgrade, so it can affect all subsequent tests.
scriptPath := filepath.Join("testdata", "upgrade-dockerd-on-ubuntu.sh")
scriptCmd := exec.Command("bash", scriptPath)
scriptOut, err := scriptCmd.CombinedOutput()
t.Logf("upgrade-dockerd-on-ubuntu.sh output:\n%s\n", string(scriptOut))
require.NoError(t, err, "upgrade-dockerd-on-ubuntu.sh failed with exit code %d", scriptCmd.ProcessState.ExitCode())

tt := []struct {
minimumRequiredClientVersion string
}{
{
minimumRequiredClientVersion: "default",
},
{
minimumRequiredClientVersion: "1.24",
},
}

for _, tc := range tt {
t.Run(tc.minimumRequiredClientVersion, func(t *testing.T) {
if tc.minimumRequiredClientVersion != "default" {
// TODO: Update the docker daemon to the specified version before running the test
updateGHLinuxRunnerDockerDaemonMinClientVersion(t, tc.minimumRequiredClientVersion)
}

// Run a container to have some metrics to collect
// Attention: this container should be started only after the settings for docker daemon are updated
// and should be removed before the docker daemon settings are reset.
cmd := exec.Command("docker", "run", "-d", "--name", "docker-client-test", "alpine", "sleep", "180")
err := cmd.Run()
require.NoError(t, err, "Failed to run docker container")
defer func() {
cmd := exec.Command("docker", "rm", "-f", "docker-client-test")
err := cmd.Run()
require.NoError(t, err, "Failed to remove docker container")
}()

output := &fakeOutput{}
monitor := &Monitor{
Output: output,
}
config := &Config{
MonitorConfig: config.MonitorConfig{
IntervalSeconds: 1,
},
}
defaults.Set(config)

err = monitor.Configure(config)
require.NoError(t, err, "Expected no error during monitor configuration")
defer monitor.Shutdown()

require.Eventually(t, func() bool {
return output.HasDatapoints()
}, 10*time.Second, 100*time.Millisecond, "Expected datapoints to be collected")
})
}
}

func updateGHLinuxRunnerDockerDaemonMinClientVersion(t *testing.T, minimumRequiredClientVersion string) {
// Fail if there is already a daemon.json file
if _, err := os.Stat("/etc/docker/daemon.json"); err == nil {
t.Fatal("daemon.json already exists, cannot update minimum required client version")
}

daemonConfig := map[string]string{
"min-api-version": minimumRequiredClientVersion,
}

configJSON, err := json.MarshalIndent(daemonConfig, "", " ")
require.NoError(t, err, "Failed to marshal daemon config")
t.Logf("Docker daemon config JSON:\n%s", string(configJSON))

// Create a temporary daemon.json file with the new configuration then
// move it using sudo to the correct location.
tempFileName := filepath.Join(t.TempDir(), "daemon.json")
err = os.WriteFile(tempFileName, configJSON, 0o644)
require.NoError(t, err, "Failed to write daemon.json")

cmd := exec.Command("sudo", "mv", tempFileName, "/etc/docker/")
err = cmd.Run()
require.NoError(t, err, "Failed to move daemon.json")

cmd = exec.Command("sudo", "service", "docker", "restart")
// Ignore error since the docker daemon might automatically restart after
// adding the config file
err = cmd.Run()
if err != nil {
t.Logf("Docker daemon restart error: %s", err)
}

t.Cleanup(func() {
cmd := exec.Command("sudo", "rm", "/etc/docker/daemon.json")
err := cmd.Run()
require.NoError(t, err, "Failed to remove daemon.json")

cmd = exec.Command("sudo", "service", "docker", "restart")
// Ignore error since the docker daemon might automatically restart after
// removing the config file
err = cmd.Run()
if err != nil {
t.Logf("Docker daemon restart error: %s", err)
}

requireDockerDaemonRunning(t)
})

requireDockerDaemonRunning(t)
}

func requireDockerDaemonRunning(t *testing.T) {
require.Eventually(t, func() bool {
status, err := getServiceStatus(t, "docker")
require.NoError(t, err, "Failed to get docker service status")
return status == "running"
}, 30*time.Second, 1*time.Second)
}

func getServiceStatus(t *testing.T, serviceName string) (string, error) {
cmd := exec.Command("service", serviceName, "status")
output, err := cmd.CombinedOutput()
if err != nil {
// The 'service status' command often returns a non-zero exit code if the service is not running
// or if there's an error. We still need to parse the output to determine the status.
t.Logf("Error getting %q service status: %v", serviceName, err)
}

outputStr := string(output)
t.Logf("%q service status output:\n%s", serviceName, outputStr)

if strings.Contains(outputStr, "active (running)") {
return "running", nil
}

return "not running", nil
}

type fakeOutput struct {
datapoints []*datapoint.Datapoint
mu sync.Mutex
}

var _ types.FilteringOutput = (*fakeOutput)(nil)

func (fo *fakeOutput) AddDatapointExclusionFilter(_ dpfilters.DatapointFilter) {
panic("unimplemented")
}

func (fo *fakeOutput) AddExtraDimension(_ string, _ string) {
panic("unimplemented")
}

func (fo *fakeOutput) Copy() types.Output {
panic("unimplemented")
}

func (fo *fakeOutput) EnabledMetrics() []string {
return []string{}
}

func (fo *fakeOutput) HasAnyExtraMetrics() bool {
panic("unimplemented")
}

func (fo *fakeOutput) HasEnabledMetricInGroup(_ string) bool {
panic("unimplemented")
}

func (fo *fakeOutput) SendDimensionUpdate(_ *types.Dimension) {
panic("unimplemented")
}

func (fo *fakeOutput) SendEvent(_ *event.Event) {
panic("unimplemented")
}

func (fo *fakeOutput) SendMetrics(_ ...pmetric.Metric) {
panic("unimplemented")
}

func (fo *fakeOutput) SendSpans(_ ...*trace.Span) {
panic("unimplemented")
}

func (fo *fakeOutput) SendDatapoints(dps ...*datapoint.Datapoint) {
fo.mu.Lock()
defer fo.mu.Unlock()
fo.datapoints = append(fo.datapoints, dps...)
}

func (fo *fakeOutput) HasDatapoints() bool {
fo.mu.Lock()
defer fo.mu.Unlock()
return len(fo.datapoints) > 0
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/bin/bash
# This script is used to upgrade the Docker daemon on Ubuntu to the latest version.

set -e

# Add Docker's official GPG key:
sudo apt update
sudo apt install -y ca-certificates curl
sudo install -m 0755 -d /etc/apt/keyrings
sudo curl -fsSL https://download.docker.com/linux/ubuntu/gpg -o /etc/apt/keyrings/docker.asc
sudo chmod a+r /etc/apt/keyrings/docker.asc

# Add the repository to Apt sources:
sudo tee /etc/apt/sources.list.d/docker.sources <<EOF
Types: deb
URIs: https://download.docker.com/linux/ubuntu
Suites: $(. /etc/os-release && echo "${UBUNTU_CODENAME:-$VERSION_CODENAME}")
Components: stable
Signed-By: /etc/apt/keyrings/docker.asc
EOF

sudo apt update
sudo apt-get install -y docker-ce --only-upgrade
sudo dockerd --version
Loading