Merge pull request ceph#65055 from dmick/build-with-container-npmcache

pybind/mgr/dashboard/frontend: add NPM_CACHEDIR envvar, use in bwc


Reviewed-by: John Mulligan <[email protected]>

Author: phlogistonjohn

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

@@ -60,6 +60,11 @@ else(WITH_SYSTEM_NPM)
   if(DEFINED ENV{NODE_MIRROR})
     set(node_mirror_opt "--mirror=$ENV{NODE_MIRROR}")
   endif()
+  if(DEFINED ENV{NPM_CACHEDIR})
+    set(npm-cache-dir "$ENV{NPM_CACHEDIR}")
+  else()
+    set(npm-cache-dir "${mgr-dashboard-nodeenv-dir}/.npm")
+  endif()
   add_custom_command(
     OUTPUT "${mgr-dashboard-nodeenv-dir}/bin/npm"
     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)
     # uid 1000 and running the unpack in a id-mapped namespace (container)
     # that lets tar set the uid to a "bad" uid outside the namespace
     COMMAND bash -c 'chown -R `id -u`:`id -g` ${mgr-dashboard-nodeenv-dir}/src'
-    COMMAND mkdir ${mgr-dashboard-nodeenv-dir}/.npm
+    COMMAND mkdir -p ${npm-cache-dir}
     WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
     COMMENT "dashboard nodeenv is being installed")
   if(DEFINED ENV{NPM_REGISTRY})
@@ -79,7 +84,7 @@ else(WITH_SYSTEM_NPM)
   add_npm_options(
     NODEENV_DIR ${mgr-dashboard-nodeenv-dir}
     TARGET mgr-dashboard-nodeenv
-    OPTION cache=${mgr-dashboard-nodeenv-dir}/.npm
+    OPTION cache=${npm-cache-dir}
     ${npm_registry_opts})
   add_custom_target(mgr-dashboard-frontend-deps
     DEPENDS node_modules mgr-dashboard-nodeenv

src/script/build-with-container.py

@@ -243,6 +243,12 @@ def _container_cmd(ctx, args, *, workdir=None, interactive=False):
         cmd.append(f"-eCCACHE_BASEDIR={ctx.cli.homedir}")
     for extra_arg in ctx.cli.extra or []:
         cmd.append(extra_arg)
+    if ctx.npm_cache_dir:
+        # use :z so that other builds can use the cache
+        cmd.extend([
+            f'--volume={ctx.npm_cache_dir}:/npmcache:z',
+            '--env=NPM_CACHEDIR=/npmcache'
+        ])
     cmd.append(ctx.image_name)
     cmd.extend(args)
     return cmd
@@ -292,6 +298,7 @@ class Steps(StrEnum):
     BUILD_CONTAINER = "build-container"
     CONTAINER = "container"
     CONFIGURE = "configure"
+    NPM_CACHE = "npmcache"
     BUILD = "build"
     BUILD_TESTS = "buildtests"
     TESTS = "tests"
@@ -395,6 +402,14 @@ def dnf_cache_dir(self):
             return path.resolve()
         return None
 
+    @property
+    def npm_cache_dir(self):
+        if self.cli.npm_cache_path:
+            path = pathlib.Path(self.cli.npm_cache_path)
+            path = path.expanduser()
+            return path.resolve()
+        return None
+
     @property
     def map_user(self):
         # TODO: detect if uid mapping is needed
@@ -528,6 +543,14 @@ def dnf_cache_dir(ctx):
     (cache_dir / ".DNF_CACHE").touch(exist_ok=True)
 
 
+@Builder.set(Steps.NPM_CACHE)
+def npm_cache_dir(ctx):
+    """Set up an NPM cache directory for reuse across container builds."""
+    if not ctx.cli.npm_cache_path:
+        return
+    ctx.npm_cache_dir.mkdir(parents=True, exist_ok=True)
+
+
 @Builder.set(Steps.BUILD_CONTAINER)
 def build_container(ctx):
     """Generate a build environment container image."""
@@ -545,7 +568,7 @@ def build_container(ctx):
         cmd.append(f"--build-arg=DISTRO={ctx.from_image}")
     if ctx.dnf_cache_dir and "docker" in ctx.container_engine:
         log.warning(
-            "The --volume option is not supported by docker. Skipping dnf cache dir mounts"
+            "The --volume option is not supported by docker build/buildx. Skipping dnf cache dir mounts"
         )
     elif ctx.dnf_cache_dir:
         cmd += [
@@ -638,6 +661,7 @@ def bc_configure(ctx):
 @Builder.set(Steps.BUILD)
 def bc_build(ctx):
     """Execute a standard build."""
+    ctx.build.wants(Steps.NPM_CACHE, ctx)
     ctx.build.wants(Steps.CONFIGURE, ctx)
     cmd = _container_cmd(
         ctx,
@@ -654,6 +678,7 @@ def bc_build(ctx):
 @Builder.set(Steps.BUILD_TESTS)
 def bc_build_tests(ctx):
     """Build the tests."""
+    ctx.build.wants(Steps.NPM_CACHE, ctx)
     ctx.build.wants(Steps.CONFIGURE, ctx)
     cmd = _container_cmd(
         ctx,
@@ -670,6 +695,7 @@ def bc_build_tests(ctx):
 @Builder.set(Steps.TESTS)
 def bc_run_tests(ctx):
     """Execute the tests."""
+    ctx.build.wants(Steps.NPM_CACHE, ctx)
     ctx.build.wants(Steps.BUILD_TESTS, ctx)
     cmd = _container_cmd(
         ctx,
@@ -685,7 +711,8 @@ def bc_run_tests(ctx):
 
 @Builder.set(Steps.SOURCE_RPM)
 def bc_make_source_rpm(ctx):
-    """Build SPRMs."""
+    """Build SRPMs."""
+    ctx.build.wants(Steps.NPM_CACHE, ctx)
     ctx.build.wants(Steps.CONTAINER, ctx)
     make_srpm_cmd = f"cd {ctx.cli.homedir} && ./make-srpm.sh"
     if ctx.cli.ceph_version:
@@ -927,7 +954,11 @@ def parse_cli(build_step_names):
     )
     parser.add_argument(
         "--dnf-cache-path",
-        help="DNF caching using provided base dir",
+        help="DNF caching using provided base dir (during build-container build)",
+    )
+    parser.add_argument(
+        "--npm-cache-path",
+        help="NPM caching using provided base dir (during build)",
     )
     parser.add_argument(
         "--build-dir",