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

Allow building the image without needing to launch it #1647

Merged
merged 48 commits into from Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
a5a13ec
Empty dummy commit to allow a draft PR to be created
consideRatio Mar 24, 2023
bb910cd
Add the traitlet option and check query param with same name
GeorgianaElena Apr 6, 2023
2ed3e8b
Update api exaple to support option
GeorgianaElena Apr 6, 2023
4a11638
No need for fstrings
GeorgianaElena Apr 6, 2023
008cc34
Try out a test
GeorgianaElena Apr 7, 2023
3505fcb
Update messages emmited
GeorgianaElena Apr 7, 2023
5ec3c5a
Use the ready phase to signal end of ops
GeorgianaElena Apr 7, 2023
bc8f68c
Make the no_launch option configurable
GeorgianaElena Apr 7, 2023
5987b4e
Add no_launch to local testing config
GeorgianaElena Apr 7, 2023
dc9b84b
Rename configs and change logic
GeorgianaElena Apr 7, 2023
835a43b
Change error type
GeorgianaElena Apr 7, 2023
fbdf0c6
Update examples
GeorgianaElena Apr 7, 2023
3c310e9
Update initial test
GeorgianaElena Apr 7, 2023
27c2944
Remove unused import
GeorgianaElena Apr 7, 2023
0aff6a8
Update messages and logic to preserve old ones but also be explicit a…
GeorgianaElena Apr 20, 2023
e67a84b
Update one more message
GeorgianaElena Apr 20, 2023
b80eaf7
Add more test for failing cases
GeorgianaElena Apr 20, 2023
97cabe3
Add missing config
GeorgianaElena Apr 20, 2023
0edd782
Test with build_only true
GeorgianaElena Apr 20, 2023
7f5156b
Allow for build_only configured app fixture in all kind of tests
GeorgianaElena Apr 21, 2023
dabfb6d
Do not mark tests remote as we cannot configure binderhub with the de…
GeorgianaElena Apr 21, 2023
1f4db5f
_binderhub_config fixture has a session scope
GeorgianaElena Apr 21, 2023
2f0e3ce
Add more comments and do some cleanup
GeorgianaElena Apr 21, 2023
09da08d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 21, 2023
3a43d8e
Only test one situation as running this test is toily
GeorgianaElena Apr 24, 2023
ff0670c
Put the fixtures back into the tests as they're needed
GeorgianaElena Apr 24, 2023
fc84a66
Move logic about determining the build only action into its own function
GeorgianaElena Apr 24, 2023
fb008c8
Move the eval table to the tratilet help string
GeorgianaElena Apr 24, 2023
19d45bd
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 24, 2023
96188ed
Add note about not supporting build only UI|
GeorgianaElena Apr 24, 2023
f53636a
Add fixtture to ensure no builds are tried when not available
GeorgianaElena Apr 24, 2023
702c57a
Render an error page when the build UI is accesed and require_build_o…
GeorgianaElena Apr 28, 2023
a7d2418
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 28, 2023
252987c
Just deactivate the / and /v2 endpoints when require_build_only is True
GeorgianaElena Apr 28, 2023
0203a2d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 28, 2023
15dc9d3
Update build success metrics even if we don't launch after
GeorgianaElena Jun 16, 2023
f3d174b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 16, 2023
c20ca53
Fix indentation
GeorgianaElena Jun 19, 2023
f7a4c3c
Replace require_build_only for a more clear enable_api_only_mode config
GeorgianaElena Sep 21, 2023
5cf641d
Update how we decide if build only mode was activated
GeorgianaElena Sep 21, 2023
61d1a5d
Get the Custom404 handler in both cases
GeorgianaElena Sep 21, 2023
08d4f1d
update message
GeorgianaElena Sep 21, 2023
368cad7
Update the test config
GeorgianaElena Sep 21, 2023
693baf9
Update tests
GeorgianaElena Sep 21, 2023
b2925dc
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 21, 2023
6252eb6
Updat log message and assert
GeorgianaElena Sep 22, 2023
114b195
Drop the outcome suffix for clariry and specify a status code for the…
GeorgianaElena Oct 10, 2023
d5f686d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 10, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
138 changes: 92 additions & 46 deletions binderhub/app.py
Expand Up @@ -786,6 +786,21 @@ def _template_path_default(self):
help="Origin to use when emitting events. Defaults to hostname of request when empty",
)

enable_api_only_mode = Bool(
False,
help="""
When enabled, BinderHub will operate in an API only mode,
without a UI, and with the only registered endpoints being:
- /metrics
- /versions
- /build/([^/]+)/(.+)
- /health
- /_config
- /* -> shows a 404 page
""",
config=True,
)

_build_config_deprecated_map = {
"appendix": ("BuildExecutor", "appendix"),
"push_secret": ("BuildExecutor", "push_secret"),
Expand Down Expand Up @@ -943,6 +958,7 @@ def initialize(self, *args, **kwargs):
"auth_enabled": self.auth_enabled,
"event_log": self.event_log,
"normalized_origin": self.normalized_origin,
"enable_api_only_mode": self.enable_api_only_mode,
}
)
if self.auth_enabled:
Expand All @@ -956,55 +972,85 @@ def initialize(self, *args, **kwargs):
(r"/metrics", MetricsHandler),
(r"/versions", VersionHandler),
(r"/build/([^/]+)/(.+)", BuildHandler),
(r"/v2/([^/]+)/(.+)", ParameterizedMainHandler),
(r"/repo/([^/]+)/([^/]+)(/.*)?", LegacyRedirectHandler),
# for backward-compatible mybinder.org badge URLs
# /assets/images/badge.svg
(
r"/assets/(images/badge\.svg)",
tornado.web.StaticFileHandler,
{"path": self.tornado_settings["static_path"]},
),
# /badge.svg
(
r"/(badge\.svg)",
tornado.web.StaticFileHandler,
{"path": os.path.join(self.tornado_settings["static_path"], "images")},
),
# /badge_logo.svg
(
r"/(badge\_logo\.svg)",
tornado.web.StaticFileHandler,
{"path": os.path.join(self.tornado_settings["static_path"], "images")},
),
# /logo_social.png
(
r"/(logo\_social\.png)",
tornado.web.StaticFileHandler,
{"path": os.path.join(self.tornado_settings["static_path"], "images")},
),
# /favicon_XXX.ico
(
r"/(favicon\_fail\.ico)",
tornado.web.StaticFileHandler,
{"path": os.path.join(self.tornado_settings["static_path"], "images")},
),
(
r"/(favicon\_success\.ico)",
tornado.web.StaticFileHandler,
{"path": os.path.join(self.tornado_settings["static_path"], "images")},
),
(
r"/(favicon\_building\.ico)",
tornado.web.StaticFileHandler,
{"path": os.path.join(self.tornado_settings["static_path"], "images")},
),
(r"/about", AboutHandler),
(r"/health", self.health_handler_class, {"hub_url": self.hub_url_local}),
(r"/_config", ConfigHandler),
(r"/", MainHandler),
(r".*", Custom404),
]
if not self.enable_api_only_mode:
# In API only mode the endpoints in the list below
# are unregistered as they don't make sense in a API only scenario
handlers += [
(r"/about", AboutHandler),
(r"/v2/([^/]+)/(.+)", ParameterizedMainHandler),
(r"/", MainHandler),
(r"/repo/([^/]+)/([^/]+)(/.*)?", LegacyRedirectHandler),
# for backward-compatible mybinder.org badge URLs
# /assets/images/badge.svg
(
r"/assets/(images/badge\.svg)",
tornado.web.StaticFileHandler,
{"path": self.tornado_settings["static_path"]},
),
# /badge.svg
(
r"/(badge\.svg)",
tornado.web.StaticFileHandler,
{
"path": os.path.join(
self.tornado_settings["static_path"], "images"
)
},
),
# /badge_logo.svg
(
r"/(badge\_logo\.svg)",
tornado.web.StaticFileHandler,
{
"path": os.path.join(
self.tornado_settings["static_path"], "images"
)
},
),
# /logo_social.png
(
r"/(logo\_social\.png)",
tornado.web.StaticFileHandler,
{
"path": os.path.join(
self.tornado_settings["static_path"], "images"
)
},
),
# /favicon_XXX.ico
(
r"/(favicon\_fail\.ico)",
tornado.web.StaticFileHandler,
{
"path": os.path.join(
self.tornado_settings["static_path"], "images"
)
},
),
(
r"/(favicon\_success\.ico)",
tornado.web.StaticFileHandler,
{
"path": os.path.join(
self.tornado_settings["static_path"], "images"
)
},
),
(
r"/(favicon\_building\.ico)",
tornado.web.StaticFileHandler,
{
"path": os.path.join(
self.tornado_settings["static_path"], "images"
)
},
),
]
# This needs to be the last handler in the list, because it needs to match "everything else"
handlers.append((r".*", Custom404))
handlers = self.add_url_prefix(self.base_url, handlers)
if self.extra_static_path:
handlers.insert(
Expand Down
108 changes: 77 additions & 31 deletions binderhub/builder.py
Expand Up @@ -19,7 +19,7 @@
from tornado.iostream import StreamClosedError
from tornado.log import app_log
from tornado.queues import Queue
from tornado.web import Finish, authenticated
from tornado.web import Finish, HTTPError, authenticated

from .base import BaseHandler
from .build import ProgressEvent
Expand Down Expand Up @@ -228,6 +228,25 @@ def set_default_headers(self):
self.set_header("content-type", "text/event-stream")
self.set_header("cache-control", "no-cache")

def _get_build_only(self):
# Get the value of the `enable_api_only_mode` traitlet
enable_api_only_mode = self.settings.get("enable_api_only_mode", False)
# Get the value of the `build_only` query parameter if present
build_only_query_parameter = str(
self.get_query_argument(name="build_only", default="")
)
build_only = False
if build_only_query_parameter.lower() == "true":
if not enable_api_only_mode:
raise HTTPError(
Copy link
Member

Choose a reason for hiding this comment

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

This lacks a status code. Presumably it should be 400?

status_code=400,
log_message="Building but not launching is not permitted when"
" the API only mode was not enabled by setting `enable_api_only_mode` to True. ",
)
build_only = True

return build_only

@authenticated
async def get(self, provider_prefix, _unescaped_spec):
"""Get a built image for a given spec and repo provider.
Expand Down Expand Up @@ -408,33 +427,52 @@ async def get(self, provider_prefix, _unescaped_spec):
else:
image_found = True

if image_found:
build_only = self._get_build_only()
if build_only:
await self.emit(
{
"phase": "built",
"phase": "info",
"imageName": image_name,
"message": "Found built image, launching...\n",
"message": "The built image will not be launched "
"because the API only mode was enabled and the query parameter `build_only` was set to true\n",
}
)
with LAUNCHES_INPROGRESS.track_inprogress():
try:
await self.launch(provider)
except LaunchQuotaExceeded:
return
self.event_log.emit(
"binderhub.jupyter.org/launch",
5,
{
"provider": provider.name,
"spec": spec,
"ref": ref,
"status": "success",
"build_token": self._have_build_token,
"origin": self.settings["normalized_origin"]
if self.settings["normalized_origin"]
else self.request.host,
},
)
if image_found:
if build_only:
await self.emit(
{
"phase": "ready",
"imageName": image_name,
"message": "Done! Found built image\n",
}
)
else:
await self.emit(
{
"phase": "built",
"imageName": image_name,
"message": "Found built image, launching...\n",
}
)
with LAUNCHES_INPROGRESS.track_inprogress():
try:
await self.launch(provider)
except LaunchQuotaExceeded:
return
self.event_log.emit(
"binderhub.jupyter.org/launch",
5,
{
"provider": provider.name,
"spec": spec,
"ref": ref,
"status": "success",
"build_token": self._have_build_token,
"origin": self.settings["normalized_origin"]
if self.settings["normalized_origin"]
else self.request.host,
},
)
return

# Don't allow builds when quota is exceeded
Expand Down Expand Up @@ -504,7 +542,6 @@ def _check_result(future):

while not done:
progress = await q.get()

# FIXME: If pod goes into an unrecoverable stage, such as ImagePullBackoff or
# whatever, we should fail properly.
if progress.kind == ProgressEvent.Kind.BUILD_STATUS_CHANGE:
Expand All @@ -513,11 +550,22 @@ def _check_result(future):
# nothing to do, just waiting
continue
elif progress.payload == ProgressEvent.BuildStatus.BUILT:
if build_only:
message = "Done! Image built\n"
phase = "ready"
else:
message = "Built image, launching...\n"
event = {
"phase": phase,
"message": "Built image, launching...\n",
"message": message,
"imageName": image_name,
}
BUILD_TIME.labels(status="success").observe(
time.perf_counter() - build_starttime
)
BUILD_COUNT.labels(
status="success", **self.repo_metric_labels
).inc()
done = True
elif progress.payload == ProgressEvent.BuildStatus.RUNNING:
# start capturing build logs once the pod is running
Expand Down Expand Up @@ -549,15 +597,13 @@ def _check_result(future):
BUILD_COUNT.labels(
status="failure", **self.repo_metric_labels
).inc()

await self.emit(event)

# Launch after building an image
if build_only:
return

if not failed:
BUILD_TIME.labels(status="success").observe(
time.perf_counter() - build_starttime
)
BUILD_COUNT.labels(status="success", **self.repo_metric_labels).inc()
# Launch after building an image
with LAUNCHES_INPROGRESS.track_inprogress():
await self.launch(provider)
self.event_log.emit(
Expand Down
22 changes: 19 additions & 3 deletions binderhub/tests/conftest.py
Expand Up @@ -16,6 +16,7 @@
import requests
from tornado.httpclient import AsyncHTTPClient
from tornado.platform.asyncio import AsyncIOMainLoop
from traitlets.config import Config
from traitlets.config.loader import PyFileConfigLoader

from ..app import BinderHub
Expand Down Expand Up @@ -252,10 +253,25 @@ def app(request, io_loop, _binderhub_config):
app._configured_bhub = BinderHub(config=_binderhub_config)
return app

if hasattr(request, "param") and request.param is True:
# load conf for auth test
cfg = PyFileConfigLoader(binderhub_config_auth_additions_path).load_config()
api_only_app = False
if hasattr(request, "param"):
if request.param == "app_with_auth_config":
# load conf for auth test
cfg = PyFileConfigLoader(binderhub_config_auth_additions_path).load_config()
_binderhub_config.merge(cfg)
elif request.param == "api_only_app":
# load conf that sets BinderHub.enable_api_only_mode = True
cfg = Config({"BinderHub": {"enable_api_only_mode": True}})
_binderhub_config.merge(cfg)
api_only_app = True

if not api_only_app:
# load conf that sets BinderHub.require_build_only = False
# otherwise because _binderhub_config has a session scope,
# any previous set of require_build_only to True will stick around
cfg = Config({"BinderHub": {"enable_api_only_mode": False}})
_binderhub_config.merge(cfg)

bhub = BinderHub.instance(config=_binderhub_config)
bhub.initialize([])
bhub.start(run_loop=False)
Expand Down
6 changes: 3 additions & 3 deletions binderhub/tests/test_auth.py
Expand Up @@ -23,17 +23,17 @@ def use_session():
@pytest.mark.parametrize(
"app,path,authenticated",
[
(True, "/", True), # main page
("app_with_auth_config", "/", True), # main page
(
True,
"/v2/gh/binderhub-ci-repos/requirements/d687a7f9e6946ab01ef2baa7bd6d5b73c6e904fd",
True,
),
(True, "/metrics", False),
("app_with_auth_config", "/metrics", False),
],
indirect=[
"app"
], # send param True to app fixture, so that it loads authentication configuration
], # send param "app_with_auth_config" to app fixture, so that it loads authentication configuration
)
@pytest.mark.auth
async def test_auth(app, path, authenticated, use_session):
Expand Down