Skip to content

Commit 8df2900

Browse files
committed
fix: mark exporter offline when unregistering
instead of relying on the heartbeat Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
1 parent 9671082 commit 8df2900

2 files changed

Lines changed: 114 additions & 1 deletion

File tree

controller/internal/service/controller_service.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,12 +323,14 @@ func (s *ControllerService) Unregister(
323323
original := client.MergeFrom(exporter.DeepCopy())
324324
exporter.Status.Devices = nil
325325

326+
applyUnregisterStatus(exporter, req.GetReason())
327+
326328
if err := s.Client.Status().Patch(ctx, exporter, original); err != nil {
327329
logger.Error(err, "unable to update exporter status")
328330
return nil, status.Errorf(codes.Internal, "unable to update exporter status: %s", err)
329331
}
330332

331-
logger.Info("exporter unregistered, updated as unavailable")
333+
logger.Info("exporter unregistered, marked offline")
332334

333335
return &pb.UnregisterResponse{}, nil
334336
}
@@ -458,6 +460,23 @@ func protoStatusToString(status pb.ExporterStatus) string {
458460
}
459461
}
460462

463+
func applyUnregisterStatus(exporter *jumpstarterdevv1alpha1.Exporter, reason string) {
464+
exporter.Status.ExporterStatusValue = jumpstarterdevv1alpha1.ExporterStatusOffline
465+
statusMessage := reason
466+
if statusMessage == "" {
467+
statusMessage = "Exporter unregistered"
468+
}
469+
exporter.Status.StatusMessage = statusMessage
470+
syncOnlineConditionWithStatus(exporter)
471+
meta.SetStatusCondition(&exporter.Status.Conditions, metav1.Condition{
472+
Type: string(jumpstarterdevv1alpha1.ExporterConditionTypeRegistered),
473+
Status: metav1.ConditionFalse,
474+
ObservedGeneration: exporter.Generation,
475+
Reason: "Unregister",
476+
Message: "Exporter unregistered from the controller and became offline.",
477+
})
478+
}
479+
461480
// syncOnlineConditionWithStatus updates the Online condition based on ExporterStatusValue.
462481
// This ensures the deprecated Online boolean field in the protobuf API stays consistent
463482
// with the new ExporterStatusValue field.

controller/internal/service/controller_service_test.go

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,100 @@ func TestCheckExporterStatusForDriverCalls(t *testing.T) {
214214
}
215215
}
216216

217+
func TestApplyUnregisterStatus(t *testing.T) {
218+
t.Run("sets offline status with default message", func(t *testing.T) {
219+
exporter := &jumpstarterdevv1alpha1.Exporter{
220+
ObjectMeta: metav1.ObjectMeta{
221+
Name: "test-exporter",
222+
Namespace: "default",
223+
Generation: 1,
224+
},
225+
Status: jumpstarterdevv1alpha1.ExporterStatus{
226+
ExporterStatusValue: jumpstarterdevv1alpha1.ExporterStatusAvailable,
227+
},
228+
}
229+
230+
applyUnregisterStatus(exporter, "")
231+
232+
if exporter.Status.ExporterStatusValue != jumpstarterdevv1alpha1.ExporterStatusOffline {
233+
t.Errorf("ExporterStatusValue = %q, want %q", exporter.Status.ExporterStatusValue, jumpstarterdevv1alpha1.ExporterStatusOffline)
234+
}
235+
if exporter.Status.StatusMessage != "Exporter unregistered" {
236+
t.Errorf("StatusMessage = %q, want %q", exporter.Status.StatusMessage, "Exporter unregistered")
237+
}
238+
})
239+
240+
t.Run("uses provided reason as status message", func(t *testing.T) {
241+
exporter := &jumpstarterdevv1alpha1.Exporter{
242+
ObjectMeta: metav1.ObjectMeta{
243+
Name: "test-exporter",
244+
Namespace: "default",
245+
Generation: 1,
246+
},
247+
Status: jumpstarterdevv1alpha1.ExporterStatus{
248+
ExporterStatusValue: jumpstarterdevv1alpha1.ExporterStatusAvailable,
249+
},
250+
}
251+
252+
applyUnregisterStatus(exporter, "shutting down for maintenance")
253+
254+
if exporter.Status.StatusMessage != "shutting down for maintenance" {
255+
t.Errorf("StatusMessage = %q, want %q", exporter.Status.StatusMessage, "shutting down for maintenance")
256+
}
257+
})
258+
259+
t.Run("sets Online condition to false", func(t *testing.T) {
260+
exporter := &jumpstarterdevv1alpha1.Exporter{
261+
ObjectMeta: metav1.ObjectMeta{
262+
Name: "test-exporter",
263+
Namespace: "default",
264+
Generation: 1,
265+
},
266+
Status: jumpstarterdevv1alpha1.ExporterStatus{
267+
ExporterStatusValue: jumpstarterdevv1alpha1.ExporterStatusAvailable,
268+
},
269+
}
270+
271+
applyUnregisterStatus(exporter, "")
272+
273+
condition := meta.FindStatusCondition(exporter.Status.Conditions,
274+
string(jumpstarterdevv1alpha1.ExporterConditionTypeOnline))
275+
if condition == nil {
276+
t.Fatal("Online condition was not set")
277+
}
278+
if condition.Status != metav1.ConditionFalse {
279+
t.Errorf("Online condition status = %v, want %v", condition.Status, metav1.ConditionFalse)
280+
}
281+
})
282+
283+
t.Run("sets Registered condition to false with message", func(t *testing.T) {
284+
exporter := &jumpstarterdevv1alpha1.Exporter{
285+
ObjectMeta: metav1.ObjectMeta{
286+
Name: "test-exporter",
287+
Namespace: "default",
288+
Generation: 1,
289+
},
290+
}
291+
292+
applyUnregisterStatus(exporter, "")
293+
294+
condition := meta.FindStatusCondition(exporter.Status.Conditions,
295+
string(jumpstarterdevv1alpha1.ExporterConditionTypeRegistered))
296+
if condition == nil {
297+
t.Fatal("Registered condition was not set")
298+
}
299+
if condition.Status != metav1.ConditionFalse {
300+
t.Errorf("Registered condition status = %v, want %v", condition.Status, metav1.ConditionFalse)
301+
}
302+
if condition.Reason != "Unregister" {
303+
t.Errorf("Registered condition reason = %q, want %q", condition.Reason, "Unregister")
304+
}
305+
if condition.Message != "Exporter unregistered from the controller and became offline." {
306+
t.Errorf("Registered condition message = %q, want %q", condition.Message, "Exporter unregistered from the controller and became offline.")
307+
}
308+
})
309+
}
310+
217311
func TestSyncOnlineConditionWithStatus(t *testing.T) {
218312
tests := []struct {
219313
name string

0 commit comments

Comments
 (0)