Skip to content

Commit bddcbbe

Browse files
durable fixes (#1724)
<!-- This is an auto-generated description by cubic. --> ## Summary by cubic Fixes durable disk handling and private-pool fallback, and simplifies the `beta9` database CLI by nesting commands under `db`, removing unused flags, and cleaning output. Also ensures build containers always resolve the worker address. - **Bug Fixes** - Sync durable disks during cleanup only when a durable disk is mounted; default the durable-disks host path to `<storagePath>/durable-disks` (else `/var/lib/beta9/durable-disks`). - Always resolve the worker address for build containers; remove the `ClipVersion2` TTL refresh/early-return; handle durable-disk requests before private-pool fallback. - **New Features** - Group database commands under `db` (`db postgres`, `db redis`); drop the unused `--size` flag from `db create/scale` and trim unneeded fields from output; errors use the active CLI name. <sup>Written for commit f19b3c7. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/beam-cloud/beta9/pull/1724?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
1 parent a8243bb commit bddcbbe

8 files changed

Lines changed: 89 additions & 34 deletions

File tree

pkg/abstractions/image/builder.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,6 @@ func (b *Builder) startBuildContainer(ctx context.Context, build *Build) error {
100100
return err
101101
}
102102

103-
if b.config.ImageService.ClipVersion == uint32(types.ClipVersion2) {
104-
go b.refreshBuildContainerTTL(ctx, build.containerID)
105-
return nil
106-
}
107-
108103
hostname, err := b.containerRepo.GetWorkerAddress(ctx, build.containerID)
109104
if err != nil {
110105
build.log(true, "Failed to connect to build container.\n")

pkg/common/config.default.yaml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,8 @@ worker:
180180
- --overlay2=none
181181
- --file-access=shared
182182
# Optional per-pool durable disk and cache overrides for nodes with different storage layouts.
183-
# durableDisksPath: /var/lib/beta9/durable-disks
183+
# Defaults to <storagePath>/durable-disks when storagePath is set, otherwise /var/lib/beta9/durable-disks.
184+
# durableDisksPath: /mnt/nvme/beta9/durable-disks
184185
# cache:
185186
# disk:
186187
# hostPath: /mnt/nvme/beta9/cache
@@ -210,7 +211,8 @@ worker:
210211
- --overlay2=none
211212
- --file-access=shared
212213
# Optional per-pool durable disk and cache overrides.
213-
# durableDisksPath: /var/lib/beta9/durable-disks
214+
# Defaults to <storagePath>/durable-disks when storagePath is set, otherwise /var/lib/beta9/durable-disks.
215+
# durableDisksPath: /mnt/build-cache/beta9/durable-disks
214216
# cache:
215217
# disk:
216218
# hostPath: /mnt/build-cache/beta9/cache

pkg/scheduler/pool.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"path/filepath"
78
"strconv"
89
"strings"
910
"time"
@@ -121,6 +122,9 @@ func workerDurableDisksHostPath(poolConfig types.WorkerPoolConfig) string {
121122
if poolConfig.DurableDisksPath != "" {
122123
return poolConfig.DurableDisksPath
123124
}
125+
if poolConfig.StoragePath != "" {
126+
return filepath.Join(poolConfig.StoragePath, "durable-disks")
127+
}
124128
return types.DefaultDurableDisksPath
125129
}
126130

pkg/scheduler/pool_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,11 @@ func TestWorkerPodCommandUsesInitReaper(t *testing.T) {
8383

8484
func TestWorkerDurableDisksHostPathUsesPoolOverride(t *testing.T) {
8585
assert.Equal(t, types.DefaultDurableDisksPath, workerDurableDisksHostPath(types.WorkerPoolConfig{}))
86+
assert.Equal(t, "/mnt/nvme/beta9/storage/durable-disks", workerDurableDisksHostPath(types.WorkerPoolConfig{
87+
StoragePath: "/mnt/nvme/beta9/storage",
88+
}))
8689
assert.Equal(t, "/mnt/nvme/beta9/disks", workerDurableDisksHostPath(types.WorkerPoolConfig{
90+
StoragePath: "/mnt/nvme/beta9/storage",
8791
DurableDisksPath: "/mnt/nvme/beta9/disks",
8892
}))
8993
}

pkg/scheduler/private_pool_fallback.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,12 @@ func (a *schedulingAttempt) tryPrivatePoolFallback() bool {
3535
}
3636

3737
func (a *schedulingAttempt) privatePoolFallbackRequest() (*types.ContainerRequest, string, bool) {
38-
if a == nil || a.request == nil || a.request.HasDurableDiskMount() {
38+
if a == nil || a.request == nil {
39+
return nil, "", false
40+
}
41+
if a.request.HasDurableDiskMount() {
42+
// Durable disk fallback is handled before scheduling so snapshot
43+
// availability is checked before the pool selector is cleared.
3944
return nil, "", false
4045
}
4146

pkg/worker/lifecycle.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,19 +148,21 @@ func (s *Worker) finalizeContainer(containerId string, request *types.ContainerR
148148
}
149149

150150
func (s *Worker) clearContainer(containerId string, request *types.ContainerRequest, exitCode int) {
151+
if request != nil && request.HasDurableDiskMount() {
152+
if err := s.syncDurableDiskMounts(request); err != nil {
153+
log.Error().Str("container_id", containerId).Err(err).Msg("failed to sync durable disks during container cleanup")
154+
}
155+
}
156+
151157
s.setContainerExitCode(containerId, exitCode)
152158

153-
// Set container exit code on instance before any slower cleanup work.
159+
// Keep the local instance state consistent with the reported exit code.
154160
instance, exists := s.containerInstances.Get(containerId)
155161
if exists {
156162
instance.ExitCode = exitCode
157163
s.containerInstances.Set(containerId, instance)
158164
}
159165

160-
if err := s.syncDurableDiskMounts(request); err != nil {
161-
log.Error().Str("container_id", containerId).Err(err).Msg("failed to sync durable disks during container cleanup")
162-
}
163-
164166
s.containerLock.Lock()
165167

166168
// De-allocate GPU devices so they are available for new containers

sdk/src/beta9/cli/database.py

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,17 @@ def common(**_):
4545
pass
4646

4747

48-
@common.group(name="postgres", help="Create and manage Postgres services.")
48+
@common.group(name="db", help="Create and manage database services.")
49+
def db():
50+
pass
51+
52+
53+
@db.group(name="postgres", help="Create and manage Postgres services.")
4954
def postgres():
5055
pass
5156

5257

53-
@common.group(name="redis", help="Create and manage Redis services.")
58+
@db.group(name="redis", help="Create and manage Redis services.")
5459
def redis():
5560
pass
5661

@@ -172,12 +177,21 @@ def _deployments_by_name(service: ServiceClient, name: str):
172177
return [deployment for deployment in res.deployments if deployment.name == name]
173178

174179

180+
def _cli_name() -> str:
181+
ctx = click.get_current_context(silent=True)
182+
if ctx is not None and ctx.command_path:
183+
return ctx.command_path.split()[0]
184+
return "beta9"
185+
186+
175187
def _ensure_database_name_available(service: ServiceClient, product: DatabaseProduct, name: str) -> None:
176188
if _deployment_by_name(service, name) is None:
177189
return
190+
cli_name = _cli_name()
178191
raise click.ClickException(
179192
f"{product.kind} service {name!r} already exists. "
180-
f"Use `beam {product.kind} credentials {name}`, `beam {product.kind} status {name}`, "
193+
f"Use `{cli_name} db {product.kind} credentials {name}`, "
194+
f"`{cli_name} db {product.kind} status {name}`, "
181195
f"or delete it before creating a replacement."
182196
)
183197

@@ -247,9 +261,7 @@ def _print_result(format: str, payload: Dict[str, object]) -> None:
247261
"name",
248262
"kind",
249263
"deployment_id",
250-
"version",
251264
"disk",
252-
"disk_size",
253265
"host",
254266
"username",
255267
"database",
@@ -521,7 +533,7 @@ def _deploy_database_service(
521533
cpu=cpu,
522534
memory=memory,
523535
)
524-
stub_id, deployment_id, version = _deploy_database_stub(service, db_service)
536+
stub_id, deployment_id, _ = _deploy_database_stub(service, db_service)
525537
host = _tcp_host_for_stub(service, stub_id, deployment_id)
526538
if product.kind == "postgres":
527539
connection_url = _postgres_url(username, password, host, database)
@@ -535,9 +547,7 @@ def _deploy_database_service(
535547
"name": name,
536548
"kind": product.kind,
537549
"deployment_id": deployment_id,
538-
"version": version,
539550
"disk": db_service.disks[0].name,
540-
"disk_size": db_service.disks[0].size,
541551
"host": host,
542552
"username": username,
543553
"connection_string": connection_url,
@@ -570,7 +580,6 @@ def _create_options(func):
570580
help="Minimum database replicas to keep warm. Use 1 to keep the service warm.",
571581
)(func)
572582
func = click.option("--pool", type=click.STRING, default=None, help="Run on a private pool.")(func)
573-
func = click.option("--size", type=click.STRING, default=None, help="Durable disk size.")(func)
574583
func = click.option("--password-stdin", is_flag=True, help="Read password from stdin.")(func)
575584
func = click.option("--password-from-env", type=click.STRING, default="", help="Read password from an environment variable.")(func)
576585
func = click.option("--password", type=click.STRING, default="", help="Database password. Generated if omitted.")(func)
@@ -591,7 +600,6 @@ def create_postgres(
591600
password: str,
592601
password_from_env: str,
593602
password_stdin: bool,
594-
size: str,
595603
pool: Optional[str],
596604
min_replicas: int,
597605
format: str,
@@ -605,7 +613,7 @@ def create_postgres(
605613
username=username or name.replace("-", "_"),
606614
database=database or name.replace("-", "_") or POSTGRES.default_database,
607615
password=_password(password, password_from_env, password_stdin),
608-
size=size or POSTGRES.default_size,
616+
size=POSTGRES.default_size,
609617
pool=pool,
610618
min_replicas=min_replicas,
611619
format=format,
@@ -625,7 +633,6 @@ def create_redis(
625633
password: str,
626634
password_from_env: str,
627635
password_stdin: bool,
628-
size: str,
629636
pool: Optional[str],
630637
min_replicas: int,
631638
format: str,
@@ -639,7 +646,7 @@ def create_redis(
639646
username=username or "default",
640647
database="",
641648
password=_password(password, password_from_env, password_stdin),
642-
size=size or REDIS.default_size,
649+
size=REDIS.default_size,
643650
pool=pool,
644651
min_replicas=min_replicas,
645652
format=format,
@@ -795,7 +802,6 @@ def _status(service: ServiceClient, product: DatabaseProduct, name: str, format:
795802
"kind": product.kind,
796803
"deployment_id": deployment.id,
797804
"active": deployment.active,
798-
"version": deployment.version,
799805
"connection_string_secret": _secret_names(product, name)["url"],
800806
}
801807
if format == "json":
@@ -873,7 +879,6 @@ def redis_delete(service: ServiceClient, name: str):
873879

874880
def _scale_options(func):
875881
func = click.option("--format", type=click.Choice(("table", "json")), default="table")(func)
876-
func = click.option("--size", type=click.STRING, default=None, help="Durable disk size to use when redeploying.")(func)
877882
func = click.option("--pool", type=click.STRING, default=None, help="Pool to use when redeploying with new resources.")(func)
878883
func = click.option("--memory", type=click.STRING, default=None, help="Memory to allocate, for example 1024 or 2Gi.")(func)
879884
func = click.option("--cpu", type=click.FLOAT, default=None, help="CPU cores to allocate, for example 0.5 or 2.")(func)
@@ -893,10 +898,9 @@ def postgres_scale(
893898
cpu: Optional[float],
894899
memory: Optional[str],
895900
pool: Optional[str],
896-
size: Optional[str],
897901
format: str,
898902
):
899-
_scale_database(service, POSTGRES, name, always_on, serverless, cpu, memory, pool, size, format)
903+
_scale_database(service, POSTGRES, name, always_on, serverless, cpu, memory, pool, format)
900904

901905

902906
@redis.command(name="scale", help="Scale a Redis service.")
@@ -911,10 +915,9 @@ def redis_scale(
911915
cpu: Optional[float],
912916
memory: Optional[str],
913917
pool: Optional[str],
914-
size: Optional[str],
915918
format: str,
916919
):
917-
_scale_database(service, REDIS, name, always_on, serverless, cpu, memory, pool, size, format)
920+
_scale_database(service, REDIS, name, always_on, serverless, cpu, memory, pool, format)
918921

919922

920923
def _scale_mode_containers(always_on: bool, serverless: bool) -> int:
@@ -932,7 +935,6 @@ def _scale_database(
932935
cpu: Optional[float],
933936
memory: Optional[str],
934937
pool: Optional[str],
935-
size: Optional[str],
936938
format: str,
937939
) -> None:
938940
containers = _scale_mode_containers(always_on, serverless)
@@ -946,7 +948,7 @@ def _scale_database(
946948
username=_get_secret_value(service, secret_names["username"]),
947949
database=_get_secret_value(service, secret_names["database"]) if product.kind == "postgres" else "",
948950
password=_get_secret_value(service, secret_names["password"]),
949-
size=size or product.default_size,
951+
size=product.default_size,
950952
pool=pool,
951953
min_replicas=containers,
952954
format=format,

sdk/tests/test_cli.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import click
2+
import pytest
3+
4+
from beta9.cli import database as database_cli
5+
from beta9.cli.main import load_cli
6+
7+
8+
def test_database_commands_are_nested_under_db():
9+
cli = load_cli(check_config=False)
10+
11+
assert cli.common_group.get_command(None, "postgres") is None
12+
assert cli.common_group.get_command(None, "redis") is None
13+
14+
db = cli.common_group.get_command(None, "db")
15+
assert db is not None
16+
assert sorted(db.commands) == ["postgres", "redis"]
17+
assert "create" in db.commands["postgres"].commands
18+
assert "create" in db.commands["redis"].commands
19+
20+
21+
def test_database_commands_do_not_expose_unenforced_disk_size():
22+
cli = load_cli(check_config=False)
23+
db = cli.common_group.get_command(None, "db")
24+
25+
for product in ("postgres", "redis"):
26+
for command in ("create", "scale"):
27+
options = {param.name for param in db.commands[product].commands[command].params}
28+
assert "size" not in options
29+
30+
31+
def test_database_exists_error_uses_active_cli_name(monkeypatch):
32+
monkeypatch.setattr(database_cli, "_deployment_by_name", lambda service, name: object())
33+
34+
with click.Context(click.Command("create"), info_name="beta9"):
35+
with pytest.raises(click.ClickException) as exc:
36+
database_cli._ensure_database_name_available(None, database_cli.REDIS, "myredis")
37+
38+
message = str(exc.value)
39+
assert "beta9 db redis credentials myredis" in message
40+
assert "beta9 db redis status myredis" in message
41+
assert "beam redis" not in message

0 commit comments

Comments
 (0)