Skip to content

fix aenv service create and rm update image #61

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JacksonMei wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix aenv service create and rm update image #61

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions aenv/src/aenv/client/scheduler_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,6 @@ async def update_env_service(
self,
service_id: str,
replicas: Optional[int] = None,
image: Optional[str] = None,
environment_variables: Optional[Dict[str, str]] = None,
) -> "EnvService":
"""
Expand All @@ -621,7 +620,6 @@ async def update_env_service(
Args:
service_id: Environment service ID
replicas: Optional number of replicas
image: Optional container image
environment_variables: Optional environment variables

Returns:
Expand All @@ -638,7 +636,6 @@ async def update_env_service(

request = EnvServiceUpdateRequest(
replicas=replicas,
image=image,
environment_variables=environment_variables,
)

Expand Down
16 changes: 8 additions & 8 deletions aenv/src/aenv/core/environment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,8 +477,8 @@ async def _call_function(
if ensure_initialized:
await self._ensure_initialized()

logger.info(
f"{self._log_prefix()} Executing function in environment {self.env_name} with url={function_url} proxy_headers={self.proxy_headers}, timeout={timeout}"
logger.debug(
f"{self._log_prefix()} Executing function in environment {self.env_name} with url={function_url}"
)

try:
Expand Down Expand Up @@ -514,8 +514,8 @@ async def _call_function(
if not result.get("success", False):
raise EnvironmentError(result.get("error", "Unknown error"))

logger.info(
f"{self._log_prefix()} Function '{function_url}' executed successfully with result={result}"
logger.debug(
f"{self._log_prefix()} Function '{function_url}' executed successfully"
)
return result.get("data", {})

Expand Down Expand Up @@ -724,8 +724,8 @@ async def _wait_for_healthy(self, timeout: float = 300.0) -> None:
result = ""

while True:
logger.info(
f"{self._log_prefix()} check {self.env_name} health at round {times} with url: {self.aenv_health_url}, last_check_result={result}"
logger.debug(
f"{self._log_prefix()} check {self.env_name} health at round {times} with url: {self.aenv_health_url}"
)

try:
Expand All @@ -745,13 +745,13 @@ async def _wait_for_healthy(self, timeout: float = 300.0) -> None:
or result.get("status") == "healthy"
):
logger.info(
f"{self._log_prefix()} Environment {self.env_name} is healthy"
f"{self._log_prefix()} Environment {self.env_name} is healthy after {times + 1} attempts"
)
return

except Exception as e:
logger.debug(
f"{self._log_prefix()} Health check failed: {str(e)}, retrying..."
f"{self._log_prefix()} Health check attempt {times + 1} failed: {str(e)}, retrying..."
)

if asyncio.get_event_loop().time() - start_time > timeout:
Expand Down
1 change: 0 additions & 1 deletion aenv/src/aenv/core/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ class EnvServiceUpdateRequest(BaseModel):
"""Request to update an environment service."""

replicas: Optional[int] = Field(None, description="Number of replicas")
image: Optional[str] = Field(None, description="Container image")
environment_variables: Optional[Dict[str, str]] = Field(
None, description="Environment variables"
)
Expand Down
41 changes: 16 additions & 25 deletions aenv/src/cli/cmds/service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
- service list: List running services
- service get: Get detailed service information
- service delete: Delete a service
- service update: Update service (replicas, image, env vars)
- service update: Update service (replicas, env vars)
"""
import asyncio
import json
Expand Down Expand Up @@ -157,19 +157,21 @@ def create(
1. CLI parameters (--replicas, --port, --enable-storage)
2. config.json's deployConfig.service (new structure)
3. config.json's deployConfig (legacy flat structure, for backward compatibility)
4. System defaults
4. API Service will fetch missing parameters from envhub if not provided in CLI/config
5. System defaults

Storage creation behavior:
- Use --enable-storage flag to enable persistent storage
- Storage configuration (storageSize, storageName, mountPath) is read from config.json's deployConfig.service
- Storage configuration (storageSize, storageName, mountPath) can be provided in CLI options or config.json
- If not provided, API Service will use values from envhub's deployConfig.service
- When storage is created, replicas must be 1 (enforced by backend)
- storageClass is configured in helm values.yaml deployment, not in config.json

config.json deployConfig.service fields (new structure):
- replicas: Number of replicas (default: 1)
- port: Service port (default: 8080)
- enableStorage: Enable storage by default (default: false, CLI --enable-storage overrides)
- storageSize: Storage size like "10Gi", "20Gi" (required when --enable-storage is used)
- storageSize: Storage size like "10Gi", "20Gi" (used when --enable-storage is set)
- storageName: Storage name (default: environment name)
- mountPath: Mount path (default: /home/admin/data)

Expand All @@ -188,13 +190,13 @@ def create(
# Create using config.json in current directory
aenv service create

# Create with explicit environment name
# Create with explicit environment name (API Service will fetch config from envhub)
aenv service create myapp@1.0.0

# Create with 3 replicas and custom port (no storage)
aenv service create myapp@1.0.0 --replicas 3 --port 8000

# Create with storage enabled (storageSize must be in config.json)
# Create with storage enabled (storageSize from config.json or envhub)
aenv service create myapp@1.0.0 --enable-storage

# Create with environment variables
Expand Down Expand Up @@ -259,10 +261,11 @@ def create(
final_storage_size = service_config.get("storageSize")
if not final_storage_size:
console.print(
"[red]Error:[/red] Storage is enabled but 'storageSize' is not found in config.json's deployConfig.service.\n"
"Please add 'storageSize' (e.g., '10Gi', '20Gi') to deployConfig.service in config.json."
"[yellow]⚠️ Warning:[/yellow] Storage is enabled but 'storageSize' is not found in local config.json.\n"
" API Service will attempt to fetch storageSize from envhub.\n"
" If not found in envhub either, the service creation will fail."
)
raise click.Abort()
# Note: Don't abort here, let API Service try to fetch from envhub
final_storage_name = service_config.get("storageName")
final_mount_path = service_config.get("mountPath")

Expand Down Expand Up @@ -730,11 +733,6 @@ async def _delete():
type=int,
help="Update number of replicas",
)
@click.option(
"--image",
type=str,
help="Update container image",
)
@click.option(
"--env",
"-e",
Expand All @@ -754,32 +752,28 @@ def update_service(
cfg: Config,
service_id: str,
replicas: Optional[int],
image: Optional[str],
environment_variables: tuple,
output: str,
):
"""Update a running service

Can update replicas, image, and environment variables.
Can update replicas and environment variables.

Examples:
# Scale to 5 replicas
aenv service update myapp-svc-abc123 --replicas 5

# Update image
aenv service update myapp-svc-abc123 --image myapp:2.0.0

# Update environment variables
aenv service update myapp-svc-abc123 -e DB_HOST=newhost -e DB_PORT=3306

# Update multiple things at once
aenv service update myapp-svc-abc123 --replicas 3 --image myapp:2.0.0
aenv service update myapp-svc-abc123 --replicas 3 -e DB_HOST=newhost
"""
console = cfg.console.console()

if not replicas and not image and not environment_variables:
if not replicas and not environment_variables:
console.print(
"[red]Error:[/red] At least one of --replicas, --image, or --env must be provided"
"[red]Error:[/red] At least one of --replicas or --env must be provided"
)
raise click.Abort()

Expand All @@ -800,8 +794,6 @@ def update_service(
console.print(f"[cyan]🔄 Updating service:[/cyan] {service_id}")
if replicas is not None:
console.print(f" Replicas: {replicas}")
if image:
console.print(f" Image: {image}")
if env_vars:
console.print(f" Environment Variables: {len(env_vars)} variables")
console.print()
Expand All @@ -814,7 +806,6 @@ async def _update():
return await client.update_env_service(
service_id=service_id,
replicas=replicas,
image=image,
environment_variables=env_vars,
)

Expand Down
40 changes: 31 additions & 9 deletions api-service/controller/env_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,24 @@ func (ctrl *EnvServiceController) CreateEnvService(c *gin.Context) {
if backendEnv.DeployConfig == nil {
backendEnv.DeployConfig = make(map[string]interface{})
}

// Get service config from envhub metadata (support both new nested structure and legacy flat structure)
var serviceConfig map[string]interface{}
if svc, ok := backendEnv.DeployConfig["service"].(map[string]interface{}); ok {
serviceConfig = svc
} else {
// For backward compatibility, fall back to root deployConfig if service config is empty
serviceConfig = backendEnv.DeployConfig
}

// Merge environment variables: CLI request overrides envhub config
if req.EnvironmentVariables != nil {
backendEnv.DeployConfig["environment_variables"] = req.EnvironmentVariables
}

// Override replicas from request, otherwise use envhub config
backendEnv.DeployConfig["replicas"] = req.Replicas
Comment on lines +128 to 129
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The logic for setting replicas is inconsistent with other newly added logic for parameters like port and storageSize. The current implementation unconditionally overrides the replicas value from the request and does not fall back to the serviceConfig from envhub as the comment suggests. This differs from how port, storageSize, etc., are handled, which correctly implement the fallback logic.

To ensure consistency, the logic for replicas should be updated to also fall back to serviceConfig if not provided in the request. Note that for this to work as intended, the default assignment if req.Replicas == 0 { req.Replicas = 1 } (around line 89) should be removed to allow the request's replicas to be 0 when not specified, thus triggering the fallback.

Suggested change
// Override replicas from request, otherwise use envhub config
backendEnv.DeployConfig["replicas"] = req.Replicas
// Override replicas from request, otherwise use envhub config
if req.Replicas > 0 {
backendEnv.DeployConfig["replicas"] = req.Replicas
} else if replicas, ok := serviceConfig["replicas"].(float64); ok {
backendEnv.DeployConfig["replicas"] = int32(replicas)
}


if req.Owner != "" {
backendEnv.DeployConfig["owner"] = req.Owner
}
Expand All @@ -122,24 +136,34 @@ func (ctrl *EnvServiceController) CreateEnvService(c *gin.Context) {
backendEnv.DeployConfig["serviceName"] = req.ServiceName
}

// Storage configuration
// Storage configuration: request overrides envhub config
// If request doesn't provide storageSize but envhub has it in service config, use envhub's value
if req.StorageSize != "" {
backendEnv.DeployConfig["storageSize"] = req.StorageSize
} else if storageSize, ok := serviceConfig["storageSize"].(string); ok {
backendEnv.DeployConfig["storageSize"] = storageSize
}

if req.PVCName != "" {
backendEnv.DeployConfig["pvcName"] = req.PVCName
} else if pvcName, ok := serviceConfig["storageName"].(string); ok {
backendEnv.DeployConfig["pvcName"] = pvcName
}

if req.MountPath != "" {
backendEnv.DeployConfig["mountPath"] = req.MountPath
}
// storageClass is now configured in helm values.yaml, not passed via API
if req.StorageSize != "" {
backendEnv.DeployConfig["storageSize"] = req.StorageSize
} else if mountPath, ok := serviceConfig["mountPath"].(string); ok {
backendEnv.DeployConfig["mountPath"] = mountPath
}

// Service configuration
// Service configuration: request overrides envhub config
if req.Port > 0 {
backendEnv.DeployConfig["port"] = req.Port
} else if port, ok := serviceConfig["port"].(float64); ok {
backendEnv.DeployConfig["port"] = int32(port)
}

// Resource configuration
// Resource configuration: request overrides envhub config
if req.CPURequest != "" {
backendEnv.DeployConfig["cpuRequest"] = req.CPURequest
}
Expand Down Expand Up @@ -216,7 +240,6 @@ func (ctrl *EnvServiceController) DeleteEnvService(c *gin.Context) {
// UpdateEnvServiceRequest represents the request body for updating an EnvService
type UpdateEnvServiceRequest struct {
Replicas *int32 `json:"replicas,omitempty"`
Image *string `json:"image,omitempty"`
EnvironmentVariables *map[string]string `json:"environment_variables,omitempty"`
}

Expand All @@ -238,7 +261,6 @@ func (ctrl *EnvServiceController) UpdateEnvService(c *gin.Context) {
// Build update request
updateReq := &service.UpdateServiceRequest{
Replicas: req.Replicas,
Image: req.Image,
EnvironmentVariables: req.EnvironmentVariables,
}

Expand Down
1 change: 0 additions & 1 deletion api-service/service/schedule_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,6 @@ func (c *ScheduleClient) DeleteService(serviceName string, deleteStorage bool) (
// UpdateServiceRequest represents the request body for updating a service
type UpdateServiceRequest struct {
Replicas *int32 `json:"replicas,omitempty"`
Image *string `json:"image,omitempty"`
EnvironmentVariables *map[string]string `json:"environment_variables,omitempty"`
}

Expand Down
10 changes: 1 addition & 9 deletions controller/pkg/aenvhub_http_server/aenv_service_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,11 +553,10 @@ func (h *AEnvServiceHandler) deleteService(serviceName string, w http.ResponseWr
}
}

// updateService updates a service (replicas, image, env vars)
// updateService updates a service (replicas, env vars)
func (h *AEnvServiceHandler) updateService(serviceName string, w http.ResponseWriter, r *http.Request) {
var updateReq struct {
Replicas *int32 `json:"replicas,omitempty"`
Image *string `json:"image,omitempty"`
EnvironmentVariables *map[string]string `json:"environment_variables,omitempty"`
}

Expand All @@ -584,13 +583,6 @@ func (h *AEnvServiceHandler) updateService(serviceName string, w http.ResponseWr
deployment.Spec.Replicas = updateReq.Replicas
}

// Update image
if updateReq.Image != nil && *updateReq.Image != "" {
for i := range deployment.Spec.Template.Spec.Containers {
deployment.Spec.Template.Spec.Containers[i].Image = *updateReq.Image
}
}

// Update environment variables
if updateReq.EnvironmentVariables != nil {
for i := range deployment.Spec.Template.Spec.Containers {
Expand Down
8 changes: 2 additions & 6 deletions docs/guide/cli.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,28 +726,24 @@ Update a running service's configuration.
# Scale to 5 replicas
aenv service update myapp-svc-abc123 --replicas 5

# Update image
aenv service update myapp-svc-abc123 --image myapp:2.0.0

# Update environment variables
aenv service update myapp-svc-abc123 -e DB_HOST=newhost -e DB_PORT=3306

# Update multiple things at once
aenv service update myapp-svc-abc123 --replicas 3 --image myapp:2.0.0
aenv service update myapp-svc-abc123 --replicas 3 -e DB_HOST=newhost
```

**Options:**

| Option | Short | Description |
|---|---|---|
| `--replicas` | `-r` | Update number of replicas |
| `--image` | | Update container image |
| `--env` | `-e` | Environment variables (KEY=VALUE) |
| `--output` | `-o` | Output format (table/json) |

**Important Notes:**

- At least one of `--replicas`, `--image`, or `--env` must be provided
- At least one of `--replicas` or `--env` must be provided
- Environment variable updates merge with existing variables

### `aenv get` - Get Environment Details
Expand Down
Loading