Skip to content
This repository was archived by the owner on Jan 23, 2026. It is now read-only.

Offline exporters should be Pending and not Unsatisfiable #166

Merged
NickCao merged 1 commit into from
Oct 3, 2025
Merged

Offline exporters should be Pending and not Unsatisfiable #166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 30 additions & 26 deletions internal/controller/lease_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,38 +204,40 @@ func (r *LeaseReconciler) reconcileStatusExporterRef(
return fmt.Errorf("reconcileStatusExporterRef: failed to list matching exporters: %w", err)
}

// Filter out offline exporters
onlineExporters := filterOutOfflineExporters(matchingExporters.Items)

// No matching exporter online, lease unsatisfiable
if len(onlineExporters) == 0 {
lease.SetStatusUnsatisfiable(
"NoExporter",
"There are no online exporter matching the selector, but there are %d matching offline exporters",
len(matchingExporters.Items))
return nil
}

approvedExporters, err := r.attachMatchingPolicies(ctx, lease, onlineExporters)
approvedExporters, err := r.attachMatchingPolicies(ctx, lease, matchingExporters.Items)
if err != nil {
return fmt.Errorf("reconcileStatusExporterRef: failed to handle policy approval: %w", err)
}

if len(approvedExporters) == 0 {
lease.SetStatusUnsatisfiable(
"NoAccess",
"While there are %d online exporters matching the selector, none of them are approved by any policy for your client",
len(onlineExporters))
"While there are %d exporters matching the selector, none of them are approved by any policy for your client",
len(matchingExporters.Items),
)
return nil
}

onlineApprovedExporters := filterOutOfflineExporters(approvedExporters)
if len(onlineApprovedExporters) == 0 {
lease.SetStatusPending(
"Offline",
"While there are %d available exporters (i.e. %s), none of them are online",
len(approvedExporters),
approvedExporters[0].Exporter.Name,
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
result.RequeueAfter = time.Second
return nil
}

// Filter out exporters that are already leased
activeLeases, err := r.ListActiveLeases(ctx, lease.Namespace)
if err != nil {
return fmt.Errorf("reconcileStatusExporterRef: failed to list active leases: %w", err)
}

approvedExporters = attachExistingLeases(approvedExporters, activeLeases.Items)
orderedExporters := orderApprovedExporters(approvedExporters)
onlineApprovedExporters = attachExistingLeases(onlineApprovedExporters, activeLeases.Items)
orderedExporters := orderApprovedExporters(onlineApprovedExporters)

if len(orderedExporters) > 0 && orderedExporters[0].Policy.SpotAccess {
lease.SetStatusUnsatisfiable("SpotAccess",
Expand All @@ -244,11 +246,13 @@ func (r *LeaseReconciler) reconcileStatusExporterRef(
return nil
}

availableExporters := filterOutLeasedExporters(orderedExporters)
availableExporters := filterOutLeasedExporters(onlineApprovedExporters)
if len(availableExporters) == 0 {
lease.SetStatusPending("NotAvailable",
"There are %d approved exporters, but all of them are already leased",
len(approvedExporters))
"There are %d approved exporters, (i.e. %s) but all of them are already leased",
len(onlineApprovedExporters),
onlineApprovedExporters[0].Exporter.Name,
)
result.RequeueAfter = time.Second
return nil
}
Expand Down Expand Up @@ -461,15 +465,15 @@ func filterOutLeasedExporters(exporters []ApprovedExporter) []ApprovedExporter {
}

// filterOutOfflineExporters filters out the exporters that are not online
func filterOutOfflineExporters(matchingExporters []jumpstarterdevv1alpha1.Exporter) []jumpstarterdevv1alpha1.Exporter {
func filterOutOfflineExporters(approvedExporters []ApprovedExporter) []ApprovedExporter {
onlineExporters := slices.DeleteFunc(
matchingExporters,
func(exporter jumpstarterdevv1alpha1.Exporter) bool {
return !true || !meta.IsStatusConditionTrue(
exporter.Status.Conditions,
approvedExporters,
func(approvedExporter ApprovedExporter) bool {
return !meta.IsStatusConditionTrue(
approvedExporter.Exporter.Status.Conditions,
string(jumpstarterdevv1alpha1.ExporterConditionTypeRegistered),
) || !meta.IsStatusConditionTrue(
exporter.Status.Conditions,
approvedExporter.Exporter.Status.Conditions,
string(jumpstarterdevv1alpha1.ExporterConditionTypeOnline),
)
},
Expand Down
133 changes: 132 additions & 1 deletion internal/controller/lease_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,69 @@ var _ = Describe("Lease Controller", func() {
})

When("trying to lease an offline exporter", func() {
It("should fail right away", func() {
It("should set status to pending with offline reason", func() {
lease := leaseDutA2Sec.DeepCopy()

ctx := context.Background()

setExporterOnlineConditions(ctx, testExporter1DutA.Name, metav1.ConditionFalse)
setExporterOnlineConditions(ctx, testExporter2DutA.Name, metav1.ConditionFalse)

Expect(k8sClient.Create(ctx, lease)).To(Succeed())
_ = reconcileLease(ctx, lease)

updatedLease := getLease(ctx, lease.Name)
Expect(updatedLease.Status.ExporterRef).To(BeNil())

Expect(meta.IsStatusConditionTrue(
updatedLease.Status.Conditions,
string(jumpstarterdevv1alpha1.LeaseConditionTypePending),
)).To(BeTrue())

// Check that the condition has the correct reason
condition := meta.FindStatusCondition(updatedLease.Status.Conditions, string(jumpstarterdevv1alpha1.LeaseConditionTypePending))
Expect(condition).NotTo(BeNil())
Expect(condition.Reason).To(Equal("Offline"))
})
})

When("trying to lease approved exporters that are offline", func() {
It("should set status to pending with offline reason", func() {
lease := leaseDutA2Sec.DeepCopy()

ctx := context.Background()

// Create a policy that approves the exporters
policy := &jumpstarterdevv1alpha1.ExporterAccessPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "test-policy",
Namespace: "default",
},
Spec: jumpstarterdevv1alpha1.ExporterAccessPolicySpec{
ExporterSelector: metav1.LabelSelector{
MatchLabels: map[string]string{
"dut": "a",
},
},
Policies: []jumpstarterdevv1alpha1.Policy{
{
Priority: 0,
From: []jumpstarterdevv1alpha1.From{
{
ClientSelector: metav1.LabelSelector{
MatchLabels: map[string]string{
"name": "client",
},
},
},
},
},
},
},
}
Expect(k8sClient.Create(ctx, policy)).To(Succeed())

// Set exporters offline while they are approved by policy
setExporterOnlineConditions(ctx, testExporter1DutA.Name, metav1.ConditionFalse)
setExporterOnlineConditions(ctx, testExporter2DutA.Name, metav1.ConditionFalse)

Expand All @@ -164,10 +222,77 @@ var _ = Describe("Lease Controller", func() {
updatedLease := getLease(ctx, lease.Name)
Expect(updatedLease.Status.ExporterRef).To(BeNil())

Expect(meta.IsStatusConditionTrue(
updatedLease.Status.Conditions,
string(jumpstarterdevv1alpha1.LeaseConditionTypePending),
)).To(BeTrue())

// Check that the condition has the correct reason
condition := meta.FindStatusCondition(updatedLease.Status.Conditions, string(jumpstarterdevv1alpha1.LeaseConditionTypePending))
Expect(condition).NotTo(BeNil())
Expect(condition.Reason).To(Equal("Offline"))
Expect(condition.Message).To(ContainSubstring("none of them are online"))

// Clean up
Expect(k8sClient.Delete(ctx, policy)).To(Succeed())
})
})

When("trying to lease exporters that match selector but are not approved by any policy", func() {
It("should set status to unsatisfiable with NoAccess reason", func() {
lease := leaseDutA2Sec.DeepCopy()

ctx := context.Background()

// Create a policy that does NOT approve the exporters (different client selector)
policy := &jumpstarterdevv1alpha1.ExporterAccessPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "test-policy",
Namespace: "default",
},
Spec: jumpstarterdevv1alpha1.ExporterAccessPolicySpec{
ExporterSelector: metav1.LabelSelector{
MatchLabels: map[string]string{
"dut": "a",
},
},
Policies: []jumpstarterdevv1alpha1.Policy{
{
Priority: 0,
From: []jumpstarterdevv1alpha1.From{
{
ClientSelector: metav1.LabelSelector{
MatchLabels: map[string]string{
"name": "different-client", // Different from testClient
},
},
},
},
},
},
},
}
Expect(k8sClient.Create(ctx, policy)).To(Succeed())

Expect(k8sClient.Create(ctx, lease)).To(Succeed())
_ = reconcileLease(ctx, lease)

updatedLease := getLease(ctx, lease.Name)
Expect(updatedLease.Status.ExporterRef).To(BeNil())

Expect(meta.IsStatusConditionTrue(
updatedLease.Status.Conditions,
string(jumpstarterdevv1alpha1.LeaseConditionTypeUnsatisfiable),
)).To(BeTrue())

// Check that the condition has the correct reason
condition := meta.FindStatusCondition(updatedLease.Status.Conditions, string(jumpstarterdevv1alpha1.LeaseConditionTypeUnsatisfiable))
Expect(condition).NotTo(BeNil())
Expect(condition.Reason).To(Equal("NoAccess"))
Expect(condition.Message).To(ContainSubstring("none of them are approved by any policy"))

// Clean up
Expect(k8sClient.Delete(ctx, policy)).To(Succeed())
})
})

Expand Down Expand Up @@ -225,6 +350,12 @@ var _ = Describe("Lease Controller", func() {
updatedLease.Status.Conditions,
string(jumpstarterdevv1alpha1.LeaseConditionTypePending),
)).To(BeTrue())

// Check that the condition has the correct reason and message format
condition := meta.FindStatusCondition(updatedLease.Status.Conditions, string(jumpstarterdevv1alpha1.LeaseConditionTypePending))
Expect(condition).NotTo(BeNil())
Expect(condition.Reason).To(Equal("NotAvailable"))
Expect(condition.Message).To(ContainSubstring("but all of them are already leased"))
})

It("should be acquired when a valid exporter lease times out", func() {
Expand Down
3 changes: 3 additions & 0 deletions internal/controller/suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ var testClient = &jumpstarterdevv1alpha1.Client{
ObjectMeta: metav1.ObjectMeta{
Name: "client",
Namespace: "default",
Labels: map[string]string{
"name": "client",
},
},
}

Expand Down
Loading