Add more context to the deletion of nodes (microsoft#3102)

* Add more context to the deletion of nodes * Update src/ApiService/ApiService/onefuzzlib/ScalesetOperations.cs Co-authored-by: Teo Voinea <58236992+tevoinea@users.noreply.github.com> * adding NODE_DELETED error code --------- Co-authored-by: Teo Voinea <58236992+tevoinea@users.noreply.github.com>
AdamL-Microsoft · May 12, 2023 · 447b4c6 · 447b4c6
1 parent cee37b5
commit 447b4c6
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 13 deletions.
diff --git a/src/ApiService/ApiService/Functions/AgentRegistration.cs b/src/ApiService/ApiService/Functions/AgentRegistration.cs
@@ -131,7 +131,7 @@ private async Async.Task<HttpResponseData> Post(HttpRequestData req) {
 
         var existingNode = await _context.NodeOperations.GetByMachineId(machineId);
         if (existingNode is not null) {
-            await _context.NodeOperations.Delete(existingNode);
+            await _context.NodeOperations.Delete(existingNode, "Node is re registering");
         }
 
         if (os != null && pool.Os != os) {

diff --git a/src/ApiService/ApiService/OneFuzzTypes/Enums.cs b/src/ApiService/ApiService/OneFuzzTypes/Enums.cs
@@ -38,6 +38,7 @@ public enum ErrorCode {
     GITHUB_VALIDATION_INVALID_REPOSITORY = 481,
     UNEXPECTED_DATA_SHAPE = 482,
     UNABLE_TO_SEND = 483,
+    NODE_DELETED = 484,
 }
 
 public enum VmState {

diff --git a/src/ApiService/ApiService/TestHooks/NodeOperationsTestHooks.cs b/src/ApiService/ApiService/TestHooks/NodeOperationsTestHooks.cs
@@ -185,7 +185,7 @@ public async Task<HttpResponseData> DeleteNode([HttpTrigger(AuthorizationLevel.A
             var s = await req.ReadAsStringAsync();
             var node = JsonSerializer.Deserialize<Node>(s!, EntityConverter.GetJsonSerializerOptions());
 
-            await _nodeOps.Delete(node!);
+            await _nodeOps.Delete(node!, "testing delete node");
             var resp = req.CreateResponse(HttpStatusCode.OK);
             return resp;
         }

diff --git a/src/ApiService/ApiService/onefuzzlib/NodeOperations.cs b/src/ApiService/ApiService/onefuzzlib/NodeOperations.cs
@@ -33,7 +33,7 @@ IAsyncEnumerable<Node> SearchStates(Guid? poolId = default,
         bool excludeUpdateScheduled = false,
         int? numResults = default);
 
-    new Async.Task Delete(Node node);
+    Async.Task Delete(Node node, string reason);
 
     Async.Task ReimageLongLivedNodes(Guid scaleSetId);
 
@@ -609,8 +609,10 @@ public async Async.Task MarkTasksStoppedEarly(Node node, Error? error) {
         }
     }
 
-    public new async Async.Task Delete(Node node) {
-        await MarkTasksStoppedEarly(node, Error.Create(ErrorCode.INVALID_NODE, "node is being deleted"));
+    public async Async.Task Delete(Node node, string reason) {
+        var error = Error.Create(ErrorCode.NODE_DELETED, reason, $"Node {node.MachineId} is being deleted");
+
+        await MarkTasksStoppedEarly(node, error);
         await _context.NodeTasksOperations.ClearByMachineId(node.MachineId);
         await _context.NodeMessageOperations.ClearMessages(node.MachineId);
         var r = await base.Delete(node);

diff --git a/src/ApiService/ApiService/onefuzzlib/ScalesetOperations.cs b/src/ApiService/ApiService/onefuzzlib/ScalesetOperations.cs
@@ -472,7 +472,7 @@ public async Async.Task<Scaleset> Halt(Scaleset scaleset) {
 
         await foreach (var node in _context.NodeOperations.SearchStates(scalesetId: scaleset.ScalesetId)) {
             _log.Info($"deleting node {scaleset.ScalesetId:Tag:ScalesetId} - {node.MachineId:Tag:MachineId}");
-            await _context.NodeOperations.Delete(node);
+            await _context.NodeOperations.Delete(node, "scaleset is being shutdown");
         }
         _log.Info($"scaleset delete starting - {scaleset.ScalesetId:Tag:ScalesetId}");
 
@@ -528,7 +528,7 @@ public async Async.Task<Scaleset> Halt(Scaleset scaleset) {
         await foreach (var node in nodes) {
             if (!azureNodes.ContainsKey(node.MachineId)) {
                 _log.Info($"{node.MachineId:Tag:MachineId} no longer in scaleset {scaleSet.ScalesetId:Tag:ScalesetId}");
-                await _context.NodeOperations.Delete(node);
+                await _context.NodeOperations.Delete(node, "node is being cleaned up because it is no longer in the scaleset");
             }
         }
 
@@ -618,7 +618,7 @@ where x.State.ReadyForReset()
             _log.Warning(reimageNodes.ErrorV);
             return (false, scaleSet);
         }
-        var deleteNodes = await DeleteNodes(scaleSet, toDelete.Values);
+        var deleteNodes = await DeleteNodes(scaleSet, toDelete.Values, "Node was ReadyForReset");
         if (!deleteNodes.IsOk) {
             _log.Warning(deleteNodes.ErrorV);
             return (toReimage.Count > 0, scaleSet);
@@ -636,7 +636,7 @@ public async Async.Task<OneFuzzResultVoid> ReimageNodes(Scaleset scaleset, IEnum
 
         if (scaleset.State == ScalesetState.Shutdown) {
             _log.Info($"scaleset shutting down, deleting rather than reimaging nodes {scaleset.ScalesetId:Tag:ScalesetId}");
-            return await DeleteNodes(scaleset, nodes);
+            return await DeleteNodes(scaleset, nodes, "scaleset is shutting down");
         }
 
         if (scaleset.State == ScalesetState.Halt) {
@@ -671,7 +671,7 @@ public async Async.Task<OneFuzzResultVoid> ReimageNodes(Scaleset scaleset, IEnum
                 }
                 await Async.Task.WhenAll(nodesToReimage
                     .Select(async node => {
-                        await _context.NodeOperations.Delete(node);
+                        await _context.NodeOperations.Delete(node, "Node decommissioned");
                     }));
                 return OneFuzzResultVoid.Ok;
 
@@ -685,7 +685,7 @@ await Async.Task.WhenAll(nodesToReimage
                     .Select(async node => {
                         var r = await _context.NodeOperations.ReleaseScaleInProtection(node);
                         if (r.IsOk) {
-                            await _context.NodeOperations.Delete(node);
+                            await _context.NodeOperations.Delete(node, "scaleset is scaling in");
                         }
                     }));
                 return OneFuzzResultVoid.Ok;
@@ -695,7 +695,7 @@ await Async.Task.WhenAll(nodesToReimage
     }
 
 
-    public async Async.Task<OneFuzzResultVoid> DeleteNodes(Scaleset scaleset, IEnumerable<Node> nodes) {
+    public async Async.Task<OneFuzzResultVoid> DeleteNodes(Scaleset scaleset, IEnumerable<Node> nodes, string reason) {
         if (nodes is null || !nodes.Any()) {
             _log.Info($"no nodes to delete: scaleset_id: {scaleset.ScalesetId:Tag:ScalesetId}");
             return OneFuzzResultVoid.Ok;
@@ -725,7 +725,7 @@ public async Async.Task<OneFuzzResultVoid> DeleteNodes(Scaleset scaleset, IEnume
         }
         await Async.Task.WhenAll(nodesToDelete
             .Select(async node => {
-                await _context.NodeOperations.Delete(node);
+                await _context.NodeOperations.Delete(node, reason);
             }));
         return OneFuzzResultVoid.Ok;
     }