Skip to content

Commit ad7e611

Browse files
committed
pybind/mgr/dashboard/frontend: add NPM_CACHEDIR envvar, use in bwc
Add an optional NPM_CACHEDIR environment variable to serve as the cache parameter for npm in the dashboard frontend build. The idea is to allow it to persist across builds so that we decrease the load on registry.npmjs.org, which has been throttling our requests when using build-with-container.py, and also hopefully improve the time of the frontend npm operations. build-with-container.py also grows a --npm-cache-path option to allow setting it for container builds and passing the envvar to the build. Fixes: https://tracker.ceph.com/issues/72298 Signed-off-by: Dan Mick <[email protected]>
1 parent dd36628 commit ad7e611

File tree

2 files changed

+41
-5
lines changed

2 files changed

+41
-5
lines changed

src/pybind/mgr/dashboard/frontend/CMakeLists.txt

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@ else(WITH_SYSTEM_NPM)
6060
if(DEFINED ENV{NODE_MIRROR})
6161
set(node_mirror_opt "--mirror=$ENV{NODE_MIRROR}")
6262
endif()
63+
if(DEFINED ENV{NPM_CACHEDIR})
64+
set(npm-cache-dir "$ENV{NPM_CACHEDIR}")
65+
else()
66+
set(npm-cache-dir "${mgr-dashboard-nodeenv-dir}/.npm")
67+
endif()
6368
add_custom_command(
6469
OUTPUT "${mgr-dashboard-nodeenv-dir}/bin/npm"
6570
COMMAND ${CMAKE_SOURCE_DIR}/src/tools/setup-virtualenv.sh --python=${MGR_PYTHON_EXECUTABLE} ${mgr-dashboard-nodeenv-dir}
@@ -70,7 +75,7 @@ else(WITH_SYSTEM_NPM)
7075
# uid 1000 and running the unpack in a id-mapped namespace (container)
7176
# that lets tar set the uid to a "bad" uid outside the namespace
7277
COMMAND bash -c 'chown -R `id -u`:`id -g` ${mgr-dashboard-nodeenv-dir}/src'
73-
COMMAND mkdir ${mgr-dashboard-nodeenv-dir}/.npm
78+
COMMAND mkdir -p ${npm-cache-dir}
7479
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
7580
COMMENT "dashboard nodeenv is being installed")
7681
if(DEFINED ENV{NPM_REGISTRY})
@@ -79,7 +84,7 @@ else(WITH_SYSTEM_NPM)
7984
add_npm_options(
8085
NODEENV_DIR ${mgr-dashboard-nodeenv-dir}
8186
TARGET mgr-dashboard-nodeenv
82-
OPTION cache=${mgr-dashboard-nodeenv-dir}/.npm
87+
OPTION cache=${npm-cache-dir}
8388
${npm_registry_opts})
8489
add_custom_target(mgr-dashboard-frontend-deps
8590
DEPENDS node_modules mgr-dashboard-nodeenv

src/script/build-with-container.py

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,12 @@ def _container_cmd(ctx, args, *, workdir=None, interactive=False):
243243
cmd.append(f"-eCCACHE_BASEDIR={ctx.cli.homedir}")
244244
for extra_arg in ctx.cli.extra or []:
245245
cmd.append(extra_arg)
246+
if ctx.npm_cache_dir:
247+
# use :z so that other builds can use the cache
248+
cmd.extend([
249+
f'--volume={ctx.npm_cache_dir}:/npmcache:z',
250+
'--env=NPM_CACHEDIR=/npmcache'
251+
])
246252
cmd.append(ctx.image_name)
247253
cmd.extend(args)
248254
return cmd
@@ -292,6 +298,7 @@ class Steps(StrEnum):
292298
BUILD_CONTAINER = "build-container"
293299
CONTAINER = "container"
294300
CONFIGURE = "configure"
301+
NPM_CACHE = "npmcache"
295302
BUILD = "build"
296303
BUILD_TESTS = "buildtests"
297304
TESTS = "tests"
@@ -395,6 +402,14 @@ def dnf_cache_dir(self):
395402
return path.resolve()
396403
return None
397404

405+
@property
406+
def npm_cache_dir(self):
407+
if self.cli.npm_cache_path:
408+
path = pathlib.Path(self.cli.npm_cache_path)
409+
path = path.expanduser()
410+
return path.resolve()
411+
return None
412+
398413
@property
399414
def map_user(self):
400415
# TODO: detect if uid mapping is needed
@@ -528,6 +543,14 @@ def dnf_cache_dir(ctx):
528543
(cache_dir / ".DNF_CACHE").touch(exist_ok=True)
529544

530545

546+
@Builder.set(Steps.NPM_CACHE)
547+
def npm_cache_dir(ctx):
548+
"""Set up an NPM cache directory for reuse across container builds."""
549+
if not ctx.cli.npm_cache_path:
550+
return
551+
ctx.npm_cache_dir.mkdir(parents=True, exist_ok=True)
552+
553+
531554
@Builder.set(Steps.BUILD_CONTAINER)
532555
def build_container(ctx):
533556
"""Generate a build environment container image."""
@@ -545,7 +568,7 @@ def build_container(ctx):
545568
cmd.append(f"--build-arg=DISTRO={ctx.from_image}")
546569
if ctx.dnf_cache_dir and "docker" in ctx.container_engine:
547570
log.warning(
548-
"The --volume option is not supported by docker. Skipping dnf cache dir mounts"
571+
"The --volume option is not supported by docker build/buildx. Skipping dnf cache dir mounts"
549572
)
550573
elif ctx.dnf_cache_dir:
551574
cmd += [
@@ -638,6 +661,7 @@ def bc_configure(ctx):
638661
@Builder.set(Steps.BUILD)
639662
def bc_build(ctx):
640663
"""Execute a standard build."""
664+
ctx.build.wants(Steps.NPM_CACHE, ctx)
641665
ctx.build.wants(Steps.CONFIGURE, ctx)
642666
cmd = _container_cmd(
643667
ctx,
@@ -654,6 +678,7 @@ def bc_build(ctx):
654678
@Builder.set(Steps.BUILD_TESTS)
655679
def bc_build_tests(ctx):
656680
"""Build the tests."""
681+
ctx.build.wants(Steps.NPM_CACHE, ctx)
657682
ctx.build.wants(Steps.CONFIGURE, ctx)
658683
cmd = _container_cmd(
659684
ctx,
@@ -670,6 +695,7 @@ def bc_build_tests(ctx):
670695
@Builder.set(Steps.TESTS)
671696
def bc_run_tests(ctx):
672697
"""Execute the tests."""
698+
ctx.build.wants(Steps.NPM_CACHE, ctx)
673699
ctx.build.wants(Steps.BUILD_TESTS, ctx)
674700
cmd = _container_cmd(
675701
ctx,
@@ -685,7 +711,8 @@ def bc_run_tests(ctx):
685711

686712
@Builder.set(Steps.SOURCE_RPM)
687713
def bc_make_source_rpm(ctx):
688-
"""Build SPRMs."""
714+
"""Build SRPMs."""
715+
ctx.build.wants(Steps.NPM_CACHE, ctx)
689716
ctx.build.wants(Steps.CONTAINER, ctx)
690717
make_srpm_cmd = f"cd {ctx.cli.homedir} && ./make-srpm.sh"
691718
if ctx.cli.ceph_version:
@@ -927,7 +954,11 @@ def parse_cli(build_step_names):
927954
)
928955
parser.add_argument(
929956
"--dnf-cache-path",
930-
help="DNF caching using provided base dir",
957+
help="DNF caching using provided base dir (during build-container build)",
958+
)
959+
parser.add_argument(
960+
"--npm-cache-path",
961+
help="NPM caching using provided base dir (during build)",
931962
)
932963
parser.add_argument(
933964
"--build-dir",

0 commit comments

Comments
 (0)