Merge pull request #198 from prona-p4-learning-platform/fix-dockerpro…

…vider-delete-leftover-containers Fix DockerProvider, FirecrackerProvider and OpenStackProvider delete leftover instances during pruning, if environment was already deleted in persister
prona-p4-learning-platform · Jul 7, 2024 · 9c19518 · 9c19518
2 parents 501afbc + ae7739b
commit 9c19518
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 28 deletions.
diff --git a/backend/src/providers/DockerProvider.ts b/backend/src/providers/DockerProvider.ts
@@ -347,15 +347,7 @@ export default class DockerProvider implements InstanceProvider {
             `${container.Names[0]} was created at ${createdAt.toISOString()} and should be deleted`,
           );
 
-          //await this.deleteServer(container.Id).catch((reason) => {
-          //  const originalMessage =
-          //     reason instanceof Error ? reason.message : "Unknown error";
-          //   throw new Error(
-          //     `DockerProvider: Failed to delete container (${container.Names[0]}) to be pruned.\n` +
-          //       originalMessage,
-          //   );
-          // });
-          await Environment.deleteInstanceEnvironments(container.Id).catch(
+          const deleted = await Environment.deleteInstanceEnvironments(container.Id).catch(
             (reason) => {
               const originalMessage =
                 reason instanceof Error ? reason.message : "Unknown error";
@@ -365,6 +357,19 @@ export default class DockerProvider implements InstanceProvider {
               );
             },
           );
+          if (!deleted) {
+            console.log(
+              `DockerProvider: Could not delete environment during pruning, environment seams to be gone already, deleting leftover container: (${container.Names[0]}).`,
+            );
+            await this.deleteServer(container.Id).catch((reason) => {
+             const originalMessage =
+                reason instanceof Error ? reason.message : "Unknown error";
+              throw new Error(
+                `DockerProvider: Failed to delete container (${container.Names[0]}) to be pruned.\n` +
+                  originalMessage,
+              );
+            });
+          }
         }
       }
     }

diff --git a/backend/src/providers/FirecrackerProvider.ts b/backend/src/providers/FirecrackerProvider.ts
@@ -740,15 +740,7 @@ export default class FirecrackerProvider implements InstanceProvider {
 
     for (const [microVMId, microVM] of this.firecrackers.entries()) {
       if (microVM.expirationDate < deadline) {
-        //await this.deleteServer(microVMId).catch((reason) => {
-        //  const originalMessage =
-        //  reason instanceof Error ? reason.message : "Unknown error";
-        //  throw(new Error(
-        //      `FirecrackerProvider: Could not delete expired microVM (${microVMId}) during pruning.\n` +
-        //        originalMessage,
-        //  ));
-        //})
-        await Environment.deleteInstanceEnvironments(microVMId).catch(
+        const deleted = await Environment.deleteInstanceEnvironments(microVMId).catch(
           (reason) => {
             const originalMessage =
               reason instanceof Error ? reason.message : "Unknown error";
@@ -758,12 +750,26 @@ export default class FirecrackerProvider implements InstanceProvider {
             );
           },
         );
-        console.log(
-          "FirecrackerProvider: deleted expired microVM: " +
-            microVMId +
-            " expiration date: " +
-            microVM.expirationDate.toISOString(),
-        );
+        if (!deleted) {
+          console.log(
+            `FirecrackerProvider: Could not delete environment during pruning, environment seams to be gone already, deleting leftover microVM: (${microVMId}).`,
+          );
+          await this.deleteServer(microVMId).catch((reason) => {
+            const originalMessage =
+            reason instanceof Error ? reason.message : "Unknown error";
+            throw(new Error(
+              `FirecrackerProvider: Could not delete expired microVM (${microVMId}) during pruning.\n` +
+                originalMessage,
+            ));
+          })
+        } else {
+          console.log(
+            "FirecrackerProvider: deleted expired microVM: " +
+              microVMId +
+              " expiration date: " +
+              microVM.expirationDate.toISOString(),
+          ); 
+        }
         return;
       }
     }
@@ -816,7 +822,7 @@ export default class FirecrackerProvider implements InstanceProvider {
           reason instanceof Error ? reason.message : "Unknown error";
 
         console.log(
-          "DockerProvider: SSH connection failed - retrying...\n" +
+          "FirecrackerProvider: SSH connection failed - retrying...\n" +
             originalMessage,
         );
 
@@ -829,7 +835,7 @@ export default class FirecrackerProvider implements InstanceProvider {
       await this.sleep(1000);
     }
 
-    throw new Error("DockerProvider: Timed out waiting for SSH connection.");
+    throw new Error("FirecrackerProvider: Timed out waiting for SSH connection.");
   }
 
   sleep(ms: number): Promise<void> {

diff --git a/backend/src/providers/OpenStackProvider.ts b/backend/src/providers/OpenStackProvider.ts
@@ -777,7 +777,7 @@ export default class OpenStackProvider implements InstanceProvider {
           );
           providerInstance.axiosInstance
             .get(providerInstance.endpointPublicComputeURL + "/servers/detail")
-            .then(function (response) {
+            .then(async function (response) {
               const servers = response.data.servers as Array<Server>;
               for (const server of servers) {
                 if (server.metadata.learn_sdn_hub_user !== undefined) {
@@ -790,7 +790,20 @@ export default class OpenStackProvider implements InstanceProvider {
                         timestampCreated.toISOString() +
                         "and should be deleted",
                     );
-                    Environment.deleteInstanceEnvironments(server.id);
+                    const deleted = await Environment.deleteInstanceEnvironments(server.id);
+                    if (!deleted) {
+                      console.log(
+                        `OpenStackProvider: Could not delete environment during pruning, environment seams to be gone already, deleting leftover instance: (${server.id}).`,
+                      );
+                      await providerInstance.deleteServer(server.id).catch((reason) => {
+                       const originalMessage =
+                          reason instanceof Error ? reason.message : "Unknown error";
+                        throw new Error(
+                          `OpenStackProvider: Failed to delete instance (${server.id}) to be pruned.\n` +
+                            originalMessage,
+                        );
+                      });
+                    }
                   }
                 }
               }