Added volume support in app config

akclace · akclace · commit 5d9b9b57eeb8 · 2024-09-25T13:13:03.000-07:00
diff --git a/cmd/clace/app_cmds.go b/cmd/clace/app_cmds.go
@@ -80,6 +80,13 @@ func appCreateCommand(commonFlags []cli.Flag, clientConfig *types.ClientConfig)
 			Usage:   "Set an argument for building the container image. Format is argKey=argValue",
 		})
 
+	flags = append(flags,
+		&cli.StringSliceFlag{
+			Name:    "container-volume",
+			Aliases: []string{"cvol"},
+			Usage:   "Set an container volume entry",
+		})
+
 	flags = append(flags,
 		&cli.StringSliceFlag{
 			Name:    "app-config",
@@ -150,6 +157,8 @@ Examples:
 				cargMap[key] = value
 			}
 
+			containerVolumes := cCtx.StringSlice("container-volume")
+
 			appConfig := cCtx.StringSlice("app-config")
 			confMap := make(map[string]string)
 			for _, def := range appConfig {
@@ -171,6 +180,7 @@ Examples:
 				ParamValues:      paramValues,
 				ContainerOptions: coptMap,
 				ContainerArgs:    cargMap,
+				ContainerVolumes: containerVolumes,
 				AppConfig:        confMap,
 			}
 			var createResult types.AppCreateResponse
diff --git a/cmd/clace/app_update_cmds.go b/cmd/clace/app_update_cmds.go
@@ -266,6 +266,7 @@ func appUpdateMetadataCommand(commonFlags []cli.Flag, clientConfig *types.Client
 			appUpdateAppSpec(commonFlags, clientConfig),
 			appUpdateConfig(commonFlags, clientConfig, "container-option", "copt", types.AppMetadataContainerOptions),
 			appUpdateConfig(commonFlags, clientConfig, "container-arg", "carg", types.AppMetadataContainerArgs),
+			appUpdateConfig(commonFlags, clientConfig, "container-volumes", "cvol", types.AppMetadataContainerVolumes),
 			appUpdateConfig(commonFlags, clientConfig, "app-config", "conf", types.AppMetadataAppConfig),
 		},
 	}
diff --git a/internal/app/app.go b/internal/app/app.go
@@ -460,7 +460,8 @@ func (a *App) loadContainerManager(stripAppPath bool) error {
 	}
 
 	a.containerManager, err = NewContainerManager(a.Logger, a,
-		fileName, a.systemConfig, port, lifetime, scheme, health, buildDir, a.sourceFS, a.paramMap, a.appConfig.Container, stripAppPath)
+		fileName, a.systemConfig, port, lifetime, scheme, health, buildDir,
+		a.sourceFS, a.paramMap, a.appConfig.Container, stripAppPath, a.Metadata.ContainerVolumes)
 	if err != nil {
 		return fmt.Errorf("error creating container manager: %w", err)
 	}
diff --git a/internal/app/container/volume.go b/internal/app/container/volume.go
@@ -21,7 +21,7 @@ func GenVolumeName(appId types.AppId, dirName string) VolumeName {
 
 func (c ContainerCommand) VolumeExists(config *types.SystemConfig, name VolumeName) bool {
 	c.Debug().Msgf("Checking volume exists %s", name)
-	cmd := exec.Command(config.ContainerCommand, "volume", "exists", string(name))
+	cmd := exec.Command(config.ContainerCommand, "volume", "inspect", string(name))
 	output, err := cmd.CombinedOutput()
 	if err != nil {
 		c.Debug().Msgf("volume exists check failed %s %s %s", name, err, output)
diff --git a/internal/app/container_manager.go b/internal/app/container_manager.go
@@ -66,7 +66,8 @@ type ContainerManager struct {
 
 func NewContainerManager(logger *types.Logger, app *App, containerFile string,
 	systemConfig *types.SystemConfig, configPort int64, lifetime, scheme, health, buildDir string, sourceFS appfs.ReadableFS,
-	paramMap map[string]string, containerConfig types.Container, stripAppPath bool) (*ContainerManager, error) {
+	paramMap map[string]string, containerConfig types.Container, stripAppPath bool,
+	containerVolumes []string) (*ContainerManager, error) {
 
 	image := ""
 	volumes := []string{}
@@ -138,6 +139,7 @@ func NewContainerManager(logger *types.Logger, app *App, containerFile string,
 		command:         container.ContainerCommand{Logger: logger},
 		paramMap:        paramMap,
 		volumes:         volumes,
+		extraVolumes:    containerVolumes,
 		containerConfig: containerConfig,
 		stateLock:       sync.RWMutex{},
 		currentState:    ContainerStateUnknown,
@@ -268,6 +270,46 @@ func (m *ContainerManager) GetHealthUrl(appHealthUrl string) string {
 	return healthUrl
 }
 
+func getMapHash(input map[string]string) (string, error) {
+	keys := []string{}
+	for k := range input {
+		keys = append(keys, k)
+	}
+	slices.Sort(keys) // Sort the keys to ensure consistent hash
+
+	hashBuilder := strings.Builder{}
+	for _, paramName := range keys {
+		paramVal := input[paramName]
+		// Default to string
+		hashBuilder.WriteString(paramName)
+		hashBuilder.WriteByte(0)
+		hashBuilder.WriteString(paramVal)
+		hashBuilder.WriteByte(0)
+	}
+
+	sha := sha256.New()
+	if _, err := sha.Write([]byte(hashBuilder.String())); err != nil {
+		return "", err
+	}
+	return hex.EncodeToString(sha.Sum(nil)), nil
+}
+
+func getSliceHash(input []string) (string, error) {
+	slices.Sort(input) // Sort the keys to ensure consistent hash
+
+	hashBuilder := strings.Builder{}
+	for _, v := range input {
+		hashBuilder.WriteString(v)
+		hashBuilder.WriteByte(0)
+	}
+
+	sha := sha256.New()
+	if _, err := sha.Write([]byte(hashBuilder.String())); err != nil {
+		return "", err
+	}
+	return hex.EncodeToString(sha.Sum(nil)), nil
+}
+
 func (m *ContainerManager) GetEnvMap() (map[string]string, string) {
 	paramKeys := []string{}
 	for k := range m.paramMap {
@@ -327,15 +369,8 @@ func (m *ContainerManager) createSpecFiles() ([]string, error) {
 	return created, nil
 }
 
-func (m *ContainerManager) getVolumes() []string {
-	allVolumes := append(m.volumes, m.extraVolumes...)
-	slices.Sort(allVolumes)
-	return slices.Compact(allVolumes)
-}
-
 func (m *ContainerManager) createVolumes() error {
-	allVolumes := m.getVolumes()
-	for _, v := range allVolumes {
+	for _, v := range m.volumes {
 		volumeName := container.GenVolumeName(m.app.Id, v)
 		if !m.command.VolumeExists(m.systemConfig, volumeName) {
 			err := m.command.VolumeCreate(m.systemConfig, volumeName)
@@ -344,19 +379,62 @@ func (m *ContainerManager) createVolumes() error {
 			}
 		}
 	}
+
+	for _, volArg := range m.extraVolumes {
+		parsedName := m.parseVolumeName(volArg)
+		if parsedName == "" {
+			continue
+		}
+		genVolumeName := container.GenVolumeName(m.app.Id, parsedName)
+		if !m.command.VolumeExists(m.systemConfig, genVolumeName) {
+			err := m.command.VolumeCreate(m.systemConfig, genVolumeName)
+			if err != nil {
+				return fmt.Errorf("error creating volume %s: %w", genVolumeName, err)
+			}
+		}
+	}
 	return nil
 }
 
 func (m *ContainerManager) getMountArgs() []string {
-	allVolumes := m.getVolumes()
 	args := []string{}
-	for _, v := range allVolumes {
+	for _, v := range m.volumes {
 		volumeName := container.GenVolumeName(m.app.Id, v)
 		args = append(args, fmt.Sprintf("--mount=type=volume,source=%s,target=%s", volumeName, v))
 	}
+
+	for _, volArg := range m.extraVolumes {
+		parsedName := m.parseVolumeName(volArg)
+		if parsedName == "" {
+			args = append(args, fmt.Sprintf("--volume=%s", volArg))
+		} else {
+			genVolumeName := container.GenVolumeName(m.app.Id, parsedName)
+			split := strings.Split(volArg, ":")
+			var volString string
+			if len(split) > 1 {
+				split[0] = string(genVolumeName)
+				volString = strings.Join(split, ":")
+			} else {
+				volString = string(genVolumeName) + ":" + volArg
+			}
+			args = append(args, fmt.Sprintf("--volume=%s", volString))
+		}
+	}
 	return args
 }
 
+// parseVolumeName gets the first part of the volume definition. It returns "" for a bind
+// mount. Otherwise it returns the volume name
+func (m *ContainerManager) parseVolumeName(arg string) string {
+	split := strings.Split(arg, ":")
+	firstPart := split[0]
+	if len(split) > 1 && len(firstPart) > 0 && firstPart[:1] == "/" {
+		return ""
+	}
+
+	return firstPart
+}
+
 func (m *ContainerManager) DevReload(dryRun bool) error {
 	var imageName container.ImageName = container.ImageName(m.image)
 	if imageName == "" {
@@ -485,13 +563,27 @@ func (m *ContainerManager) getAppHash() (string, error) {
 	}
 
 	_, envHash := m.GetEnvMap()
-	fullHashVal := fmt.Sprintf("%s-%s", sourceHash, envHash)
+
+	coptHash, err := getMapHash(m.app.Metadata.ContainerOptions)
+	if err != nil {
+		return "", fmt.Errorf("error getting copt hash: %w", err)
+	}
+	cargHash, err := getMapHash(m.app.Metadata.ContainerArgs)
+	if err != nil {
+		return "", fmt.Errorf("error getting carg hash: %w", err)
+	}
+	cvolHash, err := getSliceHash(m.app.Metadata.ContainerVolumes)
+	if err != nil {
+		return "", fmt.Errorf("error getting cvol hash: %w", err)
+	}
+	fullHashVal := fmt.Sprintf("%s-%s-%s-%s-%s", sourceHash, envHash, coptHash, cargHash, cvolHash)
 	sha := sha256.New()
 	if _, err := sha.Write([]byte(fullHashVal)); err != nil {
 		return "", err
 	}
 	fullHash := hex.EncodeToString(sha.Sum(nil))
-	m.Debug().Msgf("Source hash %s Env hash %s Full hash %s", sourceHash, envHash, fullHash)
+	m.Debug().Msgf("Source hash %s Env hash %s copt hash %s args hash %s cvol hash %s Full hash %s",
+		sourceHash, envHash, coptHash, cargHash, cvolHash, fullHash)
 	return fullHash, nil
 }
 
diff --git a/internal/server/app_apis.go b/internal/server/app_apis.go
@@ -95,6 +95,7 @@ func (s *Server) CreateApp(ctx context.Context, appPath string, approve, dryRun
 	appEntry.Metadata.ParamValues = appRequest.ParamValues
 	appEntry.Metadata.ContainerOptions = appRequest.ContainerOptions
 	appEntry.Metadata.ContainerArgs = appRequest.ContainerArgs
+	appEntry.Metadata.ContainerVolumes = appRequest.ContainerVolumes
 	appEntry.Metadata.AppConfig = appRequest.AppConfig
 
 	auditResult, err := s.createApp(ctx, &appEntry, approve, dryRun, appRequest.GitBranch, appRequest.GitCommit, appRequest.GitAuthName)
diff --git a/internal/server/app_updates.go b/internal/server/app_updates.go
@@ -566,6 +566,11 @@ func (s *Server) updateAppMetadataConfig(appEntry *types.AppEntry, configType ty
 		return nil
 	}
 
+	if configType == types.AppMetadataContainerVolumes {
+		appEntry.Metadata.ContainerVolumes = configEntries
+		return nil
+	}
+
 	for _, entry := range configEntries {
 		key, value, ok := strings.Cut(entry, "=")
 
@@ -604,6 +609,7 @@ func (s *Server) updateAppMetadataConfig(appEntry *types.AppEntry, configType ty
 			} else {
 				delete(appEntry.Metadata.AppConfig, key)
 			}
+		// case AppMetadataContainerVolumes not expected here, already handled
 		default:
 			return fmt.Errorf("invalid config type %s", configType)
 		}
diff --git a/internal/types/api.go b/internal/types/api.go
@@ -40,6 +40,7 @@ type CreateAppRequest struct {
 	ParamValues      map[string]string `json:"param_values"`
 	ContainerOptions map[string]string `json:"container_options"`
 	ContainerArgs    map[string]string `json:"container_args"`
+	ContainerVolumes []string          `json:"container_volumes"`
 	AppConfig        map[string]string `json:"appconfig"`
 }
 
diff --git a/internal/types/types.go b/internal/types/types.go
@@ -315,6 +315,7 @@ type AppMetadata struct {
 	SpecFiles        *SpecFiles        `json:"spec_files"`
 	ContainerOptions map[string]string `json:"container_options"`
 	ContainerArgs    map[string]string `json:"container_args"`
+	ContainerVolumes []string          `json:"container_volumes"`
 	AppConfig        map[string]string `json:"appconfig"`
 }
 
@@ -398,6 +399,7 @@ const (
 	AppMetadataAppConfig        AppMetadataConfigType = "app_config"
 	AppMetadataContainerOptions AppMetadataConfigType = "container_options"
 	AppMetadataContainerArgs    AppMetadataConfigType = "container_args"
+	AppMetadataContainerVolumes AppMetadataConfigType = "container_volumes"
 )
 
 type AppVersion struct {
diff --git a/tests/test_containers.yaml b/tests/test_containers.yaml
@@ -10,7 +10,7 @@ tests:
   container0030: # setup flask prod app
     command: ../clace app create --spec python-flask --carg PYTHON_VERSION=3.12.4-slim --copt cpu-shares=1000 --approve ./flaskapp /cont_flaskprod
   container0031: # invalid python version carg fails
-    command: ../clace app create --spec python-flask --container-arg PYTHON_VERSION=4invalid-slim --approve ./flaskapp /cont_flaskdev2
+    command: ../clace app create --spec python-flask --cvol /tmp:/atmp --cvol /testvol --container-arg PYTHON_VERSION=4invalid-slim --approve ./flaskapp /cont_flaskdev2
   container0032: # check curl
     command: curl -sS localhost:${HTTP_PORT}/cont_flaskdev2
     stdout: 4invalid

Original file line number	Diff line number	Diff line change
`@@ -266,6 +266,7 @@ func appUpdateMetadataCommand(commonFlags []cli.Flag, clientConfig *types.Client`
`266`	`266`	`appUpdateAppSpec(commonFlags, clientConfig),`
`267`	`267`	`appUpdateConfig(commonFlags, clientConfig, "container-option", "copt", types.AppMetadataContainerOptions),`
`268`	`268`	`appUpdateConfig(commonFlags, clientConfig, "container-arg", "carg", types.AppMetadataContainerArgs),`
	`269`	`+ appUpdateConfig(commonFlags, clientConfig, "container-volumes", "cvol", types.AppMetadataContainerVolumes),`
`269`	`270`	`appUpdateConfig(commonFlags, clientConfig, "app-config", "conf", types.AppMetadataAppConfig),`
`270`	`271`	`},`
`271`	`272`	`}`
Original file line number	Diff line number	Diff line change
`@@ -460,7 +460,8 @@ func (a *App) loadContainerManager(stripAppPath bool) error {`
`460`	`460`	`}`
`461`	`461`
`462`	`462`	`a.containerManager, err = NewContainerManager(a.Logger, a,`
`463`		`- fileName, a.systemConfig, port, lifetime, scheme, health, buildDir, a.sourceFS, a.paramMap, a.appConfig.Container, stripAppPath)`
	`463`	`+ fileName, a.systemConfig, port, lifetime, scheme, health, buildDir,`
	`464`	`+ a.sourceFS, a.paramMap, a.appConfig.Container, stripAppPath, a.Metadata.ContainerVolumes)`
`464`	`465`	`if err != nil {`
`465`	`466`	`return fmt.Errorf("error creating container manager: %w", err)`
`466`	`467`	`}`
Original file line number	Diff line number	Diff line change
`@@ -566,6 +566,11 @@ func (s Server) updateAppMetadataConfig(appEntry types.AppEntry, configType ty`
`566`	`566`	`return nil`
`567`	`567`	`}`
`568`	`568`
	`569`	`+ if configType == types.AppMetadataContainerVolumes {`
	`570`	`+ appEntry.Metadata.ContainerVolumes = configEntries`
	`571`	`+ return nil`
	`572`	`+ }`
	`573`	`+`
`569`	`574`	`for _, entry := range configEntries {`
`570`	`575`	`key, value, ok := strings.Cut(entry, "=")`
`571`	`576`
`@@ -604,6 +609,7 @@ func (s Server) updateAppMetadataConfig(appEntry types.AppEntry, configType ty`
`604`	`609`	`} else {`
`605`	`610`	`delete(appEntry.Metadata.AppConfig, key)`
`606`	`611`	`}`
	`612`	`+ // case AppMetadataContainerVolumes not expected here, already handled`
`607`	`613`	`default:`
`608`	`614`	`return fmt.Errorf("invalid config type %s", configType)`
`609`	`615`	`}`
Original file line number	Diff line number	Diff line change
`@@ -40,6 +40,7 @@ type CreateAppRequest struct {`
`40`	`40`	ParamValues map[string]string `json:"param_values"`
`41`	`41`	ContainerOptions map[string]string `json:"container_options"`
`42`	`42`	ContainerArgs map[string]string `json:"container_args"`
	`43`	+ ContainerVolumes []string `json:"container_volumes"`
`43`	`44`	AppConfig map[string]string `json:"appconfig"`
`44`	`45`	`}`
`45`	`46`
Original file line number	Diff line number	Diff line change
`@@ -315,6 +315,7 @@ type AppMetadata struct {`
`315`	`315`	SpecFiles *SpecFiles `json:"spec_files"`
`316`	`316`	ContainerOptions map[string]string `json:"container_options"`
`317`	`317`	ContainerArgs map[string]string `json:"container_args"`
	`318`	+ ContainerVolumes []string `json:"container_volumes"`
`318`	`319`	AppConfig map[string]string `json:"appconfig"`
`319`	`320`	`}`
`320`	`321`
`@@ -398,6 +399,7 @@ const (`
`398`	`399`	`AppMetadataAppConfig AppMetadataConfigType = "app_config"`
`399`	`400`	`AppMetadataContainerOptions AppMetadataConfigType = "container_options"`
`400`	`401`	`AppMetadataContainerArgs AppMetadataConfigType = "container_args"`
	`402`	`+ AppMetadataContainerVolumes AppMetadataConfigType = "container_volumes"`
`401`	`403`	`)`
`402`	`404`
`403`	`405`	`type AppVersion struct {`