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

Update docs, update cpu/memory request, limits for update-api agent DaemonSet #55

Merged
merged 2 commits into from
Feb 25, 2021

Conversation

etungsten
Copy link
Contributor

@etungsten etungsten commented Feb 15, 2021

Issue number:
#54
#47

Description of changes:

Author: Erikson Tung <etung@amazon.com>
Date:   Mon Feb 15 13:17:29 2021 -0800

    deployment: change cpu and memory request, limits for api agent pods
    
    Significantly lower cpu, memory request/limits for the update-api agent
    DaemonSet


commit 96771ced4149e95ebc92ae65aeb63a436e573d74
Author: Erikson Tung <etung@amazon.com>
Date:   Mon Feb 15 13:05:04 2021 -0800

    README: change recommended update interface version to 2.0.0, add desc
    
    Update documentation to include short descriptions of the update
    interface versions. Include compatibility information

Testing done:

Monitored the memory usage of the agent using the update API. I never saw it peak above 25MB in physical memory usage.
Tried different CPU request values, the agent still works fine in updating nodes when requesting 10 millicores.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@bgdnlp
Copy link

bgdnlp commented Feb 16, 2021

Should the example development deployment that is linked to just below in the README also be adjusted or maybe removed?

To me it seems to be a bit out of date and the suggested deployment looks simple enough to serve as a starting point, I would completely remove that line.

@etungsten
Copy link
Contributor Author

etungsten commented Feb 22, 2021

Push above removes mention of the the development deployment as a customization starting point since that might be outdated.

Fixing it up should be addressed separately

Copy link
Contributor

@srgothi92 srgothi92 left a comment

Choose a reason for hiding this comment

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

Looks good! except for one minor confusion of commits I think. Multiple commit are pain to update especially if change is required in last commit in the chain.

Fixing it up should be addressed separately

Do you know where we use ./dev/deployment.yaml ? Should we delete it instead of fixing it ? I feel we should delete kind-cluster.yaml as well, we cannot use that as direct alternative to our actual bottlerocket cluster. It adds more confusion then to help. However, all of these suggestion are not for this PR, we can create an issue for cleanup.

README.md Show resolved Hide resolved
Update documentation to include short descriptions of the update
interface versions. Include compatiblity information
Significantly lower cpu, memory request for the update-api agent
DaemonSet
@etungsten etungsten merged commit 4f24dc2 into bottlerocket-os:develop Feb 25, 2021
@etungsten etungsten deleted the res-doc-update branch February 25, 2021 20:34
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