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

Add cache for podman ps #87

Merged
merged 16 commits into from
Nov 21, 2023
Merged

Add cache for podman ps #87

merged 16 commits into from
Nov 21, 2023

Conversation

christoph-hamm
Copy link
Contributor

Issues: #1

Currently for each podman and podman-kube workload the Ankaios agent calls the command podman ps once per second.
This needs a lot CPU resources.
The Ankaios agent shall cache podman ps result, and use the result for all podman workloads.

Definition of Done

The PR shall be merged only if all items mentioned in
CONTRIBUTING.md
have been followed. In case an item is not applicable as described, please provide a short explanation in the description.

@christoph-hamm christoph-hamm marked this pull request as draft November 14, 2023 11:00
@christoph-hamm christoph-hamm marked this pull request as ready for review November 17, 2023 09:05
@christoph-hamm christoph-hamm added this to the v0.2 milestone Nov 17, 2023
@krucod3 krucod3 self-requested a review November 17, 2023 14:29
Copy link
Contributor

@krucod3 krucod3 left a comment

Choose a reason for hiding this comment

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

Finished with the review of the code. There were some small findings that I directly fixed. The fixes are in single commits s.t. they can be reverted if you don't agree with them.

What we definitely have to take care of is the bug with the pod states in case one of the pods in a workloads disappears (see below for details).

I still need to review the tests for the podman_cli.

agent/src/runtime_connectors/podman/podman_runtime.rs Outdated Show resolved Hide resolved
agent/src/runtime_connectors/podman/podman_runtime.rs Outdated Show resolved Hide resolved
agent/src/runtime_connectors/podman_cli.rs Outdated Show resolved Hide resolved
agent/src/runtime_connectors/podman_cli.rs Outdated Show resolved Hide resolved
agent/src/runtime_connectors/podman_cli.rs Outdated Show resolved Hide resolved
agent/src/runtime_connectors/podman_cli.rs Outdated Show resolved Hide resolved
agent/src/runtime_connectors/podman_cli.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@krucod3 krucod3 left a comment

Choose a reason for hiding this comment

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

Review finished, findings documented.

agent/src/runtime_connectors/podman_cli.rs Show resolved Hide resolved
agent/src/runtime_connectors/podman_cli.rs Outdated Show resolved Hide resolved
christoph-hamm and others added 2 commits November 20, 2023 13:45
Issue-Id: #1

Co-authored-by: Kaloyan <36224699+krucod3@users.noreply.github.com>
krucod3
krucod3 previously approved these changes Nov 21, 2023
Copy link
Contributor

@krucod3 krucod3 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@krucod3 krucod3 dismissed their stale review November 21, 2023 06:19

The unit tests are failing

Copy link
Contributor

@krucod3 krucod3 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@krucod3 krucod3 merged commit b156960 into main Nov 21, 2023
6 checks passed
@krucod3 krucod3 deleted the 1-podman_ps_cache branch November 21, 2023 15:21
@windsource windsource added the enhancement New feature or request. Issue will appear in the change log "Features" label Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request. Issue will appear in the change log "Features" ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants