Skip to content

Update compute resources to account for MCAD and InstaScale #305

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 10, 2023
Merged

Update compute resources to account for MCAD and InstaScale #305

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4b6ffea
Update compute resources to account for MCAD and InstaScale
astefanutti Sep 25, 2023
80eedbd
Patch Deployment resources for e2e tests
astefanutti Sep 25, 2023
b092dfd
Overwrite Subscription resources in OLM e2e test
astefanutti Sep 25, 2023
f728e93
e2e: Upgrade OLM to version v0.25.0
astefanutti Sep 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions config/manager/manager.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ spec:
periodSeconds: 10
resources:
limits:
cpu: 500m
memory: 128Mi
cpu: "1"
memory: 1Gi
requests:
cpu: 10m
memory: 64Mi
cpu: "1"
Copy link
Contributor

Choose a reason for hiding this comment

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

1 CPU is quite high for two controllers, is it really needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I've been reluctant, given our current CI environment is short on CPU, but MCAD has not the reputation to be lightweight, and its tests assume 2 CPUs. It may explain why no CPU requirements were specified for MCAD in the previous operator design.

I also think it may not be a good practice to drive these requirements by the limitation of our current CI environment. So they may have to be configured, depending on the environment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking whether it may have sense to reduce the request value.
This helps with resource usage for not intensive cases, while keeping the limit high enough. On the other side it can affect pod eviction order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd be inclined to have MCAD configured with the guaranteed QoS class, and with enough resources, so it performs acceptably by default.

With that in mind, I've added the extra test configuration so it can still run within the limited resources of GH Actions runners.

memory: 1Gi
serviceAccountName: controller-manager
terminationGracePeriodSeconds: 10