Skip to content

Commit dc3a9cf

Browse files
authored
Build: complete cleanup (#12430)
`clean_build` is not called when the task is terminated and there could be some lingering files around. Instead of only deleting the files for the current build, we delete the whole `settings.DOCROOT` path when the build starts.
1 parent 08c4bf9 commit dc3a9cf

File tree

3 files changed

+34
-18
lines changed

3 files changed

+34
-18
lines changed

readthedocs/projects/models.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1975,6 +1975,7 @@ def add_features(sender, **kwargs):
19751975
# Build related features
19761976
SCALE_IN_PROTECTION = "scale_in_prtection"
19771977
USE_S3_SCOPED_CREDENTIALS_ON_BUILDERS = "use_s3_scoped_credentials_on_builders"
1978+
BUILD_FULL_CLEAN = "build_full_clean"
19781979
DONT_CLEAN_BUILD = "dont_clean_build"
19791980
BUILD_HEALTHCHECK = "build_healthcheck"
19801981
BUILD_NO_ACKS_LATE = "build_no_acks_late"
@@ -2037,6 +2038,10 @@ def add_features(sender, **kwargs):
20372038
"Build: Don't clean the build directory. Only for Enterprise users with dedicated builders."
20382039
),
20392040
),
2041+
(
2042+
BUILD_FULL_CLEAN,
2043+
_("Build: Clean all build directories to avoid leftovers from other projects."),
2044+
),
20402045
(
20412046
BUILD_HEALTHCHECK,
20422047
_("Build: Use background cURL healthcheck."),

readthedocs/projects/tasks/builds.py

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,11 @@
8181
@dataclass(slots=True)
8282
class TaskData:
8383
"""
84-
Object to store all data related to a Celery task excecution.
84+
Object to store all data related to a Celery task execution.
8585
86-
We use this object from inside the task to store data while we are runnig
86+
We use this object from inside the task to store data while we are running
8787
the task. This is to avoid using `self.` inside the task due to its
88-
limitations: it's instanciated once and that instance is re-used for all
88+
limitations: it's instantiated once and that instance is re-used for all
8989
the tasks ran. This could produce sharing instance state between two
9090
different and unrelated tasks.
9191
@@ -418,9 +418,12 @@ def before_start(self, task_id, args, kwargs):
418418
protected_from_scale_in=True,
419419
)
420420

421-
# Clean the build paths completely to avoid conflicts with previous run
422-
# (e.g. cleanup task failed for some reason)
423-
clean_build(self.data.version)
421+
if self.data.project.has_feature(Feature.BUILD_FULL_CLEAN):
422+
# Clean DOCROOT path completely to avoid conflicts other projects
423+
clean_build()
424+
else:
425+
# Clean the build paths for this version to avoid conflicts with previous run
426+
clean_build(self.data.version)
424427

425428
# NOTE: this is never called. I didn't find anything in the logs, so we
426429
# can probably remove it
@@ -558,7 +561,7 @@ def get_valid_artifact_types(self):
558561
559562
TODO: remove the limitation of only 1 file.
560563
Add support for multiple PDF files in the output directory and
561-
grab them by using glob syntaxt between other files that could be garbage.
564+
grab them by using glob syntax between other files that could be garbage.
562565
"""
563566
valid_artifacts = []
564567
for artifact_type in ARTIFACT_TYPES:
@@ -789,7 +792,7 @@ def update_build(self, state=None):
789792
log.exception("Error while updating the build object.", state=state)
790793

791794
def execute(self):
792-
# Clonning
795+
# Cloning
793796
self.update_build(state=BUILD_STATE_CLONING)
794797

795798
# TODO: remove the ``create_vcs_environment`` hack. Ideally, this should be

readthedocs/projects/tasks/utils.py

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@
2525
log = structlog.get_logger(__name__)
2626

2727

28-
def clean_build(version):
28+
def clean_build(version=None):
2929
"""Clean the files used in the build of the given version."""
3030

31-
if version.project.has_feature(
31+
if version and version.project.has_feature(
3232
Feature.DONT_CLEAN_BUILD,
3333
):
3434
log.info(
@@ -38,15 +38,23 @@ def clean_build(version):
3838
)
3939
return
4040

41-
del_dirs = [
42-
os.path.join(version.project.doc_path, dir_, version.slug)
43-
for dir_ in ("checkouts", "envs", "conda", "artifacts")
44-
]
45-
del_dirs.append(os.path.join(version.project.doc_path, ".cache"))
41+
if version:
42+
del_dirs = [
43+
os.path.join(version.project.doc_path, dir_, version.slug)
44+
for dir_ in ("checkouts", "envs", "conda", "artifacts")
45+
]
46+
del_dirs.append(os.path.join(version.project.doc_path, ".cache"))
47+
48+
log.info("Removing directories.", directories=del_dirs)
49+
for path in del_dirs:
50+
safe_rmtree(path, ignore_errors=True)
4651

47-
log.info("Removing directories.", directories=del_dirs)
48-
for path in del_dirs:
49-
safe_rmtree(path, ignore_errors=True)
52+
# Clean up DOCROOT (e.g. `user_builds/`) completely
53+
else:
54+
log.info("Removing DOCROOT directory.", docroot=settings.DOCROOT)
55+
safe_rmtree(settings.DOCROOT, ignore_errors=True)
56+
os.makedirs(settings.DOCROOT)
57+
return
5058

5159

5260
@app.task(queue="web")

0 commit comments

Comments
 (0)