|
| 1 | +# Copilot Instructions for Tenama |
| 2 | + |
| 3 | +This document provides guidance for AI agents contributing to the tenama project. |
| 4 | + |
| 5 | +## Project Overview |
| 6 | + |
| 7 | +**Tenama** is a Kubernetes namespace manager that enables non-cluster-admins to create temporary namespaces with automatic lifecycle management and optional global resource limits. |
| 8 | + |
| 9 | +**Architecture**: Event-driven with Kubernetes Watch API (no polling) |
| 10 | +**Language**: Go 1.25 |
| 11 | +**Key Dependencies**: k8s.io/client-go v0.34.2, Echo v4, gopkg.in/yaml.v2 |
| 12 | + |
| 13 | +## Critical Architecture Patterns |
| 14 | + |
| 15 | +### 1. Event-Driven Resource Tracking (Real-Time, No Polling) |
| 16 | + |
| 17 | +The system uses Kubernetes Watch API for **immediate** namespace lifecycle events: |
| 18 | + |
| 19 | +**File**: `internal/handlers/watcher.go` (431 lines) |
| 20 | + |
| 21 | +**Key Pattern**: |
| 22 | + |
| 23 | +``` |
| 24 | +Watch Event Stream: |
| 25 | + Added → addToResourceTracking(ns) |
| 26 | + Modified → updateResourceTracking(ns) |
| 27 | + Deleted → removeFromResourceTracking(ns.Name) |
| 28 | +``` |
| 29 | + |
| 30 | +**Thread Safety**: All resource tracking uses `sync.RWMutex` + `nsResources` map (namespace → resources) |
| 31 | + |
| 32 | +**When Adding Features**: |
| 33 | + |
| 34 | +- Watch events automatically trigger - no polling needed |
| 35 | +- Always release locks BEFORE calling other methods (deadlock prevention) |
| 36 | +- Namespace resources extracted from labels: `tenama/resource-cpu`, `tenama/resource-memory`, `tenama/resource-storage` |
| 37 | + |
| 38 | +### 2. Validation Gate Pattern (O(1) Checks) |
| 39 | + |
| 40 | +**File**: `internal/handlers/api_namespaces.go` (CreateNamespace handler, ~line 72) |
| 41 | + |
| 42 | +**Pattern**: |
| 43 | + |
| 44 | +```go |
| 45 | +// 1. Check if creation would exceed limits |
| 46 | +if !c.watcher.CanCreateNamespace(requestedResources) { |
| 47 | + return c.sendErrorResponse(..., http.StatusTooManyRequests) |
| 48 | +} |
| 49 | +// 2. Create namespace (actual reservation at ADDED watch event) |
| 50 | +c.createNamespace(ctx, c.clientset, nsSpec, ...) |
| 51 | +``` |
| 52 | + |
| 53 | +**Critical**: Validation is O(1) via map lookup; actual reservation happens when watcher receives ADDED event (not atomic race condition due to Kubernetes eventually-consistent model) |
| 54 | + |
| 55 | +### 3. Configuration with camelCase YAML Fields |
| 56 | + |
| 57 | +**File**: `internal/models/config.go` |
| 58 | + |
| 59 | +**Convention**: ALL YAML tags use camelCase, NOT snake_case: |
| 60 | + |
| 61 | +```yaml |
| 62 | +globalLimits: # NOT global_limits |
| 63 | + enabled: true |
| 64 | + resources: # NOT resource |
| 65 | + requests: |
| 66 | + cpu: "5000m" |
| 67 | + memory: "10Gi" |
| 68 | + storage: "50Gi" |
| 69 | +``` |
| 70 | +
|
| 71 | +**When Editing config.yaml or config.go**: Always maintain camelCase convention |
| 72 | +
|
| 73 | +### 4. Dependency Injection via Container |
| 74 | +
|
| 75 | +**File**: `internal/handlers/container.go` |
| 76 | + |
| 77 | +**Pattern**: |
| 78 | + |
| 79 | +- Single `Container` struct holds clientset, config, and watcher |
| 80 | +- Echo handlers receive Container via method receiver: `func (c *Container) CreateNamespace(ctx echo.Context)` |
| 81 | +- Watcher attached after startup: `c.SetWatcher(watcher)` |
| 82 | + |
| 83 | +**When Adding Handlers**: Attach dependencies to Container, not as globals |
| 84 | + |
| 85 | +### 5. Resource Quantity Handling |
| 86 | + |
| 87 | +**Files**: `internal/handlers/watcher.go`, `internal/handlers/api_namespaces.go` |
| 88 | + |
| 89 | +**Pattern**: Use k8s.io/apimachinery/pkg/api/resource.Quantity for all resource math: |
| 90 | + |
| 91 | +- Never use float64 for resources (precision loss) |
| 92 | +- Use `.Sign()` to check negative values: `if quantity.Sign() < 0` |
| 93 | +- Use `.Add()` and `.Sub()` for accumulation |
| 94 | +- Use `quantity.String()` for display (not `%v` with Quantity objects) |
| 95 | + |
| 96 | +**Helper Function**: `formatResourceQuantity(rl v1.ResourceList, resourceName) string` - converts Quantity to readable string with "not set" fallback |
| 97 | + |
| 98 | +### 6. HTTP Status Code Convention |
| 99 | + |
| 100 | +- **200 OK**: Namespace created successfully |
| 101 | +- **400 Bad Request**: Invalid input (missing infix, parse errors) |
| 102 | +- **409 Conflict**: Namespace already exists |
| 103 | +- **429 Too Many Requests**: Global resource limits exceeded |
| 104 | +- **500 Internal Server Error**: Kubernetes API errors |
| 105 | + |
| 106 | +## File Organization & Key Entry Points |
| 107 | + |
| 108 | +| File | Purpose | Key Exports | |
| 109 | +| ------------------------------------------- | ------------------------------------------- | ----------------------------------------------------- | |
| 110 | +| `cmd/tenama/main.go` | Startup (config load, watcher init, routes) | `convertConfigResourcesToResourceList()` | |
| 111 | +| `internal/handlers/watcher.go` | Event tracking & resource accounting | `NamespaceWatcher`, `Watch()`, `CanCreateNamespace()` | |
| 112 | +| `internal/handlers/api_namespaces.go` | HTTP handlers for namespace CRUD | `CreateNamespace()`, `DeleteNamespace()` | |
| 113 | +| `internal/handlers/api_info.go` | /info endpoint with GlobalLimits status | `GetBuildInfo()`, `quantityMapToStrings()` | |
| 114 | +| `internal/handlers/container.go` | Dependency injection | `Container`, `NewContainer()` | |
| 115 | +| `internal/models/config.go` | Configuration model (YAML parsing) | `Config`, `GlobalLimits`, `Resources` | |
| 116 | +| `internal/handlers/middleware_basicAuth.go` | Authentication | `BasicAuthValidator()` | |
| 117 | + |
| 118 | +## Testing Patterns |
| 119 | + |
| 120 | +### Fake Clientset for Unit Tests |
| 121 | + |
| 122 | +```go |
| 123 | +import "k8s.io/client-go/kubernetes/fake" |
| 124 | +
|
| 125 | +fakeCS := fake.NewSimpleClientset() |
| 126 | +watcher := NewNamespaceWatcher(fakeCS.CoreV1(), "test-") |
| 127 | +``` |
| 128 | + |
| 129 | +### Concurrent Resource Tracking Tests |
| 130 | + |
| 131 | +**File**: `internal/handlers/watcher_test.go` |
| 132 | + |
| 133 | +Test concurrent add/remove operations to verify `sync.RWMutex` safety. Always test: |
| 134 | + |
| 135 | +- Multiple goroutines adding resources simultaneously |
| 136 | +- Read operations during modifications |
| 137 | +- Final state consistency |
| 138 | + |
| 139 | +## Common Tasks & Where to Make Changes |
| 140 | + |
| 141 | +### Adding a New Resource Type (e.g., GPU) |
| 142 | + |
| 143 | +1. **config.go**: Add field to `Resources` struct (e.g., `GPU string`) |
| 144 | +2. **watcher.go**: Update `extractNamespaceResources()` to parse `tenama/resource-gpu` label |
| 145 | +3. **watcher.go**: Resource math in `addToResourceTracking()`, `removeFromResourceTracking()`, `updateResourceTracking()` |
| 146 | +4. **api_namespaces.go**: Add to error message in `formatResourceQuantity()` check |
| 147 | +5. **api_info.go**: Already works via generic `quantityMapToStrings()` |
| 148 | + |
| 149 | +### Fixing Mutex Issues |
| 150 | + |
| 151 | +**Critical Pattern** (from recent bug fix): |
| 152 | + |
| 153 | +```go |
| 154 | +// ❌ WRONG: defer + manual unlock = double-unlock panic |
| 155 | +func method() { |
| 156 | + nw.resourceMu.Lock() |
| 157 | + defer nw.resourceMu.Unlock() |
| 158 | + nw.resourceMu.Unlock() // PANIC! |
| 159 | +} |
| 160 | +
|
| 161 | +// ✅ CORRECT: defer OR manual unlock, never both |
| 162 | +func method() { |
| 163 | + nw.resourceMu.Lock() |
| 164 | + // ... do work ... |
| 165 | + nw.resourceMu.Unlock() |
| 166 | +} |
| 167 | +``` |
| 168 | + |
| 169 | +If you need to unlock before calling another method: |
| 170 | + |
| 171 | +```go |
| 172 | +nw.resourceMu.Unlock() |
| 173 | +// Safe to call other methods here |
| 174 | +nw.addToResourceTracking(ns) // Has its own Lock() |
| 175 | +``` |
| 176 | + |
| 177 | +### Validation Error Messages |
| 178 | + |
| 179 | +**Convention**: ALL lowercase, specific resource type: |
| 180 | + |
| 181 | +```go |
| 182 | +// ✅ Correct |
| 183 | +"invalid cpu quantity: ..." |
| 184 | +"invalid memory quantity: ..." |
| 185 | +"invalid storage quantity: ..." |
| 186 | +
|
| 187 | +// ❌ Wrong |
| 188 | +"Invalid CPU Quantity" |
| 189 | +"CPU Error" |
| 190 | +``` |
| 191 | + |
| 192 | +## Build & Test Commands |
| 193 | + |
| 194 | +```bash |
| 195 | +# Build |
| 196 | +go build -o tenama ./cmd/tenama |
| 197 | +
|
| 198 | +# Run with debug config |
| 199 | +./tenama -config config/config.yaml |
| 200 | +
|
| 201 | +# Test all handlers & watcher |
| 202 | +go test ./internal/... -v |
| 203 | +
|
| 204 | +# Test with coverage (tracking) |
| 205 | +go test ./internal/handlers/watcher_test.go -v |
| 206 | +``` |
| 207 | + |
| 208 | +## Kubernetes Integration Points |
| 209 | + |
| 210 | +- **Label-based resource extraction**: Namespaces created with `tenama/resource-*` labels |
| 211 | +- **Watch selector**: `created-by=tenama` label required for namespace tracking |
| 212 | +- **Cleanup trigger**: Namespace deletion via DELETED watch event |
| 213 | +- **Resource quota**: Created per namespace in `craftNamespaceQuotaSpecification()` |
| 214 | +- **Service account token**: Kubernetes automatically injects into mounted Secret |
| 215 | + |
| 216 | +## Configuration Considerations |
| 217 | + |
| 218 | +**Production vs Development**: |
| 219 | + |
| 220 | +- **duration**: "168h" (7 days) in production, "30s" for quick testing |
| 221 | +- **globalLimits.enabled**: true/false controls entire feature |
| 222 | +- **logLevel**: "debug" for development, "info"/"warn" for production |
| 223 | +- **basicAuth**: Required for all API endpoints except /info, /docs, /healthz, /readiness |
| 224 | + |
| 225 | +## Known Limitations & Design Decisions |
| 226 | + |
| 227 | +1. **Race Condition (Acknowledged)**: Namespace validation and creation are not atomic. Reservation happens at ADDED watch event, not at API request time. This is acceptable due to Kubernetes eventually-consistent model. |
| 228 | + |
| 229 | +2. **No Polling**: Watcher uses Watch API exclusively. No periodic cleanup interval needed. |
| 230 | + |
| 231 | +3. **camelCase YAML Only**: The config system is strict about camelCase. Always use `globalLimits`, not `global_limits`. |
| 232 | + |
| 233 | +4. **Per-Namespace Resources**: Global limits apply cluster-wide to all tenama-managed namespaces. Per-namespace quotas are separate via ResourceQuota. |
| 234 | + |
| 235 | +## Recent Bug Fixes (Reference) |
| 236 | + |
| 237 | +All fixes implemented in commit 4e03565: |
| 238 | + |
| 239 | +1. ✅ Mutex double-unlock → Manual unlock only in `updateResourceTracking()` |
| 240 | +2. ✅ Negative value validation → `.Sign() < 0` check with warning logs |
| 241 | +3. ✅ Config duration → Restored "168h" production default |
| 242 | +4. ✅ Error capitalization → All lowercase in error messages |
| 243 | +5. ✅ Error formatting → `formatResourceQuantity()` helper for Quantity→string |
| 244 | +6. ✅ Race condition → Documented as acknowledged trade-off |
| 245 | + |
| 246 | +## API Endpoints Overview |
| 247 | + |
| 248 | +| Method | Path | Auth | Returns | Notes | |
| 249 | +| ------ | ----------------- | --------- | ------------------------ | ------------------------------------- | |
| 250 | +| GET | /info | - | BuildInfo + GlobalLimits | No auth needed | |
| 251 | +| POST | /namespace | BasicAuth | Namespace + kubeconfig | Returns HTTP 429 if limits exceeded | |
| 252 | +| GET | /namespace | BasicAuth | List of namespace names | Filtered by `created-by=tenama` | |
| 253 | +| GET | /namespace/{name} | BasicAuth | Namespace found message | Validation only, no resources | |
| 254 | +| DELETE | /namespace/{name} | BasicAuth | Success/error message | Triggers resource cleanup via watcher | |
| 255 | + |
| 256 | +--- |
| 257 | + |
| 258 | +**Last Updated**: After commit 4e03565 (all PR review fixes completed) |
| 259 | +**Tests**: 19/19 passing (watcher, handlers, config models) |
| 260 | +**Status**: Production-ready with event-driven architecture |
0 commit comments