refactor: provision pt.1 (#544)

alessio-perugini · web-flow · commit c09f117da2ea · 2025-08-21T09:02:16.000+02:00
diff --git a/cmd/arduino-app-cli/internal/servicelocator/servicelocator.go b/cmd/arduino-app-cli/internal/servicelocator/servicelocator.go
@@ -43,11 +43,15 @@ var (
 	})
 
 	GetProvisioner = sync.OnceValue(func() *orchestrator.Provision {
-		pythonImage, _ := getPythonImageAndTag()
+		pythonImage, usedPythonImageTag := getPythonImageAndTag()
+		slog.Debug("Using pythonImage", slog.String("image", pythonImage))
+
 		return f.Must(orchestrator.NewProvision(
 			GetDockerClient(),
-			shouldUseDynamicProvisioning(),
 			pythonImage,
+			usedPythonImageTag,
+			runnerVersion,
+			globalConfig,
 		))
 	})
 
@@ -81,13 +85,7 @@ var (
 	})
 
 	GetStaticStore = sync.OnceValue(func() *store.StaticStore {
-		var baseDir string
-		if GetProvisioner().IsUsingDynamicProvision() {
-			baseDir = GetProvisioner().DynamicProvisionDir().String()
-		} else {
-			baseDir = globalConfig.AssetsDir().Join(runnerVersion).String()
-		}
-		return store.NewStaticStore(baseDir)
+		return store.NewStaticStore(globalConfig.AssetsDir().Join(GetUsedPythonImageTag()).String())
 	})
 
 	GetBrickService = sync.OnceValue(func() *bricks.Service {
@@ -121,9 +119,3 @@ func getPythonImageAndTag() (string, string) {
 	}
 	return pythonImage, usedPythonImageTag
 }
-
-func shouldUseDynamicProvisioning() bool {
-	pythonImage, usedPythonImageTag := getPythonImageAndTag()
-	slog.Debug("Using pythonImage", slog.String("image", pythonImage))
-	return usedPythonImageTag != runnerVersion
-}
diff --git a/internal/orchestrator/config/config.go b/internal/orchestrator/config/config.go
@@ -95,10 +95,10 @@ func (c *Configuration) init() error {
 	if err := c.AppsDir().MkdirAll(); err != nil {
 		return err
 	}
-	if err := c.dataDir.MkdirAll(); err != nil {
+	if err := c.ExamplesDir().MkdirAll(); err != nil {
 		return err
 	}
-	if err := c.ExamplesDir().MkdirAll(); err != nil {
+	if err := c.AssetsDir().MkdirAll(); err != nil {
 		return err
 	}
 	return nil
diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go
@@ -154,7 +154,8 @@ func StartApp(
 				cancel()
 				return
 			}
-			if err := ProvisionApp(ctx, provisioner, bricksIndex, mapped_env, &app, cfg, staticStore); err != nil {
+
+			if err := provisioner.App(ctx, bricksIndex, &app, cfg, mapped_env, staticStore); err != nil {
 				yield(StreamMessage{error: err})
 				return
 			}
diff --git a/internal/orchestrator/orchestrator_test.go b/internal/orchestrator/orchestrator_test.go
@@ -341,6 +341,7 @@ func setTestOrchestratorConfig(t *testing.T) config.Configuration {
 
 	tmpDir := paths.New(t.TempDir())
 	t.Setenv("ARDUINO_APP_CLI__APPS_DIR", tmpDir.Join("apps").String())
+	t.Setenv("ARDUINO_APP_CLI__CONFIG_DIR", tmpDir.Join("config").String())
 	t.Setenv("ARDUINO_APP_CLI__DATA_DIR", tmpDir.Join("data").String())
 	cfg, err := config.NewFromEnv()
 	require.NoError(t, err)
diff --git a/internal/orchestrator/provision.go b/internal/orchestrator/provision.go
@@ -7,11 +7,9 @@ import (
 	"log/slog"
 	"maps"
 	"os"
-	"path/filepath"
 	"regexp"
 	"slices"
 	"strings"
-	"time"
 
 	"github.com/containerd/errdefs"
 
@@ -46,49 +44,39 @@ type service struct {
 	Labels     map[string]string `yaml:"labels,omitempty"`
 }
 
-func ProvisionApp(
-	ctx context.Context,
-	provisioner *Provision,
-	bricksIndex *bricksindex.BricksIndex,
-	mapped_env map[string]string,
-	app *app.ArduinoApp,
-	cfg config.Configuration,
-	staticStore *store.StaticStore,
-) error {
-	start := time.Now()
-	defer func() {
-		slog.Info("Provisioning took", "duration", time.Since(start).String())
-	}()
-	return provisioner.App(ctx, bricksIndex, app, cfg, mapped_env, staticStore)
-}
-
 type Provision struct {
-	docker              command.Cli
-	useDynamicProvision bool
-	pythonImage         string
+	docker      command.Cli
+	pythonImage string
 }
 
 func NewProvision(
 	docker command.Cli,
-	useDynamicProvision bool,
 	pythonImage string,
+	usedPythonImageTag string,
+	pinnedPythonImagTag string,
+	cfg config.Configuration,
 ) (*Provision, error) {
-	provisioner := &Provision{
-		docker:              docker,
-		useDynamicProvision: useDynamicProvision,
-		pythonImage:         pythonImage,
-	}
-
-	dyanmicProvisionDir := provisioner.DynamicProvisionDir()
-	if useDynamicProvision {
-		_ = dyanmicProvisionDir.RemoveAll()
-		_ = dyanmicProvisionDir.MkdirAll()
-		if err := dynamicProvisioning(context.Background(), docker.Client(), pythonImage, dyanmicProvisionDir.String()); err != nil {
+	isDevelopmentMode := usedPythonImageTag != pinnedPythonImagTag
+	if isDevelopmentMode {
+		dynamicProvisionDir := cfg.AssetsDir().Join(usedPythonImageTag)
+		_ = dynamicProvisionDir.RemoveAll()
+		tmpProvisionDir, err := cfg.AssetsDir().MkTempDir("dynamic-provisioning")
+		if err != nil {
+			return nil, fmt.Errorf("failed to perform creation of dynamic provisioning dir: %w", err)
+		}
+		if err := dynamicProvisioning(context.Background(), docker.Client(), pythonImage, tmpProvisionDir.String()); err != nil {
 			return nil, fmt.Errorf("failed to perform dynamic provisioning: %w", err)
 		}
+		if err := tmpProvisionDir.Join(".cache").Rename(dynamicProvisionDir); err != nil {
+			return nil, fmt.Errorf("failed to rename tmp provisioning folder: %w", err)
+		}
+		_ = tmpProvisionDir.RemoveAll()
 	}
 
-	return provisioner, nil
+	return &Provision{
+		docker:      docker,
+		pythonImage: pythonImage,
+	}, nil
 }
 
 func (p *Provision) App(
@@ -103,33 +91,13 @@ func (p *Provision) App(
 		return fmt.Errorf("provisioning failed: arduinoApp is nil")
 	}
 
-	dst := arduinoApp.ProvisioningStateDir().Join("compose")
-	if err := dst.RemoveAll(); err != nil {
-		return fmt.Errorf("failed to remove compose directory: %w", err)
-	}
-	if p.useDynamicProvision {
-		composeDir := paths.New(paths.TempDir().String(), ".cache", "compose")
-		if err := composeDir.CopyDirTo(dst); err != nil {
-			return fmt.Errorf("failed to copy compose directory: %w", err)
-		}
-	} else {
-		if err := staticStore.SaveComposeFolderTo(dst.String()); err != nil {
-			return fmt.Errorf("failed to save compose folder: %w", err)
+	if arduinoApp.ProvisioningStateDir().NotExist() {
+		if err := arduinoApp.ProvisioningStateDir().MkdirAll(); err != nil {
+			return fmt.Errorf("provisioning failed: unable to create .cache")
 		}
 	}
 
-	return generateMainComposeFile(arduinoApp, bricksIndex, p.pythonImage, cfg, mapped_env)
-}
-
-func (p *Provision) IsUsingDynamicProvision() bool {
-	return p.useDynamicProvision
-}
-
-func (p *Provision) DynamicProvisionDir() *paths.Path {
-	if p.useDynamicProvision {
-		return paths.TempDir().Join("dyanmic-provision", ".cache")
-	}
-	return nil
+	return generateMainComposeFile(arduinoApp, bricksIndex, p.pythonImage, cfg, mapped_env, staticStore)
 }
 
 func dynamicProvisioning(
@@ -209,28 +177,33 @@ func generateMainComposeFile(
 	pythonImage string,
 	cfg config.Configuration,
 	mapped_env map[string]string,
+	staticStore *store.StaticStore,
 ) error {
 	slog.Debug("Generating main compose file for the App")
 
 	var composeFiles paths.PathList
 	services := []string{}
 	brickServices := map[string][]string{}
 	for _, brick := range app.Descriptor.Bricks {
-		brickPath := filepath.Join(strings.Split(brick.ID, ":")...)
-		composeFilePath := app.ProvisioningStateDir().Join("compose", brickPath, "brick_compose.yaml")
-		if composeFilePath.Exist() {
-			composeFiles.Add(composeFilePath)
-			svcs, e := extracServicesFromComposeFile(composeFilePath)
-			if e != nil {
-				slog.Error("Failed to extract services from compose file", slog.String("compose_file", composeFilePath.String()), slog.Any("error", e))
-				continue
-			}
-			brickServices[brick.ID] = svcs
-			services = append(services, svcs...)
-			slog.Debug("Brick compose file found", slog.String("module", brick.ID), slog.String("path", composeFilePath.String()))
-		} else {
+		composeFilePath, err := staticStore.GetBrickComposeFilePathFromID(brick.ID)
+		if err != nil {
+			slog.Error("brick compose id not valid", slog.String("error", err.Error()), slog.String("brick_id", brick.ID))
+			continue
+		}
+		if !composeFilePath.Exist() {
 			slog.Debug("Brick compose file not found", slog.String("module", brick.ID), slog.String("path", composeFilePath.String()))
+			continue
+		}
+
+		composeFiles.Add(composeFilePath)
+		svcs, e := extracServicesFromComposeFile(composeFilePath)
+		if e != nil {
+			slog.Error("Failed to extract services from compose file", slog.String("compose_file", composeFilePath.String()), slog.Any("error", e))
+			continue
 		}
+		brickServices[brick.ID] = svcs
+		services = append(services, svcs...)
+		slog.Debug("Brick compose file found", slog.String("module", brick.ID), slog.String("path", composeFilePath.String()))
 	}
 
 	// Create a single docker-mainCompose that includes all the required services
diff --git a/internal/orchestrator/provision_test.go b/internal/orchestrator/provision_test.go
@@ -9,17 +9,19 @@ import (
 
 	"github.com/bcmi-labs/orchestrator/internal/orchestrator/app"
 	"github.com/bcmi-labs/orchestrator/internal/orchestrator/bricksindex"
+	"github.com/bcmi-labs/orchestrator/internal/store"
 
 	yaml "github.com/goccy/go-yaml"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestProvisionAppWithOverrides(t *testing.T) {
 	cfg := setTestOrchestratorConfig(t)
 	tempDirectory := t.TempDir()
 
+	staticStore := store.NewStaticStore(cfg.AssetsDir().String())
+
 	// Define a mock app with bricks that require overrides
 	app := app.ArduinoApp{
 		Name: "TestApp",
@@ -36,10 +38,10 @@ func TestProvisionAppWithOverrides(t *testing.T) {
 		},
 		FullPath: paths.New(tempDirectory),
 	}
+	require.NoError(t, app.ProvisioningStateDir().MkdirAll())
 	// Add compose files for the bricks - video object detection
-	videoObjectDetectionComposePath := paths.New(tempDirectory).Join(".cache").Join("compose").Join("arduino").Join("video_object_detection")
-	err := videoObjectDetectionComposePath.MkdirAll()
-	assert.Nil(t, err, "Failed to create compose directory for video object detection")
+	videoObjectDetectionComposePath := cfg.AssetsDir().Join("compose", "arduino", "video_object_detection")
+	require.NoError(t, videoObjectDetectionComposePath.MkdirAll())
 	composeForVideoObjectDetection := `
 version: '3.8'
 services:
@@ -48,8 +50,8 @@ services:
     ports:
     - "8080:8080"
 `
-	err = videoObjectDetectionComposePath.Join("brick_compose.yaml").WriteFile([]byte(composeForVideoObjectDetection))
-	assert.Nil(t, err, "Failed to write compose file for video object detection")
+	err := videoObjectDetectionComposePath.Join("brick_compose.yaml").WriteFile([]byte(composeForVideoObjectDetection))
+	require.NoError(t, err)
 
 	bricksIndexContent := []byte(`
 bricks:
@@ -80,40 +82,40 @@ bricks:
   - name: EI_OBJ_DETECTION_MODEL
     default_value: /models/ootb/ei/yolo-x-nano.eim
     description: path to the model file`)
-	err = paths.New(tempDirectory, "bricks-list.yaml").WriteFile(bricksIndexContent)
+	err = cfg.AssetsDir().Join("bricks-list.yaml").WriteFile(bricksIndexContent)
 	require.NoError(t, err)
 
 	// Override brick index with custom test content
-	bricksIndex, err := bricksindex.GenerateBricksIndexFromFile(paths.New(tempDirectory))
-	assert.Nil(t, err, "Failed to load bricks index with custom content")
+	bricksIndex, err := bricksindex.GenerateBricksIndexFromFile(cfg.AssetsDir())
+	require.Nil(t, err, "Failed to load bricks index with custom content")
 
 	br, ok := bricksIndex.FindBrickByID("arduino:video_object_detection")
-	assert.True(t, ok, "Brick arduino:video_object_detection should exist in the index")
-	assert.NotNil(t, br, "Brick arduino:video_object_detection should not be nil")
-	assert.Equal(t, "Object Detection", br.Name, "Brick name should match")
+	require.True(t, ok, "Brick arduino:video_object_detection should exist in the index")
+	require.NotNil(t, br, "Brick arduino:video_object_detection should not be nil")
+	require.Equal(t, "Object Detection", br.Name, "Brick name should match")
 
 	// Run the provision function to generate the main compose file
 	env := map[string]string{}
-	err = generateMainComposeFile(&app, bricksIndex, "arduino:appslab-python-apps-base:dev-latest", cfg, env)
+	err = generateMainComposeFile(&app, bricksIndex, "arduino:appslab-python-apps-base:dev-latest", cfg, env, staticStore)
 
 	// Validate that the main compose file and overrides are created
-	assert.Nil(t, err, "Failed to generate main compose file")
+	require.NoError(t, err, "Failed to generate main compose file")
 	composeFilePath := paths.New(tempDirectory).Join(".cache").Join("app-compose.yaml")
-	assert.True(t, composeFilePath.Exist(), "Main compose file should exist")
+	require.True(t, composeFilePath.Exist(), "Main compose file should exist")
 	overridesFilePath := paths.New(tempDirectory).Join(".cache").Join("app-compose-overrides.yaml")
-	assert.True(t, overridesFilePath.Exist(), "Override compose file should exist")
+	require.True(t, overridesFilePath.Exist(), "Override compose file should exist")
 
 	// Open override file and check for the expected override
 	overridesContent, err := overridesFilePath.ReadFile()
-	assert.Nil(t, err, "Failed to read overrides file")
+	require.Nil(t, err, "Failed to read overrides file")
 	type services struct {
 		Services map[string]map[string]interface{} `yaml:"services"`
 	}
 	content := services{}
 	err = yaml.Unmarshal(overridesContent, &content)
-	assert.Nil(t, err, "Failed to unmarshal overrides content")
-	assert.NotNil(t, content.Services["ei-video-obj-detection-runner"], "Override for ei-video-obj-detection-runner should exist")
-	assert.NotNil(t, content.Services["ei-video-obj-detection-runner"]["devices"], "Override for ei-video-obj-detection-runner devices should exist")
+	require.Nil(t, err, "Failed to unmarshal overrides content")
+	require.NotNil(t, content.Services["ei-video-obj-detection-runner"], "Override for ei-video-obj-detection-runner should exist")
+	require.NotNil(t, content.Services["ei-video-obj-detection-runner"]["devices"], "Override for ei-video-obj-detection-runner devices should exist")
 }
 
 func TestVolumeParser(t *testing.T) {
@@ -145,9 +147,9 @@ services:
 			"CUSTOM_PATH": tempDirectory,
 		}
 		volumes, err := extractVolumesFromComposeFile(volumesFromFile.String())
-		assert.Nil(t, err, "Failed to extract volumes from compose file")
+		require.Nil(t, err, "Failed to extract volumes from compose file")
 		provisionComposeVolumes(volumesFromFile.String(), volumes, app, env)
-		assert.True(t, app.FullPath.Join("data").Join("influx-data").Exist(), "Volume directory should exist")
+		require.True(t, app.FullPath.Join("data").Join("influx-data").Exist(), "Volume directory should exist")
 	})
 
 	t.Run("TestPreProvsionVolumesCustomEnvUsingDefault", func(t *testing.T) {
@@ -178,9 +180,9 @@ services:
 		// No env, use macro default value
 		env := map[string]string{}
 		volumes, err := extractVolumesFromComposeFile(volumesFromFile.String())
-		assert.Nil(t, err, "Failed to extract volumes from compose file")
+		require.Nil(t, err, "Failed to extract volumes from compose file")
 		provisionComposeVolumes(volumesFromFile.String(), volumes, app, env)
-		assert.True(t, app.FullPath.Join("customized").Join("data").Join("influx-data").Exist(), "Volume directory should exist")
+		require.True(t, app.FullPath.Join("customized").Join("data").Join("influx-data").Exist(), "Volume directory should exist")
 	})
 
 	t.Run("TestPreProvsionVolumesAsStructure", func(t *testing.T) {
@@ -210,9 +212,9 @@ services:
 		}
 		env := map[string]string{}
 		volumes, err := extractVolumesFromComposeFile(volumesFromFile.String())
-		assert.Nil(t, err, "Failed to extract volumes from compose file")
+		require.Nil(t, err, "Failed to extract volumes from compose file")
 		provisionComposeVolumes(volumesFromFile.String(), volumes, app, env)
-		assert.True(t, app.FullPath.Join("data").Join("influx-data").Exist(), "Volume directory should exist")
+		require.True(t, app.FullPath.Join("data").Join("influx-data").Exist(), "Volume directory should exist")
 	})
 
 	t.Run("TestPreProvsionVolumes", func(t *testing.T) {
@@ -240,9 +242,9 @@ services:
 		}
 		env := map[string]string{}
 		volumes, err := extractVolumesFromComposeFile(volumesFromFile.String())
-		assert.Nil(t, err, "Failed to extract volumes from compose file")
+		require.Nil(t, err, "Failed to extract volumes from compose file")
 		provisionComposeVolumes(volumesFromFile.String(), volumes, app, env)
-		assert.True(t, app.FullPath.Join("data").Join("influx-data").Exist(), "Volume directory should exist")
+		require.True(t, app.FullPath.Join("data").Join("influx-data").Exist(), "Volume directory should exist")
 	})
 
 }
diff --git a/internal/store/store.go b/internal/store/store.go

Original file line number	Diff line number	Diff line change
`@@ -95,10 +95,10 @@ func (c *Configuration) init() error {`
`95`	`95`	`if err := c.AppsDir().MkdirAll(); err != nil {`
`96`	`96`	`return err`
`97`	`97`	`}`
`98`		`- if err := c.dataDir.MkdirAll(); err != nil {`
	`98`	`+ if err := c.ExamplesDir().MkdirAll(); err != nil {`
`99`	`99`	`return err`
`100`	`100`	`}`
`101`		`- if err := c.ExamplesDir().MkdirAll(); err != nil {`
	`101`	`+ if err := c.AssetsDir().MkdirAll(); err != nil {`
`102`	`102`	`return err`
`103`	`103`	`}`
`104`	`104`	`return nil`
Original file line number	Diff line number	Diff line change
`@@ -154,7 +154,8 @@ func StartApp(`
`154`	`154`	`cancel()`
`155`	`155`	`return`
`156`	`156`	`}`
`157`		`- if err := ProvisionApp(ctx, provisioner, bricksIndex, mapped_env, &app, cfg, staticStore); err != nil {`
	`157`	`+`
	`158`	`+ if err := provisioner.App(ctx, bricksIndex, &app, cfg, mapped_env, staticStore); err != nil {`
`158`	`159`	`yield(StreamMessage{error: err})`
`159`	`160`	`return`
`160`	`161`	`}`