CMP-3847: Added TestOperatorResourceLimitsConfigurable tests operator's resource limits and requests can be configured via patching the deployment#1084
Conversation
…hat Operator's resource limits and requests can be configured via patching the deployment
|
[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 |
|
Skipping CI for Draft Pull Request. |
|
@taimurhafeez: No Jira issue with key FCMP-3847 exists in the tracker at https://issues.redhat.com/. 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. |
|
@taimurhafeez: This pull request references CMP-3847 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 "4.22.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. |
|
🤖 To deploy this PR, run the following command: |
|
🤖 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. |
rhmdnd
left a comment
There was a problem hiding this comment.
Couple comments on testing OLM behavior and some recommendations for restoring the defaults.
Thanks for the PR!
| "resources": { | ||
| "limits": { | ||
| "cpu": "200m", | ||
| "memory": "500Mi" |
There was a problem hiding this comment.
If we change these default values in the CSV, the test will revert to a non-default value, right?
We might want to grab the original values and restore those at the end of the test (since these can change).
There was a problem hiding this comment.
Oh - I suppose we're asserting these are the default values above. In that case, the test would already fail.
Maybe we can have the default values as constants?
| oldPodUID := podList.Items[0].UID | ||
|
|
||
| // Apply the patch | ||
| err = f.Client.Patch(context.TODO(), deployment, client.RawPatch(types.StrategicMergePatchType, patchData)) |
There was a problem hiding this comment.
Will this change persist? Or will OLM revert the operator limits back to what's in the CSV?
If so, we may need to adjust the test to patch the CSV?
| if len(podList.Items) == 0 { | ||
| t.Fatal("no compliance-operator pods found before patch") | ||
| } | ||
| oldPodUID := podList.Items[0].UID |
There was a problem hiding this comment.
This might break in the off chance the operator is rolling out an update and there is one terminating pod and one running pod. Checking the pod name, it's state, and the list length == 1 might make this more robust.
| // NodeSelector: workerNodesLabel, | ||
| // ComplianceScanSettings: compv1alpha1.ComplianceScanSettings{ | ||
| // Debug: true, | ||
| // }, |
There was a problem hiding this comment.
nit: unrelated whitespace change.
TestOperatorResourceLimitsConfigurable verifies that the Compliance Operator's CPU and memory resource limits can be dynamically configured by patching its Deployment object. This ensures that cluster administrators can tune the operator's resource consumption to meet their cluster's needs.
Can be executed:
make e2e-serial E2E_GO_TEST_FLAGS="-v -run TestOperatorResourceLimitsConfigurable"Expected Output:
Used with PR-960.
Assisted by Claude.