Skip to content

Determine the fluentd async option based on Docker server version #4558

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 16, 2025

Conversation

singholt
Copy link
Contributor

@singholt singholt commented Apr 3, 2025

Summary

Docker's fluentd log driver supports passing an async option. The fluentd-async-connect option was deprecated in Docker v20.10.0 and removed completely in v28.0.0, in favor of fluentd-async. See: https://docs.docker.com/engine/deprecated/#fluentd-async-connect-log-opt. Functionally, both options are equivalent, see: moby/moby#39086.

This PR makes ECS agent support the new fluentd-async option, while maintaining backwards-compatibility with older Docker server versions.

Implementation details

  1. Added the new fluentd-asyncoption alongside the old fluentd-async-connect option. in the Docker task engine.

  2. In order to be backwards compatible with Docker versions older than 20.10.0, ECS agent will continue using the legacy fluentd-async-connect option for old versions. The new fluentd-async option will be used on instances with Docker version 20.10.0 and above.

Since the refactor on Docker's side isn't versioned and affects all client API versions, the ECS agent will determine which option to use, using the Docker server version. This is already available to the Docker task engine.

// Version returns the underlying docker version.
func (engine *DockerTaskEngine) Version() (string, error) {
return engine.client.Version(engine.ctx, dockerclient.VersionTimeout)
}

Testing

New tests cover the changes: Yes

  1. Updated existing unit tests and added new unit tests.
  2. Ran Firelens functional tests against this PR on a container instance that has Docker v25 (version shipped in the ECS AMIs), the tests passed.
  3. Ran Firelens functional tests against this PR on a container instance that has Docker v28 installed, the tests passed.
  4. Ran Firelens functional tests against this PR on a container instance that has Docker v19, the tests passed.

Description for the changelog

Enhancement: Determine the fluentd log driver's async option based on Docker server version

Additional Information

Does this PR include breaking model changes? If so, Have you added transformation functions? no

Does this PR include the addition of new environment variables in the README? no

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@singholt singholt changed the base branch from master to dev April 3, 2025 16:16
@singholt singholt force-pushed the fluentd-async branch 5 times, most recently from a7cf71e to e1d0b9c Compare April 15, 2025 19:56
@singholt singholt changed the title Replace "fluentd-async-connect" with "fluentd-async" Determine the fluentd async option based on Docker server version Apr 15, 2025
@singholt singholt force-pushed the fluentd-async branch 10 times, most recently from 2204383 to cc8dcb9 Compare April 15, 2025 22:08
Docker's fluentd log driver supports passing an "async" option.
The "fluentd-async-connect" option was deprecated in Docker v20.10.0 and completely removed in v28.0.0.
Reference: https://docs.docker.com/engine/deprecated/#fluentd-async-connect-log-opt
Functionally, both options are equivalent, see: moby/moby#39086.

Since Docker's refactor of the fluentd async option was not versioned and affects all API versions,
we need to determine the appropriate option using the Docker Server version, in order to be backwards compatible.
@singholt singholt marked this pull request as ready for review April 15, 2025 23:02
@singholt singholt requested a review from a team as a code owner April 15, 2025 23:02
@singholt singholt linked an issue Apr 15, 2025 that may be closed by this pull request
@singholt singholt merged commit 1a770f8 into aws:dev Apr 16, 2025
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incompatible with Docker v28 (API 1.48)
4 participants