Skip to content
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

engine: Use known versions for logging drivers #875

Merged
merged 1 commit into from
Jul 11, 2017

Conversation

samuelkarp
Copy link
Contributor

@samuelkarp samuelkarp commented Jul 4, 2017

Summary

The ECS agent's capability system is used for conditionally-enabling
features based on whether the underlying Docker daemon and the ECS agent
support those features. This is a reasonable approach for many of the
features that Docker supports, especially those that involve structural
elements in the Config and HostConfig objects of the Docker API. It's
particularly important that the agent reports only the versions that
have full structural compatibility as "supported" to the ECS backend, as
some parts of the Config and HostConfig are sent from the backend as
serialized JSON and then are deserialized in the agent into defined
structs. (If we were to deserialize JSON that contained unknown fields,
we'd lose the data in those unknown fields.)

However, there are a few unstructured elements that the Docker daemon
uses, especially for driver selection and driver options. We know that
the agent can deserialize those fields, so there's no risk of losing
data. We've also found that it can be time-consuming to validate that we
have complete structural API representation for new versions, which
complicates the process for adding support for new drivers.

This commit changes the logic to separate "known" API versions from
"supported" API versions. A "known" version can be used to gate
non-structural API changes (things like logging driver availability)
from API changes that do require structural compatibility. "known" API
versions are not reported in the capability map, but can be used as
feature gates for logging drivers.

Implementation details

New functions that report "known" API versions and pass through to the gating logic. Changed gating logic for logging drivers to use "known" versions instead of "supported" versions.

Testing

  • Builds on Linux (make release)
  • Builds on Windows (go build -out amazon-ecs-agent.exe ./agent)
  • Unit tests on Linux (make test) pass
  • Unit tests on Windows (go test -timeout=25s ./agent/...) pass
  • Integration tests on Linux (make run-integ-tests) pass
  • Integration tests on Windows (.\scripts\run-integ-tests.ps1) pass
  • Functional tests on Linux (make run-functional-tests) pass
  • Functional tests on Windows (.\scripts\run-functional-tests.ps1) pass

New tests cover the changes: updated a test to cover this change

Description for the changelog

None, but this would be a prerequisite for #870.

Licensing

This contribution is under the terms of the Apache 2.0 License: yes

The ECS agent's capability system is used for conditionally-enabling
features based on whether the underlying Docker daemon and the ECS agent
support those features. This is a reasonable approach for many of the
features that Docker supports, especially those that involve structural
elements in the Config and HostConfig objects of the Docker API. It's
particularly important that the agent reports only the versions that
have full structural compatibility as "supported" to the ECS backend, as
some parts of the Config and HostConfig are sent from the backend as
serialized JSON and then are deserialized in the agent into defined
structs. (If we were to deserialize JSON that contained unknown fields,
we'd lose the data in those unknown fields.)

However, there are a few unstructured elements that the Docker daemon
uses, especially for driver selection and driver options. We know that
the agent can deserialize those fields, so there's no risk of losing
data. We've also found that it can be time-consuming to validate that we
have complete structural API representation for new versions, which
complicates the process for adding support for new drivers.

This commit changes the logic to separate "known" API versions from
"supported" API versions.  A "known" version can be used to gate
non-structural API changes (things like logging driver availability)
from API changes that do require structural compatibility. "known" API
versions are not reported in the capability map, but can be used as
feature gates for logging drivers.
Copy link

@richardpen richardpen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you also want to update the getKnownAPIVersions list, looks the latest version is 1.29.

@samuelkarp
Copy link
Contributor Author

Do you also want to update the getKnownAPIVersions list, looks the latest version is 1.29.

I don't think it's necessary since we don't have anything that uses 1.29 yet.

@@ -17,8 +17,8 @@
package mock_ecr

import (
ecr0 "github.com/aws/amazon-ecs-agent/agent/ecr"
ecr "github.com/aws/amazon-ecs-agent/agent/ecr/model/ecr"
ecr "github.com/aws/amazon-ecs-agent/agent/ecr"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ecr and ecr0 aren't deterministically named here. This change will just cause confusion in the git history.

Could you either:
a) fix this and make it deterministic
b) regenerate the mock until it doesn't flip definitions for ecr and ecr0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not as part of this change.

@samuelkarp samuelkarp merged commit baf952b into aws:dev Jul 11, 2017
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.

4 participants