Decouple ENI in AttachmentStateChange #3969
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fierlion
force-pushed
the
fierlion/attachmentInterface
branch
2 times, most recently
from
1f87bdf
to
2bfefd2
Compare
fierlion
force-pushed
the
fierlion/attachmentInterface
branch
3 times, most recently
from
a9a9f4a
to
3c9761e
Compare
fierlion
changed the title
amogh09
reviewed
Oct 20, 2023
fierlion
force-pushed
the
fierlion/attachmentInterface
branch
from
3c9761e
to
0e3b384
Compare
amogh09
approved these changes
Oct 20, 2023
mye956
approved these changes
Oct 21, 2023
timj-hh
pushed a commit
to timj-hh/amazon-ecs-agent
that referenced
this pull request
Oct 23, 2023
singholt
added a commit
to singholt/amazon-ecs-agent
that referenced
this pull request
Oct 26, 2023
This reverts commit 75db5cb. 2. Revert "update daemon log path host root in container (aws#3982)" This reverts commit 6166b6e. 3. Revert "Fix loading CSI driver container from state if it exists" This reverts commit 1391502. 4. Revert "Latest FE model files (aws#3974)" This reverts commit b589a08. 5. Revert "Move utils function Uint16SliceToStringSlice to shared lib" This reverts commit 882bcb2. 6. Revert "Move remaining credentials module to shared library" This reverts commit f58f228. 7. Revert "fix merge import failure (aws#3976)" This reverts commit 46e21e6. 8. Revert "Fixing task volumes from payload for EBS-backed tasks" This reverts commit fc97633. 9. Revert "Create Generic Attachment to decouple from ENI and save attachment state. (aws#3969)" This reverts commit 12e23ed. 10. Revert "Add support for finding EBS devices on Xen instances (aws#3971)" This reverts commit 3500ac2. 11. Revert "Add network builder and platform APIs (aws#3939)" This reverts commit b8d2a7f. 12. Revert "Minor refactor of TTL cache (allow nil TTL + get TTL)" This reverts commit 4b33664. 13. Revert "Move httpclient to ecs-agent/ and minor refactor" This reverts commit 041ca3f. 14. Revert "Add missing EBSTaskAttach changes from feature branch" This reverts commit 7a0d116.
singholt
added a commit
to singholt/amazon-ecs-agent
that referenced
this pull request
Oct 26, 2023
1. Revert "Load Managed Daemon images in background (aws#3984)" This reverts commit 75db5cb. 2. Revert "update daemon log path host root in container (aws#3982)" This reverts commit 6166b6e. 3. Revert "Fix loading CSI driver container from state if it exists" This reverts commit 1391502. 4. Revert "Latest FE model files (aws#3974)" This reverts commit b589a08. 5. Revert "Move utils function Uint16SliceToStringSlice to shared lib" This reverts commit 882bcb2. 6. Revert "Move remaining credentials module to shared library" This reverts commit f58f228. 7. Revert "fix merge import failure (aws#3976)" This reverts commit 46e21e6. 8. Revert "Fixing task volumes from payload for EBS-backed tasks" This reverts commit fc97633. 9. Revert "Create Generic Attachment to decouple from ENI and save attachment state. (aws#3969)" This reverts commit 12e23ed. 10. Revert "Add support for finding EBS devices on Xen instances (aws#3971)" This reverts commit 3500ac2. 11. Revert "Add network builder and platform APIs (aws#3939)" This reverts commit b8d2a7f. 12. Revert "Minor refactor of TTL cache (allow nil TTL + get TTL)" This reverts commit 4b33664. 13. Revert "Move httpclient to ecs-agent/ and minor refactor" This reverts commit 041ca3f. 14. Revert "Add missing EBSTaskAttach changes from feature branch" This reverts commit 7a0d116.
singholt
added a commit
that referenced
this pull request
Oct 26, 2023
1. Revert "Load Managed Daemon images in background (#3984)" This reverts commit 75db5cb. 2. Revert "update daemon log path host root in container (#3982)" This reverts commit 6166b6e. 3. Revert "Fix loading CSI driver container from state if it exists" This reverts commit 1391502. 4. Revert "Latest FE model files (#3974)" This reverts commit b589a08. 5. Revert "Move utils function Uint16SliceToStringSlice to shared lib" This reverts commit 882bcb2. 6. Revert "Move remaining credentials module to shared library" This reverts commit f58f228. 7. Revert "fix merge import failure (#3976)" This reverts commit 46e21e6. 8. Revert "Fixing task volumes from payload for EBS-backed tasks" This reverts commit fc97633. 9. Revert "Create Generic Attachment to decouple from ENI and save attachment state. (#3969)" This reverts commit 12e23ed. 10. Revert "Add support for finding EBS devices on Xen instances (#3971)" This reverts commit 3500ac2. 11. Revert "Add network builder and platform APIs (#3939)" This reverts commit b8d2a7f. 12. Revert "Minor refactor of TTL cache (allow nil TTL + get TTL)" This reverts commit 4b33664. 13. Revert "Move httpclient to ecs-agent/ and minor refactor" This reverts commit 041ca3f. 14. Revert "Add missing EBSTaskAttach changes from feature branch" This reverts commit 7a0d116.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The problem we've encountered can be summarized with this code: https://github.com/aws/amazon-ecs-agent/blob/master/agent/api/statechange.go#L98-L103
An attachment state change is only ever wrapping the
Attachment *ni.ENIAttachmenttype.In order to save the Resource attachments to our bolt db, we can't keep calling them "ENIs", (which is our current hack to get around the limitation).
This update puts both the ENI and the Resource attachment types behind an Attachment interface. Attachment access has moved to use the generic interface methods exposed by the underlying Attachments.
Note that the bulk of the changed files are to update the import paths.
Implementation details
the ecs-agent library
/apidirectory now has a newattachmenttype witheniandresourcesubdirectories, each containing their attachment type implementations. All Attachment access is now via the interface methods, except in our boltdb save to disk process for agent restarts which unpacks the generics via type switch and handles the state save/load via separate data handlers for each type.Note that the
eniattachment.gofile is still duplicated per #3801 to support the ongoing common networking library changes. For now the files are identical except for their package name to allow for the imports to work. Ultimately once the common library is in place we can remove the duplication.Testing
manual test cases:
start an ebs backed task, stop agent/start agent, stop task and see that the stop task cleanup unstage process works.
used a boltdb to json utility to validate the agent.db file contains the attachment payload.
New tests cover the changes: yes
Description for the changelog
Add generic Attachment interface to allow for multiple attachment types.
Does this PR include breaking model changes? If so, Have you added transformation functions?
No this change only includes an additive change -- ResourceAttachments
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.