Using generateString in all scripts that deploy a machine #3822

maayarosama · 2025-01-16T15:15:54Z

Description

Using generateString in all scripts that deploy a machine

Related Issues

random name generation in scripts #3804

Tested Scenarios

run scripts

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

amiraabouhadid · 2025-01-19T08:36:40Z

issue isn't just for scripts that deploy a VM, please update PR title and scripts accordingly . I'd do a global search for 'name' in scripts and add generateString() for matches

maayarosama · 2025-01-19T09:03:34Z

issue isn't just for scripts that deploy a VM, please update PR title and scripts accordingly . I'd do a global search for 'name' in scripts and add generateString() for matches

I'm not sure what scripts exactly you want me to change, so can you please provide me with the scripts?

amiraabouhadid · 2025-01-20T12:06:32Z

for example:

tfgrid-sdk-ts/packages/grid_client/scripts/algorand.ts

Line 26 in 88715bd

const name = "testAcc";

tfgrid-sdk-ts/packages/grid_client/scripts/algorand.ts

Line 30 in 88715bd

tfgrid-sdk-ts/packages/grid_client/scripts/algorand.ts

Line 46 in 88715bd

description: "test",

tfgrid-sdk-ts/packages/grid_client/scripts/algorand.ts

Line 68 in 88715bd

description: "test",

tfgrid-sdk-ts/packages/grid_client/scripts/blockchain.ts

Line 23 in 88715bd

tfgrid-sdk-ts/packages/grid_client/scripts/blockchain.ts

Line 27 in 88715bd

tfgrid-sdk-ts/packages/grid_client/scripts/blockchain.ts

Line 32 in 88715bd

tfgrid-sdk-ts/packages/grid_client/scripts/blockchain.ts

Line 42 in 88715bd

0oM4R

I could run the script twice, i have 2 deployments

but in the deployment list we have to deployements with same name

i suggest to add the random string as postfix to the machine.name

0oM4R · 2025-01-30T09:38:35Z

deployed two VMS and got only one network wedtest I'm not sure if we have to use random string in the network name as well

0oM4R · 2025-01-30T09:39:27Z

also what about storing the random string and reuse it in the file, instead of generating 2 or 3 random strings

maayarosama · 2025-02-07T20:10:10Z

deployed two VMS and got only one network wedtest I'm not sure if we have to use random string in the network name as well

This's not an issue as there's already a network created on node 128, so the network just needs to update the newly reserved ports

AhmedHanafy725 · 2025-02-09T11:17:57Z

deployed two VMS and got only one network wedtest I'm not sure if we have to use random string in the network name as well

the network name should be random as well. If we use the same name between 2 scripts that require 2 different specs, the deployment will fail (e.g.: require mycelium in the second script while the first one doesn't have it)

ramezsaeed · 2025-02-09T17:40:23Z

deployed two VMS and got only one network wedtest I'm not sure if we have to use random string in the network name as well

the network name should be random as well. If we use the same name between 2 scripts that require 2 different specs, the deployment will fail (e.g.: require mycelium in the second script while the first one doesn't have it)

+1 for Hanafy suggestion and lets add this in test scenarios.

amiraabouhadid

please also add it in multiple_vms script

0oM4R

network name is randomized

the name we want to be randomized is the network.name

for example

tfgrid-sdk-ts/packages/grid_client/scripts/applications/nextcloud.ts

Lines 70 to 76 in d36709c

    
           const vms: MachinesModel = { 
        
             name, 
        
             network: { 
        
               name: "wedtest", 
        
               ip_range: "10.249.0.0/16", 
        
             }, 
        
             machines: [

we suggest to make it random; and keep it with the generatedstring before
name = 'net${name}

0oM4R

Thank you Mayar!

AhmedHanafy725 · 2025-02-12T09:17:42Z

packages/grid_client/scripts/applications/gitea.ts

@@ -44,7 +44,7 @@ async function cancel(client, vms, gw) {
 }

 async function main() {
-  const name = "giteainstance";
+  const name = "gt" + generateString(8);
  const networkName = "giteanetwork";


For the sake of consistency, use a random network name here too

AhmedHanafy725 · 2025-02-12T09:21:27Z

packages/grid_client/scripts/algorand.ts

@@ -23,7 +24,7 @@ const hexMessage = message
  .split("")
  .map((letter, i) => message.charCodeAt(i).toString(16))
  .join("");
-const name = "testAcc";
+const name = generateString(10);

 async function main() {
  const account: AlgorandAccountCreateModel = {


please use random name for account creation and deletion

AhmedHanafy725 · 2025-02-12T09:31:49Z

packages/grid_client/scripts/orchestrators/kubernetes_worker.ts

@@ -26,7 +26,8 @@ async function deleteWorker(client, worker) {
 }

 async function main() {
-  const name = "testk8s";
+  const name = "k8s" + generateString(8);


please add a comment for the user to change the name to an existing cluster name so he can add the worker to it

AhmedHanafy725 · 2025-02-12T09:36:58Z

packages/grid_client/scripts/zdb.ts

@@ -24,7 +24,7 @@ async function cancel(client, zdb) {
 }

 async function main() {
-  const name = "tttzdbs";
+  const name = "zdb" + generateString(8);


please use this name as the zdb instance name to have a unique contract hash

amiraabouhadid · 2025-02-17T14:55:03Z

packages/grid_client/scripts/vm_with_qsfs.ts

-  const name = "wed2710t1";
+  const name = "vm" + generateString(8);
+  const networName = `net${name}`;
+
  const grid3 = await getClient(`vm/${name}`);

  const qsfs_name = "wed2710q1";


why don't we use generate string for qsfs_name?

amiraabouhadid · 2025-02-17T14:56:48Z

packages/grid_client/scripts/wireguard_vm.ts

can we use generateString for name in line 7?

amiraabouhadid · 2025-02-17T15:00:44Z

packages/grid_client/scripts/orchestrators/kubernetes_with_qsfs.ts

can we use generateString for qsfs_name?

Using generateString in all scripts that deploy a vm

88715bd

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

maayarosama changed the title ~~Using generateString in all scripts that deploy a vm~~ Using generateString in all scripts that deploy a machine Jan 19, 2025

0oM4R reviewed Jan 20, 2025

View reviewed changes

Add generateString in machine.name

1dbc649

maayarosama requested a review from 0oM4R January 23, 2025 09:42

0oM4R mentioned this pull request Feb 2, 2025

GridClient:Dynamic_SingleVM: enhance the script #3846

Merged

15 tasks

AhmedHanafy725 requested a review from ramezsaeed February 2, 2025 12:31

0oM4R mentioned this pull request Feb 2, 2025

Grid Client Nightly failed during schedule on qa #3834

Open

maayarosama marked this pull request as draft February 4, 2025 10:53

maayarosama added 2 commits February 7, 2025 22:44

Using name variable istead of generating multiple strings

61b8aa5

Merge branch 'development' into development_scripts

a700aeb

maayarosama marked this pull request as ready for review February 7, 2025 20:47

ramezsaeed mentioned this pull request Feb 9, 2025

QA progress threefoldtech/home#1575

Open

amiraabouhadid requested changes Feb 10, 2025

View reviewed changes

Applying random name generation on multiple_vms script

d36709c

maayarosama requested a review from amiraabouhadid February 10, 2025 09:18

0oM4R requested changes Feb 10, 2025

View reviewed changes

Randomizing networks name

b454c8b

maayarosama requested a review from 0oM4R February 10, 2025 14:50

ramezsaeed approved these changes Feb 10, 2025

View reviewed changes

Randomizing network name

2cb4b22

0oM4R approved these changes Feb 11, 2025

View reviewed changes

AhmedHanafy725 requested changes Feb 12, 2025

View reviewed changes

maayarosama marked this pull request as draft February 16, 2025 09:21

maayarosama added 2 commits February 16, 2025 11:22

Merge branch 'development' into development_scripts

59b5490

Adding generateString to all scriptsincluding tfchain scripts

1f00f15

maayarosama marked this pull request as ready for review February 16, 2025 11:34

maayarosama requested review from AhmedHanafy725 and 0oM4R February 16, 2025 11:34

amiraabouhadid reviewed Feb 17, 2025

View reviewed changes

packages/grid_client/scripts/wireguard_vm.ts

Copy link

Contributor

amiraabouhadid Feb 17, 2025 •

edited

Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use generateString for name in line 7?

amiraabouhadid reviewed Feb 17, 2025

View reviewed changes

packages/grid_client/scripts/orchestrators/kubernetes_with_qsfs.ts

Copy link

Contributor

amiraabouhadid Feb 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use generateString for qsfs_name?

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Using generateString in all scripts that deploy a machine #3822

Using generateString in all scripts that deploy a machine #3822

maayarosama commented Jan 16, 2025 •

edited

Loading

amiraabouhadid commented Jan 19, 2025 •

edited

Loading

maayarosama commented Jan 19, 2025

amiraabouhadid commented Jan 20, 2025

0oM4R left a comment

0oM4R commented Jan 30, 2025

0oM4R commented Jan 30, 2025

maayarosama commented Feb 7, 2025

AhmedHanafy725 commented Feb 9, 2025

ramezsaeed commented Feb 9, 2025

amiraabouhadid left a comment

0oM4R left a comment

0oM4R left a comment

AhmedHanafy725 Feb 12, 2025

AhmedHanafy725 Feb 12, 2025

AhmedHanafy725 Feb 12, 2025

AhmedHanafy725 Feb 12, 2025

amiraabouhadid Feb 17, 2025

amiraabouhadid Feb 17, 2025 •

edited

Loading

amiraabouhadid Feb 17, 2025

	const vms: MachinesModel = {
	name,
	network: {
	name: "wedtest",
	ip_range: "10.249.0.0/16",
	},
	machines: [

Using generateString in all scripts that deploy a machine #3822

Are you sure you want to change the base?

Using generateString in all scripts that deploy a machine #3822

Conversation

maayarosama commented Jan 16, 2025 • edited Loading

Description

Related Issues

Tested Scenarios

To consider

Preliminary Checks:

UI Checks:

Code Quality Checks:

Testing Checklist

General Checklist

amiraabouhadid commented Jan 19, 2025 • edited Loading

maayarosama commented Jan 19, 2025

amiraabouhadid commented Jan 20, 2025

0oM4R left a comment

Choose a reason for hiding this comment

0oM4R commented Jan 30, 2025

0oM4R commented Jan 30, 2025

maayarosama commented Feb 7, 2025

AhmedHanafy725 commented Feb 9, 2025

ramezsaeed commented Feb 9, 2025

amiraabouhadid left a comment

Choose a reason for hiding this comment

0oM4R left a comment

Choose a reason for hiding this comment

0oM4R left a comment

Choose a reason for hiding this comment

AhmedHanafy725 Feb 12, 2025

Choose a reason for hiding this comment

AhmedHanafy725 Feb 12, 2025

Choose a reason for hiding this comment

AhmedHanafy725 Feb 12, 2025

Choose a reason for hiding this comment

AhmedHanafy725 Feb 12, 2025

Choose a reason for hiding this comment

amiraabouhadid Feb 17, 2025

Choose a reason for hiding this comment

amiraabouhadid Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

amiraabouhadid Feb 17, 2025

Choose a reason for hiding this comment

maayarosama commented Jan 16, 2025 •

edited

Loading

amiraabouhadid commented Jan 19, 2025 •

edited

Loading

amiraabouhadid Feb 17, 2025 •

edited

Loading