Skip to content

Commit 9b8bbcd

Browse files
authored
Merge pull request #142 from underpass-ai/codex/runtime-saturation-notify-quality-followups
[codex] Address saturation notify quality follow-ups
2 parents e90e01d + 44c4856 commit 9b8bbcd

8 files changed

Lines changed: 363 additions & 44 deletions

File tree

docs/capability-catalog.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ Do not edit manually. Regenerate with `make catalog-docs`.
132132
| `k8s.rollout_pause` | `cluster` | `medium` | `yes` | `reversible` | `guaranteed` |
133133
| `k8s.rollout_status` | `cluster` | `medium` | `yes` | `none` | `guaranteed` |
134134
| `k8s.rollout_undo` | `cluster` | `high` | `yes` | `reversible` | `best-effort` |
135-
| `k8s.scale_deployment` | `cluster` | `medium` | `yes` | `reversible` | `guaranteed` |
135+
| `k8s.scale_deployment` | `cluster` | `medium` | `yes` | `reversible` | `best-effort` |
136136
| `k8s.set_image` | `cluster` | `high` | `yes` | `irreversible` | `` |
137137

138138
## kafka.*

e2e/tests/24-runtime-saturation-notify-tools/test_runtime_saturation_notify_tools.py

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,26 @@ def run(self) -> int:
434434
self.record_step("scale_deployment_succeeded", "passed", {"target_replicas": 2})
435435
print_success("Scale deployment updated the target to 2 replicas")
436436

437-
print_step(6, "Restart pods in label-selector mode deletes only bounded replicas")
437+
print_step(6, "Restart pods without mode is rejected")
438+
status, body, invocation = self.invoke(
439+
session_id=saturation_sid,
440+
tool_name="k8s.restart_pods",
441+
args={
442+
"namespace": self.namespace,
443+
"deployment_name": self.target_name,
444+
},
445+
approved=True,
446+
timeout=180,
447+
)
448+
if status != 200 or invocation is None or invocation.get("status") != "failed":
449+
raise RuntimeError(f"expected failed restart_pods invocation, got status={status}, invocation={invocation}")
450+
error = self.extract_error(invocation, body)
451+
if error.get("code") != "invalid_argument" or error.get("message") != "mode is required":
452+
raise RuntimeError(f"expected invalid_argument/mode is required, got {error}")
453+
self.record_step("restart_pods_mode_required", "passed")
454+
print_success("Restart pods without mode failed with invalid_argument as expected")
455+
456+
print_step(7, "Restart pods in label-selector mode deletes only bounded replicas")
438457
restart_invocation = self._invoke_ok(
439458
session_id=saturation_sid,
440459
tool_name="k8s.restart_pods",
@@ -461,7 +480,7 @@ def run(self) -> int:
461480
self.record_step("restart_pods_succeeded", "passed", {"deleted_pod": deleted_pods[0]})
462481
print_success("Restart pods deleted one bounded replica and the deployment recovered")
463482

464-
print_step(7, "Circuit break installs a bounded network policy")
483+
print_step(8, "Circuit break installs a bounded network policy")
465484
circuit_invocation = self._invoke_ok(
466485
session_id=saturation_sid,
467486
tool_name="k8s.circuit_break",
@@ -489,7 +508,7 @@ def run(self) -> int:
489508
)
490509
print_success("Circuit break installed a NetworkPolicy-backed block")
491510

492-
print_step(8, "Notify escalation succeeds for the e2e route")
511+
print_step(9, "Notify escalation succeeds for the e2e route")
493512
incident_id = f"e2e-incident-{self.run_id}"
494513
status, body, invocation = self.invoke(
495514
session_id=notify_sid,
@@ -529,7 +548,7 @@ def run(self) -> int:
529548
)
530549
print_success("Notify escalation delivered through the configured e2e route")
531550

532-
print_step(9, "Second notify within one minute is rate-limited")
551+
print_step(10, "Second notify within one minute is rate-limited")
533552
status, body, invocation = self.invoke(
534553
session_id=notify_sid,
535554
tool_name="notify.escalation_channel",

internal/adapters/tools/catalog_defaults.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2564,14 +2564,14 @@ capabilities:
25642564
- '{"namespace":"underpass-runtime","deployment_name":"workspace","wait_for_rollout":true,"timeout_seconds":180}'
25652565

25662566
- name: k8s.scale_deployment
2567-
description: Scale a deployment to an absolute replica target or by a bounded delta.
2567+
description: Scale a deployment to an absolute replica target or by a bounded delta; the absolute path is effectively idempotent, but the delta path is best-effort under replay.
25682568
input_schema: '{"type":"object","required":["namespace","deployment_name"],"properties":{"namespace":{"type":"string","maxLength":253},"deployment_name":{"type":"string","maxLength":253},"replicas":{"type":"integer","minimum":0,"maximum":200},"replicas_delta":{"type":"integer","minimum":-50,"maximum":50}},"oneOf":[{"required":["replicas"]},{"required":["replicas_delta"]}],"additionalProperties":false}'
25692569
output_schema: '{"type":"object","properties":{"namespace":{"type":"string"},"deployment_name":{"type":"string"},"previous_replicas":{"type":"integer"},"target_replicas":{"type":"integer"},"applied":{"type":"boolean"},"observed_generation":{"type":"integer"},"summary":{"type":"string"},"output":{"type":"string"},"exit_code":{"type":"integer"}},"required":["namespace","deployment_name","previous_replicas","target_replicas"]}'
25702570
scope: cluster
25712571
side_effects: reversible
25722572
risk_level: medium
25732573
requires_approval: true
2574-
idempotency: guaranteed
2574+
idempotency: best-effort
25752575
constraints:
25762576
timeout_seconds: 30
25772577
output_limit_kb: 16

internal/adapters/tools/catalog_defaults_test.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
package tools
22

33
import (
4+
"encoding/json"
45
"strings"
56
"testing"
7+
8+
"github.com/underpass-ai/underpass-runtime/internal/domain"
69
)
710

811
func TestDefaultCapabilities_Metadata(t *testing.T) {
@@ -231,3 +234,53 @@ func TestValidJSON_Valid(t *testing.T) {
231234
t.Fatalf("unexpected raw JSON: %s", string(raw))
232235
}
233236
}
237+
238+
func TestSaturationNotifyCatalogContracts(t *testing.T) {
239+
capabilities := DefaultCapabilities()
240+
byName := make(map[string]domain.Capability, len(capabilities))
241+
for _, capability := range capabilities {
242+
byName[capability.Name] = capability
243+
}
244+
245+
scale, ok := byName["k8s.scale_deployment"]
246+
if !ok {
247+
t.Fatal("expected k8s.scale_deployment capability")
248+
}
249+
if scale.Idempotency != domain.IdempotencyBestEffort {
250+
t.Fatalf("expected k8s.scale_deployment idempotency=best-effort, got %s", scale.Idempotency)
251+
}
252+
if !strings.Contains(scale.Description, "delta path is best-effort") {
253+
t.Fatalf("expected scale deployment description to explain delta semantics, got %q", scale.Description)
254+
}
255+
256+
restart, ok := byName["k8s.restart_pods"]
257+
if !ok {
258+
t.Fatal("expected k8s.restart_pods capability")
259+
}
260+
var restartSchema struct {
261+
Required []string `json:"required"`
262+
}
263+
if err := json.Unmarshal(restart.InputSchema, &restartSchema); err != nil {
264+
t.Fatalf("unmarshal restart_pods input schema: %v", err)
265+
}
266+
if !containsRequiredString(restartSchema.Required, "mode") {
267+
t.Fatalf("expected restart_pods schema to require mode, got %#v", restartSchema.Required)
268+
}
269+
270+
notify, ok := byName["notify.escalation_channel"]
271+
if !ok {
272+
t.Fatal("expected notify.escalation_channel capability")
273+
}
274+
if notify.Constraints.MaxRetries != notifyEscalationMaxRetries {
275+
t.Fatalf("expected notify max_retries=%d, got %d", notifyEscalationMaxRetries, notify.Constraints.MaxRetries)
276+
}
277+
}
278+
279+
func containsRequiredString(values []string, want string) bool {
280+
for _, value := range values {
281+
if value == want {
282+
return true
283+
}
284+
}
285+
return false
286+
}

internal/adapters/tools/k8s_saturation_tools.go

Lines changed: 70 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,19 @@ type K8sCircuitBreakHandler struct {
3737
client kubernetes.Interface
3838
defaultNamespace string
3939
now func() time.Time
40+
afterFunc func(time.Duration, func()) *time.Timer
4041

4142
mu sync.Mutex
4243
timers map[string]*time.Timer
4344
}
4445

46+
const (
47+
circuitBreakPolicyPrefix = "circuit-break-"
48+
circuitBreakExpiresAtAnnotation = "underpass.ai/expires_at"
49+
circuitBreakTargetServiceAnnotation = "underpass.ai/target_service"
50+
circuitBreakDownstreamAnnotation = "underpass.ai/downstream"
51+
)
52+
4553
func NewK8sScaleDeploymentHandler(client kubernetes.Interface, defaultNamespace string) *K8sScaleDeploymentHandler {
4654
return &K8sScaleDeploymentHandler{client: client, defaultNamespace: strings.TrimSpace(defaultNamespace)}
4755
}
@@ -51,12 +59,66 @@ func NewK8sRestartPodsHandler(client kubernetes.Interface, defaultNamespace stri
5159
}
5260

5361
func NewK8sCircuitBreakHandler(client kubernetes.Interface, defaultNamespace string) *K8sCircuitBreakHandler {
54-
return &K8sCircuitBreakHandler{
62+
return newK8sCircuitBreakHandler(client, defaultNamespace, nil, nil)
63+
}
64+
65+
func newK8sCircuitBreakHandler(
66+
client kubernetes.Interface,
67+
defaultNamespace string,
68+
now func() time.Time,
69+
afterFunc func(time.Duration, func()) *time.Timer,
70+
) *K8sCircuitBreakHandler {
71+
if now == nil {
72+
now = func() time.Time { return time.Now().UTC() }
73+
}
74+
if afterFunc == nil {
75+
afterFunc = time.AfterFunc
76+
}
77+
78+
handler := &K8sCircuitBreakHandler{
5579
client: client,
5680
defaultNamespace: strings.TrimSpace(defaultNamespace),
57-
now: func() time.Time { return time.Now().UTC() },
81+
now: now,
82+
afterFunc: afterFunc,
5883
timers: map[string]*time.Timer{},
5984
}
85+
handler.reconcileExistingPolicies(context.Background())
86+
return handler
87+
}
88+
89+
func (h *K8sCircuitBreakHandler) reconcileExistingPolicies(ctx context.Context) {
90+
if h == nil || h.client == nil {
91+
return
92+
}
93+
94+
namespace := strings.TrimSpace(h.defaultNamespace)
95+
policies, err := h.client.NetworkingV1().NetworkPolicies(namespace).List(ctx, metav1.ListOptions{})
96+
if err != nil {
97+
return
98+
}
99+
100+
now := h.now().UTC()
101+
for i := range policies.Items {
102+
policy := policies.Items[i]
103+
if !strings.HasPrefix(policy.Name, circuitBreakPolicyPrefix) {
104+
continue
105+
}
106+
expiresAtRaw := strings.TrimSpace(policy.Annotations[circuitBreakExpiresAtAnnotation])
107+
if expiresAtRaw == "" {
108+
continue
109+
}
110+
expiresAt, parseErr := time.Parse(time.RFC3339, expiresAtRaw)
111+
if parseErr != nil {
112+
continue
113+
}
114+
115+
ttl := expiresAt.Sub(now)
116+
if ttl <= 0 {
117+
_ = h.client.NetworkingV1().NetworkPolicies(policy.Namespace).Delete(ctx, policy.Name, metav1.DeleteOptions{})
118+
continue
119+
}
120+
h.schedulePolicyCleanup(policy.Namespace, policy.Name, ttl)
121+
}
60122
}
61123

62124
func (h *K8sScaleDeploymentHandler) Name() string {
@@ -166,7 +228,7 @@ func (h *K8sRestartPodsHandler) Invoke(ctx context.Context, session domain.Sessi
166228

167229
mode := strings.TrimSpace(request.Mode)
168230
if mode == "" {
169-
mode = "rollout_restart"
231+
return app.ToolRunResult{}, k8sInvalidArgument("mode is required")
170232
}
171233
output := map[string]any{
172234
k8sDelivKeyNamespace: namespace,
@@ -324,7 +386,7 @@ func buildRestartPodsSelector(deployment *appsv1.Deployment, labelSelector strin
324386

325387
func circuitBreakPolicyID(namespace, targetService, downstream string) string {
326388
hash := sha256.Sum256([]byte(namespace + ":" + targetService + ":" + downstream))
327-
return fmt.Sprintf("circuit-break-%x", hash[:6])
389+
return fmt.Sprintf("%s%x", circuitBreakPolicyPrefix, hash[:6])
328390
}
329391

330392
func buildCircuitBreakNetworkPolicy(
@@ -338,9 +400,9 @@ func buildCircuitBreakNetworkPolicy(
338400
Name: policyID,
339401
Namespace: namespace,
340402
Annotations: map[string]string{
341-
"underpass.ai/target_service": targetService.Name,
342-
"underpass.ai/downstream": downstreamService.Name,
343-
"underpass.ai/expires_at": expiresAt.UTC().Format(time.RFC3339),
403+
circuitBreakTargetServiceAnnotation: targetService.Name,
404+
circuitBreakDownstreamAnnotation: downstreamService.Name,
405+
circuitBreakExpiresAtAnnotation: expiresAt.UTC().Format(time.RFC3339),
344406
},
345407
},
346408
Spec: networkingv1.NetworkPolicySpec{
@@ -380,7 +442,7 @@ func (h *K8sCircuitBreakHandler) schedulePolicyCleanup(namespace, policyID strin
380442
if existing := h.timers[policyID]; existing != nil {
381443
existing.Stop()
382444
}
383-
timer := time.AfterFunc(ttl, func() {
445+
timer := h.afterFunc(ttl, func() {
384446
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
385447
defer cancel()
386448
_ = h.client.NetworkingV1().NetworkPolicies(namespace).Delete(ctx, policyID, metav1.DeleteOptions{})

0 commit comments

Comments
 (0)