ExporterAccessPolicies improvements/status

[mangelajo]

(autogenerated from sonet 4.5 opus model)

Lease Controller Policy Analysis

This issue analyzes and tracks how the lease controller handles ExporterAccessPolicy resources
and identifies covered and uncovered corner cases.

Overview

The lease controller uses ExporterAccessPolicy resources to determine which clients
can access which exporters, with what priority, and under what constraints.

Policy Structure

type Policy struct {
    Priority        int              // Higher values = higher priority
    From            []From           // Client selectors that can use this policy
    MaximumDuration *metav1.Duration // Optional: max lease duration allowed
    SpotAccess      bool             // If true, lease can be evicted by higher-priority requests
}

type ExporterAccessPolicySpec struct {
    ExporterSelector metav1.LabelSelector // Which exporters this policy applies to
    Policies         []Policy             // List of access rules
}

Policy Resolution Flow

1. No Policies = Open Access

When no ExporterAccessPolicy resources exist in the namespace, all matching exporters
are approved with default settings:

• Priority: 0
• SpotAccess: false

// From attachMatchingPolicies()
if len(policies.Items) == 0 {
    for _, exporter := range onlineExporters {
        approvedExporters = append(approvedExporters, ApprovedExporter{
            Exporter: exporter,
            Policy: Policy{Priority: 0, SpotAccess: false},
        })
    }
    return approvedExporters, nil
}

2. Policy Matching Algorithm

For each exporter matching the lease selector:

1. Find all policies where ExporterSelector matches the exporter's labels
2. For each matching policy, check each From entry
3. If the client's labels match the ClientSelector, apply additional checks:
   • If MaximumDuration is set, verify requested duration doesn't exceed it
   • If all checks pass, add exporter to approved list with that policy's settings

3. MaximumDuration Enforcement

if p.MaximumDuration != nil {
    requestedDuration := time.Duration(0)
    if lease.Spec.Duration != nil {
        requestedDuration = lease.Spec.Duration.Duration
    } else if lease.Spec.BeginTime != nil && lease.Spec.EndTime != nil {
        requestedDuration = lease.Spec.EndTime.Sub(lease.Spec.BeginTime.Time)
    }
    if requestedDuration > p.MaximumDuration.Duration {
        continue // Skip this policy, exporter not approved via this path
    }
}

Current behavior: If the requested duration exceeds MaximumDuration, the exporter
is silently skipped for that policy. If another policy exists without duration limits,
that policy may still approve the exporter.

4. Exporter Ordering Priority

After collecting all approved exporters, they are sorted by:

1. Lease status: Unleased exporters before leased ones
2. Spot access: Non-spot access before spot access
3. Priority: Higher priority values first
4. Name: Alphabetical (for deterministic selection)

func orderApprovedExporters(exporters []ApprovedExporter) []ApprovedExporter {
    cmpFunc := func(a, b ApprovedExporter) int {
        // 1. Prefer unleased
        if a.ExistingLease != nil && b.ExistingLease == nil { return 1 }
        if a.ExistingLease == nil && b.ExistingLease != nil { return -1 }

        // 2. Prefer non-spot
        if a.Policy.SpotAccess != b.Policy.SpotAccess {
            if a.Policy.SpotAccess { return 1 }
            return -1
        }

        // 3. Higher priority first
        if a.Policy.Priority != b.Policy.Priority {
            return b.Policy.Priority - a.Policy.Priority
        }

        // 4. Alphabetical
        return strings.Compare(a.Exporter.Name, b.Exporter.Name)
    }
    slices.SortFunc(exporters, cmpFunc)
    return exporters
}

5. Spot Access (NOT IMPLEMENTED)

If the best available option is spot-access only, the lease becomes Unsatisfiable:

if len(orderedExporters) > 0 && orderedExporters[0].Policy.SpotAccess {
    lease.SetStatusUnsatisfiable("SpotAccess",
        "The only possible exporters are under spot access (i.e. %s), "+
        "but spot access is still not implemented",
        orderedExporters[0].Exporter.Name)
    return nil
}

Status Conditions

Condition	When Set
Invalid	Empty selector provided
Unsatisfiable	No matching exporters, no approved policies, or spot-access only
Pending	Exporters exist but are offline or already leased
Ready	Exporter successfully acquired

Test Coverage Analysis

✅ Covered Scenarios

Scenario	Test Location
Empty selector	When trying to lease with an empty selector
No matching exporters	When trying to lease a non existing exporter
All matching exporters offline	When trying to lease an offline exporter
Policy exists but client doesn't match	When trying to lease exporters that match selector but are not approved by any policy
All approved exporters already leased	When trying to lease a busy exporter
Ordering by priority	orderApprovedExporters unit tests
Ordering by spot access	orderApprovedExporters unit tests
Mixed ordering criteria	mixed priorities, spot access, lease status test

⚠️ Gaps and Uncovered Cases

Corner Case	Current Behavior	Issue
MaximumDuration exceeded by ALL policies	Silent skip → Unsatisfiable: NoAccess	Misleading error message - says "no policies approve" instead of "duration too long"
Two policies: one with short MaxDuration, one without	Uses policy without MaxDuration	Works correctly but no test coverage
Same exporter approved by multiple policies	Exporter appears multiple times in approved list	May cause unexpected selection behavior
Spot access eviction logic	Code exists in filterOutLeasedExporters but early return prevents execution	Dead code - spot eviction can never execute
Policy with empty From list	Never matches any client	Silent failure to match
Policy with empty ExporterSelector	Matches all exporters	Might be unintended broad access
Duration=0 or nil with no EndTime	requestedDuration = 0, always passes MaximumDuration	Lease with no duration limit might be unintended
Exporter goes offline after selection (scheduled lease)	No re-check when scheduled lease starts	Could assign to offline exporter
Concurrent lease requests for same exporter	No coordination between concurrent reconciles	Race condition acknowledged in TODO

Known TODOs in Code

1. MaximumDuration Reporting (line 370-372)

// TODO: we probably should keep this on the list of approved exporters
// but mark as excessive duration so we can report it on the status
// of lease if no other options exist### 2. Concurrent Lease Scheduling (line 281-288)
// TODO: here there's room for improvement, i.e. we could have multiple
// clients trying to lease the same exporters, we should look at priorities
// and spot access to decide which client gets the exporter, this probably means
// that we will need to construct a lease scheduler with the view of all leases
// and exporters in the system, and (maybe) a priority queue for the leases.### 3. Spot Access Eviction (line 293)
// TODO: Implement eviction of spot access leases## Recommended Test Cases to Add

1. MaximumDuration Scenarios

When("requesting duration exceeds MaximumDuration on ALL matching policies", func() {
    It("should set Unsatisfiable with a duration-specific reason", func() {
        // Create policy with MaximumDuration: 1h
        // Request lease with Duration: 2h
        // Verify error message mentions duration, not just "no access"
    })
})

When("two policies exist: one with short MaximumDuration, one unlimited", func() {
    It("should use the policy without duration limit", func() {
        // Create policy A: MaximumDuration: 1h, Priority: 10
        // Create policy B: no MaximumDuration, Priority: 5
        // Request lease with Duration: 2h
        // Verify lease is approved with Priority: 5
    })
})

### 2. Multiple Policy Matching
When("same exporter matches multiple policies with different priorities", func() {
    It("should select based on highest non-spot priority", func() {
        // Create two policies targeting same exporter
        // Policy A: Priority 10, SpotAccess: false
        // Policy B: Priority 100, SpotAccess: true
        // Verify lease uses Priority 10 (non-spot preferred)
    })
})

3. Edge Cases

When("policy has empty From list", func() {
    It("should not approve any clients", func() {
        // Create policy with ExporterSelector matching, but From: []
        // Verify Unsatisfiable: NoAccess
    })
})

When("lease has no Duration and no EndTime", func() {
    It("should handle MaximumDuration check correctly", func() {
        // Verify behavior when requestedDuration would be 0
    })
})

When("scheduled lease's exporter goes offline before BeginTime", func() {
    It("should handle the offline state appropriately", func() {
        // Create scheduled lease 2s in future
        // Mark exporter offline before BeginTime
        // Verify behavior when lease tries to start
    })
})

### 4. Spot Access (when implemented)
When("spot access lease exists and higher priority client requests", func() {
    It("should evict the spot access lease", func() {
        // This is currently blocked by "not implemented" check
    })
})

Architecture Recommendations

1. Better Duration Error Reporting: Track why each exporter was rejected and report
   the most relevant reason (duration exceeded vs. no policy match)

2. Lease Scheduler: Implement a centralized scheduler to handle concurrent requests
   with priority-based arbitration

3. Spot Access Implementation: Complete the eviction logic that's already partially
   implemented in filterOutLeasedExporters

4. Validation Webhook: Add validation for policies to catch issues like empty From
   lists or conflicting settings

5. Re-validation for Scheduled Leases: When a scheduled lease's BeginTime arrives,
   re-verify exporter status before assigning