diff --git a/pkg/provider/modal/client.go b/pkg/provider/modal/client.go index f5ab5a4..137b0fd 100644 --- a/pkg/provider/modal/client.go +++ b/pkg/provider/modal/client.go @@ -202,6 +202,8 @@ func (c *sdkClient) CreateSandbox(ctx context.Context, spec SandboxSpec) (string log.V(1).Info("modal sandbox created", "sandboxID", sb.SandboxID, "imageBuild", imageBuild.String(), "create", time.Since(createStart).String()) + // TODO: stop minting as the placement wait once the SDK exports one (the task-id poll + // behind its unexported ensureTaskID); then mint only for a Pod that declares a port. mintStart := time.Now() cred, err := c.mintCredential(ctx, sb, firstPort(spec.Ports)) if err != nil { @@ -346,16 +348,6 @@ func (c *sdkClient) mintCredential(ctx context.Context, sb *modal.Sandbox, port return Credential{URL: creds.URL, Token: creds.Token}, nil } -// MintConnectCredential implements Client. FromID attaches to a live sandbox and creates -// nothing, which is what lets a credential be minted for one this process never created. -func (c *sdkClient) MintConnectCredential(ctx context.Context, id string, port int) (Credential, error) { - sb, err := c.mc.Sandboxes.FromID(ctx, id, nil) - if err != nil { - return Credential{}, fmt.Errorf("modal: attach sandbox %s: %w", id, err) - } - return c.mintCredential(ctx, sb, port) -} - // modalProbe maps a Pod readinessProbe onto Modal's Probe. Modal supports only // TCP and Exec probes, so an HTTPGet probe degrades to a TCP probe on its port // (readiness ≈ the port accepting connections). Returns (nil, nil) when p is nil diff --git a/pkg/provider/modal/modal.go b/pkg/provider/modal/modal.go index cde6171..d3c2afe 100644 --- a/pkg/provider/modal/modal.go +++ b/pkg/provider/modal/modal.go @@ -101,16 +101,11 @@ type Client interface { // a worker, because the mint blocks on that (see sdkClient.mintCredential) — so a queued // GPU is a call that blocks for minutes, and a successful return means real capacity. // - // Minting is one-shot and there is no read-back, so - // a caller that drops the credential can only get another from MintConnectCredential. + // Minting is one-shot and there is no read-back, so a dropped credential is lost. // A sandbox that could not be given one is unreachable, so a failed mint is an ERROR // with no id, not a zero credential — the sandbox may exist, and the claim tag is what // reclaims it (see sdkClient.CreateSandbox). CreateSandbox(ctx context.Context, spec SandboxSpec) (id string, cred Credential, err error) - // MintConnectCredential mints a NEW credential for a sandbox that already exists, - // which Modal allows from the id alone. Every call returns a different token and none - // can be revoked, so it is only safe where nothing holds the previous one. - MintConnectCredential(ctx context.Context, id string, port int) (Credential, error) // TerminateSandbox terminates a sandbox by id. Must be idempotent: // terminating an already-gone sandbox returns nil. TerminateSandbox(ctx context.Context, id string) error @@ -455,7 +450,8 @@ func (p *Provider) Capabilities() provider.Capabilities { // bounded by Modal's own ~3m43s cap on the mint, not by provisionTimeout. // // The connect credential comes back on this call and only this call, since Modal mints it -// once with no read-back. The caller must persist it or it is lost; see mintCredential. +// once with no read-back, and only for a Pod that declares a containerPort. The caller must +// persist it or it is lost; see mintCredential. func (p *Provider) Provision( ctx context.Context, pod *corev1.Pod, req provider.ProvisionRequest, ) (provider.ProvisionResult, error) { @@ -493,12 +489,17 @@ func (p *Provider) Provision( if err != nil { return provider.ProvisionResult{}, err } - return provider.ProvisionResult{ - InstanceID: id, - Reserved: true, // mintCredential is a blocking call, so the GPU is reserved on return - ConnectURL: cred.URL, - ConnectToken: cred.Token, - }, nil + res := provider.ProvisionResult{ + InstanceID: id, + Reserved: true, // mintCredential is a blocking call, so the GPU is reserved on return + } + // A portless Pod is minted for too, since the mint is the placement wait, and the SDK routes + // its token to 8080. Publishing it would advertise an endpoint nothing declared, so the + // credential is dropped; it is unrevocable but held by no one. + if len(spec.Ports) > 0 { + res.ConnectURL, res.ConnectToken = cred.URL, cred.Token + } + return res, nil } // Terminate implements provider.Provider. Idempotent by the Client contract. The region diff --git a/pkg/provider/modal/modal_test.go b/pkg/provider/modal/modal_test.go index fe8d5ec..a85e3a7 100644 --- a/pkg/provider/modal/modal_test.go +++ b/pkg/provider/modal/modal_test.go @@ -51,15 +51,6 @@ type fakeClient struct { cred Credential // credential CreateSandbox returns alongside its id terminated []string - // The mint path: what MintConnectCredential returns, and what it was asked for. - // mintCnt is the assertion that matters most — minting when a token is already held - // is the one mistake nothing can undo. - mintCred Credential - mintErr error - mintCnt int - mintID string - mintPort int - // logs is the stream SandboxLogs hands back, and logsFor records the id it was // asked for — the one thing the adapter decides on that path. logs string @@ -87,15 +78,6 @@ func (f *fakeClient) CreateSandbox(_ context.Context, spec SandboxSpec) (string, return id, f.cred, nil } -func (f *fakeClient) MintConnectCredential(_ context.Context, id string, port int) (Credential, error) { - f.mintCnt++ - f.mintID, f.mintPort = id, port - if f.mintErr != nil { - return Credential{}, f.mintErr - } - return f.mintCred, nil -} - func (f *fakeClient) TerminateSandbox(_ context.Context, id string) error { f.terminated = append(f.terminated, id) return nil @@ -1293,9 +1275,10 @@ func TestProvision_ReturnsMintedCredential(t *testing.T) { cred: Credential{URL: "https://x.modal.host", Token: "tok-abc"}, } p := newTestProvider(f) + pod := gpuPod("claim-a", "H100", 1) + pod.Spec.Containers[0].Ports = []corev1.ContainerPort{{ContainerPort: 8000}} - res, err := p.Provision(context.Background(), gpuPod("claim-a", "H100", 1), - provider.ProvisionRequest{ClaimName: "claim-a"}) + res, err := p.Provision(context.Background(), pod, provider.ProvisionRequest{ClaimName: "claim-a"}) if err != nil { t.Fatalf("Provision: %v", err) } @@ -1314,6 +1297,29 @@ func TestProvision_ReturnsMintedCredential(t *testing.T) { } } +// A portless Pod is still minted for (the mint is the placement wait), but the credential +// routes to the SDK's default 8080, which the Pod never declared, so none is published. +func TestProvision_PortlessPodPublishesNoCredential(t *testing.T) { + f := &fakeClient{ + createID: "sb-1", + cred: Credential{URL: "https://x.modal.host", Token: "tok-abc"}, + } + p := newTestProvider(f) + + res, err := p.Provision(context.Background(), gpuPod("claim-a", "H100", 1), + provider.ProvisionRequest{ClaimName: "claim-a"}) + if err != nil { + t.Fatalf("Provision: %v", err) + } + if !res.Reserved { + t.Fatal("Reserved = false; the mint still ran, so the GPU is placed") + } + if res.ConnectURL != "" || res.ConnectToken != "" { + t.Fatalf("a portless Pod must carry no credential, got url=%q token set=%t", + res.ConnectURL, res.ConnectToken != "") + } +} + // A sandbox that came up but could not be given a credential is unreachable, and nothing // revisits it: minting is one-shot with no read-back, and the idempotent branch below hands // back no credential. So the create reports a failure and no id, and the Pod fails terminally @@ -1373,8 +1379,7 @@ func adoptable() *fakeClient { Tags: map[string]string{ClaimTagKey: "claim-a"}, Status: statusRunning, }}, - cred: Credential{URL: "https://created.modal.host", Token: "tok-created"}, - mintCred: Credential{URL: "https://minted.modal.host", Token: "tok-minted"}, + cred: Credential{URL: "https://created.modal.host", Token: "tok-created"}, } } @@ -1400,9 +1405,6 @@ func TestProvision_IdempotentReturnsNoCredential(t *testing.T) { if f.createCnt != 0 { t.Fatalf("created %d sandboxes; the claim tag must be adopted, not duplicated", f.createCnt) } - if f.mintCnt != 0 { - t.Fatalf("minted %d credentials for an adopted sandbox; want 0", f.mintCnt) - } if res.ConnectURL != "" || res.ConnectToken != "" { t.Fatalf("an adopted sandbox must carry no credential, got url=%q token set=%t", res.ConnectURL, res.ConnectToken != "")