Fixup: Registry cleanBundle

go/runtime/bundle/registry.go

@@ -2,6 +2,7 @@ package bundle
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"maps"
 	"os"
@@ -217,38 +218,32 @@ func (r *registry) CleanStaleBundles(ctx context.Context, runtimeID common.Names
 			return
 		default:
 			if v.Less(active) {
-				r.removeBundle(runtimeID, v, active)
+				r.logger.Info("Removing bundle with version lower then active",
+					"runtimeId", runtimeID,
+					"version", v,
+					"activeVersion", active,
+				)
+				r.cleanBundle(runtimeID, v)
 			}
 		}
 	}
 }
 
-func (r *registry) removeBundle(runtimeID common.Namespace, version version.Version, active version.Version) {
-	r.mu.Lock()
-	manifest, ok := r.manifests[runtimeID][version]
-	if !ok {
-		return
-	}
-	explDir := r.components[runtimeID][component.ID_RONL][version].ExplodedDataDir
-	manifestHash := manifest.Hash()
-	r.logger.Info("Removing (regular) bundle, together with exploded subdir with version lower than active",
-		"active_version", active,
-		"ronl_version", version,
-		"runtimeID", runtimeID,
-		"manifestHash", manifestHash,
-		"exploded_subdir", explDir,
-	)
-
-	delete(r.manifests[runtimeID], version)
-	delete(r.bundles, manifestHash)
-	for _, c := range manifest.Components {
-		delete(r.components[runtimeID][c.ID()], c.Version)
+// cleanBundle removes (regular) bundle from the registry.
+//
+// Additionally, it removes exploded subdir and bundle file from the bundle dir.
+func (r *registry) cleanBundle(runtimeID common.Namespace, version version.Version) {
+	explDir, err := r.removeBundle(runtimeID, version)
+	if err != nil {
+		r.logger.Warn("Bundle clean-up unsuccessful: invalid registry state",
+			"error", err,
+			"id", runtimeID,
+			"runtime_version", version,
+		)
 	}
-	r.mu.Unlock()
 
-	// Don't lock when removing files.
 	if err := os.RemoveAll(explDir); err != nil {
-		r.logger.Error("failed to remove stale exploded (regular) bundle",
+		r.logger.Error("failed to remove stale exploded (regular) bundle dir",
 			"id", runtimeID,
 			"runtime_version", version,
 			"ExplodedDataDir", explDir,
@@ -257,7 +252,7 @@ func (r *registry) removeBundle(runtimeID common.Namespace, version version.Vers
 	}
 
 	if err := os.Remove(explDir + FileExtension); err != nil {
-		r.logger.Error("failed to remove stale (regular) bundle file",
+		r.logger.Error("failed to remove stale exploded (regular) bundle file",
 			"id", runtimeID,
 			"runtime_version", version,
 			"ExplodedDataDir", explDir,
@@ -266,6 +261,39 @@ func (r *registry) removeBundle(runtimeID common.Namespace, version version.Vers
 	}
 }
 
+// removeBundle removes bundle for the runtimeID and version from the registry,
+// if it exists.
+func (r *registry) removeBundle(runtimeID common.Namespace, version version.Version) (string, error) {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	var explDir string
+	manifest, ok := r.manifests[runtimeID][version]
+	if !ok {
+		return explDir, errors.New("registry missing Manifest")
+	}
+	explComp := r.components[runtimeID][component.ID_RONL][version]
+	if explComp == nil {
+		return explDir, errors.New("components missing RONL component")
+	}
+	explDir = explComp.ExplodedDataDir
+	manifestHash := manifest.Hash()
+
+	r.logger.Debug("Removing (regular) bundle from registry",
+		"ronl_version", version,
+		"runtimeID", runtimeID,
+		"manifestHash", manifestHash,
+		"exploded_subdir", explDir,
+	)
+
+	// Removal should be atomic.
+	delete(r.manifests[runtimeID], version)
+	delete(r.bundles, manifestHash)
+	for _, c := range manifest.Components {
+		delete(r.components[runtimeID][c.ID()], c.Version)
+	}
+	return explDir, nil
+}
+
 // GetVersions implements Registry.
 func (r *registry) GetVersions(runtimeID common.Namespace) []version.Version {
 	r.mu.RLock()