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

ROX-23370: add requests and limits to FM initContainer #1747

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

vladbologa
Copy link
Contributor

@vladbologa vladbologa commented Apr 8, 2024

Description

This PR adds requests and limits to the initContainer of fleet-manager.

The initContainer executes fleet-manager migrate, which is why I decided to re-use the same requests and limits that we use for the fleet-manager container itself (which executes fleet-manager serve).

It's not possible to see on Prometheus the actual memory & CPU usage of the init container in production, probably because the process is too short lived and it's not getting scraped. To get an estimation of its usage, I ran this locally:

❯ /usr/bin/time -l -h -p ./fleet-manager migrate
I0408 15:37:43.709514   26957 cmd.go:20] Migration starting
I0408 15:37:43.873693   26957 202212150000_add_organisation_name.go:67] added column "OrganisationName" in schema migration "202212150000"
I0408 15:37:43.879213   26957 202301301200_add_internal_to_central_request.go:69] Successfully added the Internal column
I0408 15:37:43.892965   26957 202303220000_drop_skip_scheduling_from_clusters.go:32] Successfully removed the SkipScheduling column
I0408 15:37:43.899766   26957 202303230000_add_schedulable_to_clusters.go:32] Successfully added the Schedulable column
I0408 15:37:43.906613   26957 202304200000_add_force_reconcile_to_central_request.go:27] Successfully added the ForceReconcile column
I0408 15:37:43.992760   26957 cmd.go:23] Database has 27 migrations applied
real 0.35
user 0.08
sys 0.03
            70582272  maximum resident set size
                   0  average shared memory size
                   0  average unshared data size
                   0  average unshared stack size
                5100  page reclaims
                   0  page faults
                   0  swaps
                   0  block input operations
                   0  block output operations
                 547  messages sent
                 546  messages received
                 102  signals received
                  66  voluntary context switches
                6305  involuntary context switches
           636463809  instructions retired
           316147130  cycles elapsed
            35587776  peak memory footprint

The maximum resident set was ~70MiB. This should work well with the current memory requests / limits of 512Mi / 1024Mi.

I also looked at the CPU usage, which peaked at 80%. This should also work well with the current CPU requests / limits of 200m / 1.

Checklist (Definition of Done)

  • Unit and integration tests added
  • Added test description under Test manual
  • Documentation added if necessary (i.e. changes to dev setup, test execution, ...)
  • CI and all relevant tests are passing
  • Add the ticket number to the PR title if available, i.e. ROX-12345: ...
  • Discussed security and business related topics privately. Will move any security and business related topics that arise to private communication channel.
  • Add secret to app-interface Vault or Secrets Manager if necessary
  • RDS changes were e2e tested manually
  • Check AWS limits are reasonable for changes provisioning new resources
  • (If applicable) Changes to the dp-terraform Helm values have been reflected in the addon on integration environment

Test manual

TODO: Add manual testing efforts

# To run tests locally run:
make db/teardown db/setup db/migrate
make ocm/setup
make verify lint binary test test/integration

Copy link
Contributor

openshift-ci bot commented Apr 8, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ludydoo, vladbologa

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@vladbologa vladbologa merged commit 5ed9b23 into main Apr 8, 2024
4 checks passed
@vladbologa vladbologa deleted the vb/fm-init-limits branch April 8, 2024 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants