Update mass deployment script #3838

maayarosama · 2025-01-28T09:33:04Z

Description

Rewrote handle method inside script file not to rollback and to output each batch nodes that successfully and failed to deploy on

Related Issues

🐞 [Bug]: Mass Dep script fail to create vms #3778

Tested Scenarios

Run the script

To consider

Preliminary Checks:

Preliminary Checks
- Does it completely address the issue linked?
- What about edge cases?
- Does it meet the specified acceptance criteria?
- Are there any unintended side effects?
- Does the PR adhere to the team's coding conventions, style guides, and best practices?
- Does it integrate well with existing features?
- Does it impact the overall performance of the application?
- Are there any bottlenecks or slowdowns?
- Has it been optimized for efficiency?
- Has it been adequately tested with unit, integration, and end-to-end tests?
- Are there any known defects or issues?
- Is the code well-documented?
- Are changes to documentation reflected in the code?

UI Checks:

UI Checks
- If a UI design is provided/ does it follow it?
- Does every button work?
- Is the data displayed logical? Is it what you expected?
- Does every validation work?
- Does this interface feel intuitive?
- What about slow network? Offline?
- What if the gridproxy/graphql/chain is failing?
- What would a first time user have a hard time navigating here?

Code Quality Checks:

Code Quality Checks
- Code formatted/linted? Are there unused imports? .. etc
- Is there redundant/repeated code?
- Are there conditionals that are always true or always false?
- Can we write this more concisely?
- Can we reuse this code? If yes, where?
- Will the changes be easy to maintain and update in the future?
- Can this code become too complex to understand for other devs?
- Can this code cause future integration problems?

Testing Checklist

Does it Meet the specified acceptance criteria.
Test if changes can affect any other functionality.
Tested with unit, integration, and end-to-end tests.
Tested the un-happy path and rollback scenarios.
Documentation updated to meet the latest changes.
Check automated tests working successfully with the changes.
Can be covered by automated tests.

General Checklist

…ach batch nodes that successfully and failed to deploy on

0oM4R · 2025-01-28T12:36:21Z

packages/grid_client/scripts/mass_deployments.ts

+  }
+
+  const res = await Promise.allSettled(deploymentPromises);
+  console.log("Deployment Results in handle:", res);


i think there is no need to log this

0oM4R

I tried to run the script with 1500 vm with batch size 100
it took more than 3 hours and the summary shows that we have only about 180 while I have 2100 contracts and 3 summaries of batches have successful deployment more than the final summary
script output
https://raw.githubusercontent.com/threefoldtech/tfgrid-sdk-ts/refs/heads/development_mass_deployement_test/packages/grid_client/output.txt

AhmedHanafy725 · 2025-01-29T09:22:58Z

packages/grid_client/scripts/mass_deployments.ts

+            await grid3.machines.twinDeploymentHandler.sendToNode(twinDeployment);
+            successfulNodesSet.push(twinDeployment.nodeId);


after sending the deployment to the node, we should wait and check the deployment result

AhmedHanafy725 · 2025-01-29T09:28:41Z

packages/grid_client/scripts/mass_deployments.ts

      } catch (error) {
        log(`Error creating deployment for batch ${batch + 1}: ${error}`);
-        return { twinDeployments: null, batchIndex: index };
+        return [];


The failed deployments here are not counted

@maayarosama please explain how this was resolved

failed deployments and successfull deployments are counted in the handle method after calling waitForDeployments

…romise to calling of handle method

amiraabouhadid

why is batch 1 summary empty?

also, I don't get the single apostrophe instead of a zero?

amiraabouhadid · 2025-02-11T08:37:50Z

packages/grid_client/scripts/mass_deployments.ts

+      log(`Batch ${batch + 1} Summary:`);
+      log(`- Successful Deployments on Nodes: ${Array.from(batchSuccessfulNodes).join(", ")}`);
+      log(`- Failed Deployments on Nodes: ${Array.from(batchFailedNodes).join(", ")}`);
+      log(`- Successful Deployments: ${batchSuccessfulNodes.size}`);
+      log(`- Failed Deployments: ${batchFailedNodes.size}`);
+      log("---------------------------------------------");


great, batch 1 summary is no longer empty. But can we please put a zero/ or a dash instead of empty size/nodes values?

Suggested change

log(`Batch ${batch + 1} Summary:`);

log(`- Successful Deployments on Nodes: ${Array.from(batchSuccessfulNodes).join(", ")}`);

log(`- Failed Deployments on Nodes: ${Array.from(batchFailedNodes).join(", ")}`);

log(`- Successful Deployments: ${batchSuccessfulNodes.size}`);

log(`- Failed Deployments: ${batchFailedNodes.size}`);

log("---------------------------------------------");

log(`Batch ${batch + 1} Summary:`);

log(

`- Successful Deployments on Nodes: ${

Array.from(batchSuccessfulNodes).length ? Array.from(batchSuccessfulNodes).join(", ") : "-"

}`,

);

log(

`- Failed Deployments on Nodes: ${

Array.from(batchFailedNodes).length ? Array.from(batchFailedNodes).join(", ") : "-"

}`,

);

log(`- Successful Deployments: ${batchSuccessfulNodes.size ?? 0}`);

log(`- Failed Deployments: ${batchFailedNodes.size ?? 0}`);

log("---------------------------------------------");

amiraabouhadid · 2025-02-11T08:38:09Z

packages/grid_client/scripts/mass_deployments.ts

+  log("Final Summary:");
+  log(`- Total Successful Deployments: ${allSuccessfulNodes.length}`);
+  log(`- Total Failed Deployments: ${totalVMs - allSuccessfulNodes.length}`);
+  log(`- Offline Nodes: ${offlineNodes.join(", ")}`);
+  log(`- All Successful Deployments on Nodes: ${Array.from(allSuccessfulNodes).join(", ")}`);
+  log(`- All Failed Deployments on Nodes: ${Array.from(allFailedNodes).join(", ")}`);


apply here too

0oM4R · 2025-02-11T14:59:28Z

packages/grid_client/scripts/mass_deployments.ts

+    await grid3.machines.twinDeploymentHandler.checkNodesCapacity(twinDeployments);
+    await grid3.machines.twinDeploymentHandler.checkFarmIps(twinDeployments);
+  } catch (error) {
+    events.emit("logs", "Error Validating workloads:" + error);


then what should it continue or mark the deployments as failed?

0oM4R · 2025-02-12T14:48:26Z

packages/grid_client/scripts/mass_deployments.ts

+  results.forEach((result, index) => {
+    const twinDeployment = twinDeployments[index];
+    if (result.status === "fulfilled") {
+      passedDeployments.push(twinDeployment);


in case if the operation is not update or deploy
the result will be { status: "fulfilled", value: null }
so we are pussing null to the passedDeployments

0oM4R · 2025-02-12T14:50:05Z

packages/grid_client/scripts/mass_deployments.ts

+  const results = await Promise.allSettled(
+    twinDeployments.map(t => {
+      if ([Operations.deploy, Operations.update].includes(t.operation)) {
+        events.emit("logs", `Waiting for deployment with contract_id: ${t.deployment.contract_id} to be ready`);
+        return grid3.machines.twinDeploymentHandler.waitForDeployment(t, timeout);
+      }
+
+      return null;
+    }),


we need to filter the array to remove null values or handle the null values later

0oM4R · 2025-02-12T14:58:34Z

packages/grid_client/scripts/mass_deployments.ts

 import {
+  Deployment,


I don't think it is used anywhere if so please remove it

0oM4R · 2025-02-12T15:08:43Z

packages/grid_client/scripts/mass_deployments.ts

+  const extrinsicResults: Contract[] = await grid3.machines.twinDeploymentHandler.tfclient.applyAllExtrinsics<Contract>(
+    [...nodeExtrinsics, ...nameExtrinsics],
+  );


i think this should be in try catch block

0oM4R · 2025-02-12T15:13:30Z

packages/grid_client/scripts/mass_deployments.ts

+      if (twinDeployment.operation === Operations.deploy) {
+        events.emit("logs", `Sending deployment to node_id: ${twinDeployment.nodeId}`);
+        for (const contract of extrinsicResults) {
+          if (twinDeployment.deployment.challenge_hash() === contract.contractType.nodeContract.deploymentHash) {
+            twinDeployment.deployment.contract_id = contract.contractId;
+            if (
+              twinDeployment.returnNetworkContracts ||
+              !(
+                twinDeployment.deployment.workloads.length === 1 &&
+                twinDeployment.deployment.workloads[0].type === WorkloadTypes.network
+              )
+            )
+              resultContracts.created.push(contract);
+            break;
+          }
+        }


can you explain what are we doing here, i can see you are updating the resultContracts but its not clear the meaning of inner conditionse ?

0oM4R · 2025-02-12T15:14:19Z

packages/grid_client/scripts/mass_deployments.ts

+        events.emit("logs", `Deployment has been updated with contract_id: ${twinDeployment.deployment.contract_id}`);
+      }
+    } catch (e) {
+      events.emit("logs", `Deployment failed on node_id: ${twinDeployment.nodeId} with error: ${e}`);


I think we are missing the add to the failed deployment step.

0oM4R · 2025-02-12T15:15:06Z

packages/grid_client/scripts/mass_deployments.ts

+    }
+  }
+
+  const deletedResult = await grid3.machines.twinDeploymentHandler.tfclient.applyAllExtrinsics<number>(


add it in a try block please to avoid panic

0oM4R · 2025-02-12T15:41:09Z

packages/grid_client/scripts/mass_deployments.ts

@@ -80,39 +240,29 @@ async function main() {
      availableFor: await grid3.twins.get_my_twin_id(),
      farmIds: farmIds,
      randomize: true,
+      features: [Features.yggdrasil],


i suggest to remove ygg and add myceluim to include zos3light machines

0oM4R · 2025-02-12T15:48:29Z

packages/grid_client/scripts/mass_deployments.ts

      } catch (error) {
        log(`Error creating deployment for batch ${batch + 1}: ${error}`);
-        return { twinDeployments: null, batchIndex: index };
+        return [];
      }
    });
    console.time("Preparing Batch " + (batch + 1));
    const deploymentResults = await Promise.allSettled(deploymentPromises).then(results =>
      results.flatMap(r => (r.status === "fulfilled" ? r.value : [])),


this means the rejected deployments will be ignored

0oM4R · 2025-02-12T15:49:43Z

packages/grid_client/scripts/mass_deployments.ts

+
+          log(`Successfully handled deployments for Batch ${batch + 1}`);
+        } else {
+          errors.push(`Error handling deployments for Batch ${batch + 1}: ${result.reason}`);


should we handle the rejected one to? i mean by pusshing it to the failed deployements

0oM4R · 2025-02-13T12:38:39Z

i think we still have issue with the summary numbers

but it appears once in the batch summary

created contarcts 1460

full script output:https://gist.github.com/0oM4R/461e3ecb266aa782da912c9822258842

0oM4R · 2025-02-13T12:39:26Z

sorry misclick

samaradel · 2025-02-16T09:42:13Z

I'm not sure if you should handle this or not, otherwise, it deployed 2 batches successfully

Reworte handle method inside script file not rollback and to output e…

adfb4d5

…ach batch nodes that successfully and failed to deploy on

maayarosama requested review from AhmedHanafy725, zaelgohary, MohamedElmdary, Mahmoud-Emad, amiraabouhadid, AlaaElattar, 0oM4R and samaradel as code owners January 28, 2025 09:33

0oM4R reviewed Jan 28, 2025

View reviewed changes

Removing unused console logs

6986f14

maayarosama requested a review from 0oM4R January 28, 2025 12:51

0oM4R reviewed Jan 29, 2025

View reviewed changes

AhmedHanafy725 reviewed Jan 29, 2025

View reviewed changes

maayarosama added 2 commits February 2, 2025 11:33

Merge branch 'development' into development_mass_deployement

3f8b8d0

Refactoring mass deployment script to count failed deployments

be75a2e

maayarosama marked this pull request as draft February 4, 2025 11:28

Setting addAccess to false and refactoring handle method and adding p…

438576d

…romise to calling of handle method

maayarosama marked this pull request as ready for review February 9, 2025 19:07

maayarosama requested review from AhmedHanafy725 and 0oM4R February 9, 2025 19:07

amiraabouhadid reviewed Feb 10, 2025

View reviewed changes

maayarosama marked this pull request as draft February 10, 2025 11:49

Fixing random empty batch data

e583036

maayarosama requested a review from amiraabouhadid February 10, 2025 14:00

maayarosama marked this pull request as ready for review February 10, 2025 14:00

Adding try and catch around validation function calls

0e0199b

amiraabouhadid reviewed Feb 11, 2025

View reviewed changes

Formating logs

e0b8a40

maayarosama requested a review from amiraabouhadid February 11, 2025 10:10

Adding features to filterNodes

52c96cd

0oM4R reviewed Feb 11, 2025

View reviewed changes

0oM4R reviewed Feb 12, 2025

View reviewed changes

0oM4R closed this Feb 13, 2025

0oM4R reopened this Feb 13, 2025

zaelgohary marked this pull request as draft April 2, 2025 09:50

		await grid3.machines.twinDeploymentHandler.sendToNode(twinDeployment);
		successfulNodesSet.push(twinDeployment.nodeId);

Update mass deployment script #3838

Are you sure you want to change the base?

Update mass deployment script #3838

Uh oh!

Conversation

maayarosama commented Jan 28, 2025

Description

Related Issues

Tested Scenarios

To consider

Preliminary Checks:

UI Checks:

Code Quality Checks:

Testing Checklist

General Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0oM4R left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amiraabouhadid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0oM4R commented Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0oM4R commented Feb 13, 2025

Uh oh!

samaradel commented Feb 16, 2025

Uh oh!

Uh oh!

0oM4R commented Feb 13, 2025 •

edited

Loading