Skip to content

Add configuration for optional CPU request on build pod#1984

Open
bluek1te wants to merge 3 commits into
jupyterhub:mainfrom
bluek1te:main
Open

Add configuration for optional CPU request on build pod#1984
bluek1te wants to merge 3 commits into
jupyterhub:mainfrom
bluek1te:main

Conversation

@bluek1te

Copy link
Copy Markdown

Add ability to configure CPU request on build pod - We have cluster restrictions that require us to affix a cpu request before spawning pods so this would help us immensely.

@manics

manics commented Jul 24, 2025

Copy link
Copy Markdown
Member

Unfortunately this won't work, because the "build pod" doesn't build the image directly, it calls out to an external Docker or Podman build daemon.

@bluek1te

Copy link
Copy Markdown
Author

@manics
We just need a CPU resource attribute to be affixed to the build pod. Even if it calls out to an external docker daemon, the "build pod" still takes some amount of CPU to run. There are already configurations for memory requests/limits for this specific pod, so I don't see why this would be any different.

@bluek1te

bluek1te commented Aug 8, 2025

Copy link
Copy Markdown
Author

Bump

@rgaiacs

rgaiacs commented Sep 9, 2025

Copy link
Copy Markdown
Contributor

@bluek1te some tests are failing and I think we might have fixed it already. Can you rebase this pull request so that we can check the tests? Thanks!

@rgaiacs rgaiacs added the code:helm-chart Helm template changes. label Sep 9, 2025
@bluek1te

bluek1te commented Sep 9, 2025

Copy link
Copy Markdown
Author

@rgaiacs looks like the tests failed after syncing, but they are timeout errors and I don't think they are related to my changes. Is there an easy way to rerun these tests on my side without pushing empty commits?

@rgaiacs

rgaiacs commented Sep 10, 2025

Copy link
Copy Markdown
Contributor

Is there an easy way to rerun these tests on my side without pushing empty commits?

I recently discovered that close and open the pull request will trigger a new CI job.

There is also an option when you look at the logs of the job but I don't know what access level is required to trigger the re-run.

I'm manually trigger a re-run of the failed jobs.

@rgaiacs

rgaiacs commented Sep 10, 2025

Copy link
Copy Markdown
Contributor

During the first CI run, tests (v1.30, main) and tests (v1.29, helm, pink failed.

During the second CI run, only tests (v1.30, main) failed.

The reason for the tests to fail are different. The first time was

=========================== short test summary info ============================
FAILED binderhub/tests/test_repoproviders.py::test_gitlab_ref[v10.0.6-b3344b7f17c335a817c5d7608c5e47fd7cabc023] - tornado.curl_httpclient.CurlError: HTTP 599: Operation timed out after 20000 milliseconds with 0 bytes received
FAILED binderhub/tests/test_repoproviders.py::test_gitlab_ref[HEAD-True] - tornado.curl_httpclient.CurlError: HTTP 599: Operation timed out after 20000 milliseconds with 0 bytes received
FAILED binderhub/tests/test_repoproviders.py::test_gitlab_ref[nosuchref-None] - tornado.curl_httpclient.CurlError: HTTP 599: Operation timed out after 20001 milliseconds with 0 bytes received
===== 3 failed, 147 passed, 3 deselected, 12 warnings in 183.96s (0:03:03) =====

the second time was

=========================== short test summary info ============================
FAILED binderhub/tests/test_repoproviders.py::test_figshare[10.6084/m9.figshare.9782777.v1-10.6084/m9.figshare.9782777.v1-9782777.v1-https://doi.org/10.6084/m9.figshare.9782777.v1-figshare-9782777.v1] - tornado.httpclient.HTTPClientError: HTTP 403: Forbidden
FAILED binderhub/tests/test_repoproviders.py::test_figshare[10.6084/m9.figshare.9782777-10.6084/m9.figshare.9782777.v1-9782777.v1-https://doi.org/10.6084/m9.figshare.9782777.v1-figshare-9782777.v1] - tornado.httpclient.HTTPClientError: HTTP 403: Forbidden
===== 2 failed, 148 passed, 3 deselected, 12 warnings in 131.66s (0:02:11) =====

I don't think this pull request is related with the above test failure given that only the test for figshare failed.

@bluek1te bluek1te closed this Sep 10, 2025
@github-project-automation github-project-automation Bot moved this from Backlog to Done in PR triage (experimental) Sep 10, 2025
@bluek1te bluek1te reopened this Sep 10, 2025
@bluek1te

Copy link
Copy Markdown
Author

I've rerun the tests and they look like they are all passing now.

@rgaiacs rgaiacs requested a review from manics September 10, 2025 15:09
@bluek1te

Copy link
Copy Markdown
Author

Bump

@bluek1te

Copy link
Copy Markdown
Author

@manics

@rgaiacs rgaiacs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me.

@rgaiacs

rgaiacs commented Jan 16, 2026

Copy link
Copy Markdown
Contributor

@bluek1te Thanks for the contribution. Have you heard about our roadmap workshop? Maybe you want to attend and contribute?

Let's wait a few more days to get a second review on this pull request.

I added a reminder to myself to get this done by next week.

@minrk

minrk commented Apr 13, 2026

Copy link
Copy Markdown
Member

Sorry for the wait. This has been addressed with a more general resources in #2071 which allows you to set arbitrary resource requests/limits on the repo2docker pod.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code:helm-chart Helm template changes.

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

5 participants