From 8df2900ae5ba9f0bd028bbec51bd7fec8f04497f Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Tue, 23 Jun 2026 17:33:30 +0300 Subject: [PATCH] fix: mark exporter offline when unregistering instead of relying on the heartbeat Signed-off-by: Benny Zlotnik --- .../internal/service/controller_service.go | 21 ++++- .../service/controller_service_test.go | 94 +++++++++++++++++++ 2 files changed, 114 insertions(+), 1 deletion(-) diff --git a/controller/internal/service/controller_service.go b/controller/internal/service/controller_service.go index 821aac5df..70d253cd8 100644 --- a/controller/internal/service/controller_service.go +++ b/controller/internal/service/controller_service.go @@ -323,12 +323,14 @@ func (s *ControllerService) Unregister( original := client.MergeFrom(exporter.DeepCopy()) exporter.Status.Devices = nil + applyUnregisterStatus(exporter, req.GetReason()) + if err := s.Client.Status().Patch(ctx, exporter, original); err != nil { logger.Error(err, "unable to update exporter status") return nil, status.Errorf(codes.Internal, "unable to update exporter status: %s", err) } - logger.Info("exporter unregistered, updated as unavailable") + logger.Info("exporter unregistered, marked offline") return &pb.UnregisterResponse{}, nil } @@ -458,6 +460,23 @@ func protoStatusToString(status pb.ExporterStatus) string { } } +func applyUnregisterStatus(exporter *jumpstarterdevv1alpha1.Exporter, reason string) { + exporter.Status.ExporterStatusValue = jumpstarterdevv1alpha1.ExporterStatusOffline + statusMessage := reason + if statusMessage == "" { + statusMessage = "Exporter unregistered" + } + exporter.Status.StatusMessage = statusMessage + syncOnlineConditionWithStatus(exporter) + meta.SetStatusCondition(&exporter.Status.Conditions, metav1.Condition{ + Type: string(jumpstarterdevv1alpha1.ExporterConditionTypeRegistered), + Status: metav1.ConditionFalse, + ObservedGeneration: exporter.Generation, + Reason: "Unregister", + Message: "Exporter unregistered from the controller and became offline.", + }) +} + // syncOnlineConditionWithStatus updates the Online condition based on ExporterStatusValue. // This ensures the deprecated Online boolean field in the protobuf API stays consistent // with the new ExporterStatusValue field. diff --git a/controller/internal/service/controller_service_test.go b/controller/internal/service/controller_service_test.go index 9e506f543..8bbed1e28 100644 --- a/controller/internal/service/controller_service_test.go +++ b/controller/internal/service/controller_service_test.go @@ -214,6 +214,100 @@ func TestCheckExporterStatusForDriverCalls(t *testing.T) { } } +func TestApplyUnregisterStatus(t *testing.T) { + t.Run("sets offline status with default message", func(t *testing.T) { + exporter := &jumpstarterdevv1alpha1.Exporter{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-exporter", + Namespace: "default", + Generation: 1, + }, + Status: jumpstarterdevv1alpha1.ExporterStatus{ + ExporterStatusValue: jumpstarterdevv1alpha1.ExporterStatusAvailable, + }, + } + + applyUnregisterStatus(exporter, "") + + if exporter.Status.ExporterStatusValue != jumpstarterdevv1alpha1.ExporterStatusOffline { + t.Errorf("ExporterStatusValue = %q, want %q", exporter.Status.ExporterStatusValue, jumpstarterdevv1alpha1.ExporterStatusOffline) + } + if exporter.Status.StatusMessage != "Exporter unregistered" { + t.Errorf("StatusMessage = %q, want %q", exporter.Status.StatusMessage, "Exporter unregistered") + } + }) + + t.Run("uses provided reason as status message", func(t *testing.T) { + exporter := &jumpstarterdevv1alpha1.Exporter{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-exporter", + Namespace: "default", + Generation: 1, + }, + Status: jumpstarterdevv1alpha1.ExporterStatus{ + ExporterStatusValue: jumpstarterdevv1alpha1.ExporterStatusAvailable, + }, + } + + applyUnregisterStatus(exporter, "shutting down for maintenance") + + if exporter.Status.StatusMessage != "shutting down for maintenance" { + t.Errorf("StatusMessage = %q, want %q", exporter.Status.StatusMessage, "shutting down for maintenance") + } + }) + + t.Run("sets Online condition to false", func(t *testing.T) { + exporter := &jumpstarterdevv1alpha1.Exporter{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-exporter", + Namespace: "default", + Generation: 1, + }, + Status: jumpstarterdevv1alpha1.ExporterStatus{ + ExporterStatusValue: jumpstarterdevv1alpha1.ExporterStatusAvailable, + }, + } + + applyUnregisterStatus(exporter, "") + + condition := meta.FindStatusCondition(exporter.Status.Conditions, + string(jumpstarterdevv1alpha1.ExporterConditionTypeOnline)) + if condition == nil { + t.Fatal("Online condition was not set") + } + if condition.Status != metav1.ConditionFalse { + t.Errorf("Online condition status = %v, want %v", condition.Status, metav1.ConditionFalse) + } + }) + + t.Run("sets Registered condition to false with message", func(t *testing.T) { + exporter := &jumpstarterdevv1alpha1.Exporter{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-exporter", + Namespace: "default", + Generation: 1, + }, + } + + applyUnregisterStatus(exporter, "") + + condition := meta.FindStatusCondition(exporter.Status.Conditions, + string(jumpstarterdevv1alpha1.ExporterConditionTypeRegistered)) + if condition == nil { + t.Fatal("Registered condition was not set") + } + if condition.Status != metav1.ConditionFalse { + t.Errorf("Registered condition status = %v, want %v", condition.Status, metav1.ConditionFalse) + } + if condition.Reason != "Unregister" { + t.Errorf("Registered condition reason = %q, want %q", condition.Reason, "Unregister") + } + if condition.Message != "Exporter unregistered from the controller and became offline." { + t.Errorf("Registered condition message = %q, want %q", condition.Message, "Exporter unregistered from the controller and became offline.") + } + }) +} + func TestSyncOnlineConditionWithStatus(t *testing.T) { tests := []struct { name string