Repository navigation
CMP-3857: Add e2e test for api-resource-collector memory under load - #1181
taimurhafeez wants to merge 1 commit into
Conversation
|
@taimurhafeez: This pull request references CMP-3857 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: taimurhafeez The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
🤖 To deploy this PR, run the following command: |
|
Stale PRs are closed after 21d of inactivity. If this PR is still relevant, comment to refresh it or remove the stale label. If this PR is safe to close now please do so with /lifecycle stale |
|
Stale PRs rot after 14d of inactivity. Mark the PR as fresh by commenting If this PR is safe to close now please do so with /lifecycle rotten |
|
/remove-lifecycle rotten |
84fc6c9 to
fed8230
Compare
|
🤖 To deploy this PR, run the following command: |
|
Stale PRs are closed after 21d of inactivity. If this PR is still relevant, comment to refresh it or remove the stale label. If this PR is safe to close now please do so with /lifecycle stale |
|
Stale PRs rot after 14d of inactivity. Mark the PR as fresh by commenting If this PR is safe to close now please do so with /lifecycle rotten |
|
Rotten PRs close after 7d of inactivity. Reopen the PR by commenting /close |
|
@openshift-ci[bot]: Closed this PR. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@taimurhafeez: This pull request references CMP-3857 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.1.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
The api-resource-collector reads every API path the content asks for into memory and runs under a 202Mi limit, which makes it the component that runs out of memory first in a large environment. It has been OOMKilled before, fetching the MachineConfigs the PCI-DSS profile examines, which is why mcStreamer pages through the MachineConfig list and drops each object's Ignition file contents before collecting it. Nothing covered that. Add TestAPIResourceCollectorHandlesLargeMachineConfigs to the prerelease suite. It creates 60 MachineConfigs carrying ~900KiB of Ignition file content each, runs a platform scan, and fails if the collector is OOMKilled. Correct code holds one page, five objects, at a time; without the paging or the stripping all ~53MiB materialize at once and the collector is killed. The MachineConfigs carry a role no MachineConfigPool selects, so the MachineConfigOperator renders nothing from them and no node reboots, and the test removes them afterwards. Scan pods restart on failure, so an OOMKilled collector would otherwise surface only as the scan never reaching DONE, long after the fact. WaitForAPIResourceCollector watches the init container's status and reports the kill as soon as the kubelet records it, in LastTerminationState once the container is restarting, and including when the runtime reports only SIGKILL rather than OOMKilled. The paths the collector fetches come from the content, not from the operator, so the test also checks the collector's log for the whole MachineConfig list path. If a future datastream stops asking for the list, the load never reaches the collector, and this fails saying so instead of passing without exercising anything. This replaces the operator-pod memory test the change started out as. The operator's manager cache is scoped to WATCH_NAMESPACE, so the namespace count does not grow it, and measuring the operator pod could not have caught a collector OOM. Co-authored-by: Taimur Hafeez <taimurhafeez93@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fed8230 to
71f3295
Compare
rhmdnd
changed the title
|
🤖 To deploy this PR, run the following command: |
|
@taimurhafeez: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
| // Match the whole logged URI. The list path is a prefix of the paths that | ||
| // name a single MachineConfig, and those are fetched whole by uriStreamer, | ||
| // which is not what this test is about. | ||
| if !strings.Contains(logs, fmt.Sprintf("Fetching URI: '%s'", mcListPath)) { |
There was a problem hiding this comment.
This seems brittle... if we change the logging output, we could introduce a regression.
Supersedes #1126.
The api-resource-collector reads every API path the content asks for
into memory and runs under a 202Mi limit, which makes it the component
that runs out of memory first in a large environment. It has been
OOMKilled before, fetching the MachineConfigs the PCI-DSS profile
examines, which is why mcStreamer pages through the MachineConfig list
and drops each object's Ignition file contents before collecting it.
Nothing covered that.
Add TestAPIResourceCollectorHandlesLargeMachineConfigs to the prerelease
suite. It creates 60 MachineConfigs carrying ~900KiB of Ignition file
content each, runs a platform scan, and fails if the collector is
OOMKilled. Correct code holds one page, five objects, at a time; without
the paging or the stripping all ~53MiB materialize at once and the
collector is killed. The MachineConfigs carry a role no
MachineConfigPool selects, so the MachineConfigOperator renders nothing
from them and no node reboots, and the test removes them afterwards.
Scan pods restart on failure, so an OOMKilled collector would otherwise
surface only as the scan never reaching DONE, long after the fact.
WaitForAPIResourceCollector watches the init container's status and
reports the kill as soon as the kubelet records it, in
LastTerminationState once the container is restarting, and including
when the runtime reports only SIGKILL rather than OOMKilled.
The paths the collector fetches come from the content, not from the
operator, so the test also checks the collector's log for the whole
MachineConfig list path. If a future datastream stops asking for the
list, the load never reaches the collector, and this fails saying so
instead of passing without exercising anything.
This replaces the operator-pod memory test the change started out as.
The operator's manager cache is scoped to WATCH_NAMESPACE, so the
namespace count does not grow it, and measuring the operator pod could
not have caught a collector OOM.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com