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

ecs_client/model, functional_tests: updated model, functional tests for adding and dropping Linux capabilities #956

Merged
merged 4 commits into from
Sep 28, 2017

Conversation

sharanyad
Copy link
Contributor

@sharanyad sharanyad commented Aug 29, 2017

Summary

Model changes to Container Definition for enabling capadd and capdrop for Linux containers.

Functional tests

TODO:

  • Documentation for the new structures
  • Change required agent version for cap-add, cap-drop tests

Implementation details

  • Model changes in ContainerDefinition of the form:
linuxParameters: {
    capabilities: {
        add: [""],
        drop: [""]
    }
}
  • Functional Tests that verify if the specified capability has been added to and dropped from the task's container

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: yes

Description for the changelog

Licensing

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

Copy link
Contributor

@aaithal aaithal left a comment

Choose a reason for hiding this comment

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

Is there any particular reason for not combining add and drop int o a single task def and test?

@sharanyad
Copy link
Contributor Author

@aaithal Not very particularly. Just had it that way since they(cap-add/cap-drop) are separate flags in docker run. Would you prefer it the other way?

@aaithal
Copy link
Contributor

aaithal commented Aug 31, 2017

If we could model it in the same task def, that would be better as it'd reduce the boot strapping overhead for a new test.

@samuelkarp
Copy link
Contributor

Code looks good. Can you rebase and resolve conflicts?

Copy link
Contributor

@aaithal aaithal left a comment

Choose a reason for hiding this comment

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

:shipit:

@samuelkarp
Copy link
Contributor

@sharanyad I think we can merge this whenever you're ready.

@sharanyad
Copy link
Contributor Author

Updated the API and documentation for the new structures. Let's merge once the builds pass.

…or adding and dropping Linux capabilities

The changes include the following:
- Model changes in ContainerDefinition of the form:
linuxParameters: {
    capabilities: {
        add: [""],
        drop: [""]
    }
}
- Functional Tests that verify if the specified capability has been added to and dropped from the task's container
@sharanyad
Copy link
Contributor Author

Updated API and Docs to the latest version.
Removed LaunchType from API as per @aaithal 's comment.

@sharanyad
Copy link
Contributor Author

All builds and tests pass
@samuelkarp @aaithal Could you please have a quick look once?

@sharanyad sharanyad changed the title [WIP] ecs_client/model, functional_tests: updated model, functional tests for adding and dropping Linux capabilities ecs_client/model, functional_tests: updated model, functional tests for adding and dropping Linux capabilities Sep 28, 2017
Copy link
Contributor

@aaithal aaithal left a comment

Choose a reason for hiding this comment

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

:shipit:

@sharanyad sharanyad merged commit 640d704 into aws:dev Sep 28, 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.

3 participants