Netgoal: Update netgoal generate flags and variables #4656

algobarb · 2022-10-17T16:06:40Z

Summary

update generate.go to mimic the same behavior for participating nodes and hosts for NPNs and NPN hosts.
Update generate_network.py with better variable names and the new npn variable. Used for mainnet-model and custom recipe.
Update generate_network.py to fix conditional for "ConsensusProtocol". Noticed it will error on mainnet-model otherwise.
Update existing Makefiles.
Added more documentation for netgoal/generate and recipe folder.
Renamed the flags and variables used by netgoal/generate.go
Changed references from npnHosts to npnNodes for wallet generation and stake distribution.

Important - Functional Changes

Netgoal generate short and long-form flags have changed!

--participation-host-machines (-N)
--participation-algod-nodes (-n)
--npn-host-machines (-X)
--npn-algod-nodes (-x)

Test Plan

Tested the change by creating a binary locally and making sure I can specify number of NPNs without specifying the number of NPN hosts properly.
Tested the recipes to make sure a newly generated net.json and genesis.json using the new netgoal binary will generate with the expected result.
Tested to make sure custom recipe works.

test/testdata/deployednettemplates/recipes/README.md

codecov · 2022-10-17T16:32:00Z

Codecov Report

Merging #4656 (af0c57a) into master (4961f54) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4656      +/-   ##
==========================================
- Coverage   54.35%   54.35%   -0.01%     
==========================================
  Files         402      402              
  Lines       51793    51793              
==========================================
- Hits        28154    28151       -3     
- Misses      21272    21274       +2     
- Partials     2367     2368       +1

Impacted Files	Coverage Δ
ledger/roundlru.go	`90.56% <0.00%> (-5.67%)`	⬇️
catchup/peerSelector.go	`98.95% <0.00%> (-1.05%)`	⬇️
ledger/tracker.go	`74.04% <0.00%> (-0.86%)`	⬇️
cmd/tealdbg/debugger.go	`72.69% <0.00%> (-0.81%)`	⬇️
catchup/service.go	`68.64% <0.00%> (+0.49%)`	⬆️
network/wsPeer.go	`68.18% <0.00%> (+0.53%)`	⬆️
data/transactions/verify/txn.go	`77.14% <0.00%> (+0.95%)`	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

excalq

I agree on using long-form params, especially in a README, but it's non-blocking, so I'll approve as well.

test/testdata/deployednettemplates/recipes/README.md

brianolson

LGTM

brianolson · 2022-10-24T14:01:15Z

cmd/netgoal/generate.go

-var nodeHostsToGenerate int
-var nonPartnodesToGenerate int
-var nonPartnodesHostsToGenerate int
+var participationAlgodNodes int


This variable renaming is great!

algolucky

I tested this with the loadgenerator recipe and it generates the same net.json (just change -H to -x or the long-form).

just some random markdown comments about using links.

test/testdata/deployednettemplates/recipes/README.md

algolucky · 2022-10-24T15:29:38Z

cmd/netgoal/generate.go

+	generateCmd.Flags().IntVarP(&npnAlgodNodes, "npn-algod-nodes", "x", 0, "Total non-participation algod nodes to generate")
+	generateCmd.Flags().IntVarP(&npnHostMachines, "npn-host-machines", "X", 0, "Host machines to generate for non-participation algod nodes, default=npn-algod-nodes")


might get some questions about these, since this does make it breaking. on the other hand, it does make it more consistent.

yeah I posted these changes in #performance and #devops. I think they're the only ones who really know about the new X flag anyways. Not sure what else I should do besides answer questions if it comes up.

I also updated all of the recipes' Makefiles in go-algorand, so hopefully that will help.

Co-authored-by: algolucky <[email protected]>

algobarb · 2022-10-24T15:56:43Z

@egieseke Can you review the new netgoal changes to make sure you're aligned with it?

algojack · 2022-10-24T20:06:50Z

cmd/netgoal/README.md

+### Short-Form Flags Example
+The following will result in the same outcome as the command above.
+```
+netgoal generate -t net -r /tmp/wat -o net.json -w 100 -R 8 -N 20 -n 100 -X 5 -x 10 --node-template node.json --relay-template relay.json --non-participating-node-template nonPartNode.json


hmm, it's still using shorthand flags here in readme. Should we set a better example by using long form flags?

The long form flags example is right above the shorthand flags in the readme.

Might be easier to read in this format: https://github.com/algorand/go-algorand/blob/af0c57a67fc3cb6a88990f69d56f4565d6f79725/cmd/netgoal/README.md

egieseke

Nice work. One other change that would be nice is to allow specifying the network name so that it is used for the hostname suffix in the net.json file. Currently, that has to be added manually.

algobarb · 2022-10-25T14:02:31Z

Nice work. One other change that would be nice is to allow specifying the network name so that it is used for the hostname suffix in the net.json file. Currently, that has to be added manually.

Thanks! I will ask to merge this in as is for now. Maybe we can consider adding the hostname suffix in a follow up iteration/ticket.

algobarb added 14 commits October 6, 2022 15:44

if npn hosts is not set, set it equal to number of NPNs

51c9404

Merge branch 'master' into algobarb/npnUpdate

4b75143

make custom recipe smaller to reduce cost on testing

cf872aa

update the json on custom recipe

acce502

add -X flag and use old params

d496746

test without -X flag

9120552

make custom scenario smaller

c484ad3

remove the -H flag in python for testing

df8e6ce

revert previous testing change

48783f6

fix netgoal

ff3f3c8

add npn variable in json and generate_network python script

d9d55e2

add npn key and fix conditional for consensus protocol

03859ee

add README with netgoal generate details

a275f3f

update Makefiles

a917262

algobarb added Enhancement Team Hyper Flow labels Oct 17, 2022

algobarb self-assigned this Oct 17, 2022

algobarb requested review from a team, excalq, egieseke, algolucky, algojack, cce and brianolson October 17, 2022 16:06

algojack reviewed Oct 17, 2022

View reviewed changes

test/testdata/deployednettemplates/recipes/README.md Outdated Show resolved Hide resolved

algolucky previously approved these changes Oct 18, 2022

View reviewed changes

excalq previously approved these changes Oct 18, 2022

View reviewed changes

brianolson reviewed Oct 19, 2022

View reviewed changes

test/testdata/deployednettemplates/recipes/README.md Outdated Show resolved Hide resolved

algobarb removed the Team Hyper Flow label Oct 19, 2022

algobarb added 3 commits October 23, 2022 16:18

update variable and flag names in netgoal generate

bc2fc9b

update flags used in generate_network.py

87a6796

update steps in custom recipe README

b39a770

algobarb dismissed stale reviews from excalq and algolucky via b39a770 October 23, 2022 20:21

algobarb closed this Oct 23, 2022

algobarb added 2 commits October 23, 2022 19:14

update scenario makefiles

96356e7

update documentation

43234dc

algobarb reopened this Oct 24, 2022

algobarb added 2 commits October 23, 2022 21:34

fix variable references for wallets and stake for NPNs

a484f16

update documentation

c7fcfc3

algobarb changed the title ~~TestData: Update NPN parameters in Recipes~~ Netgoal: Update netgoal generate flags and variables Oct 24, 2022

algobarb added 2 commits October 23, 2022 21:39

update documentation

b6b7e0d

grammar

58e3410

brianolson previously approved these changes Oct 24, 2022

View reviewed changes

algolucky previously approved these changes Oct 24, 2022

View reviewed changes

Update test/testdata/deployednettemplates/recipes/README.md

af0c57a

Co-authored-by: algolucky <[email protected]>

algobarb dismissed stale reviews from algolucky and brianolson via af0c57a October 24, 2022 15:50

algolucky approved these changes Oct 24, 2022

View reviewed changes

algojack reviewed Oct 24, 2022

View reviewed changes

algobarb requested a review from onetechnical October 25, 2022 13:34

egieseke approved these changes Oct 25, 2022

View reviewed changes

onetechnical merged commit fb332de into algorand:master Oct 25, 2022

algobarb deleted the algobarb/npnUpdate branch October 25, 2022 14:41

Algo-devops-service mentioned this pull request Nov 1, 2022

go-algorand 3.12.0-beta Release PR #4730

Merged

Algo-devops-service mentioned this pull request Nov 10, 2022

go-algorand 3.12.0-stable Release PR #4783

Closed

Algo-devops-service mentioned this pull request Nov 28, 2022

go-algorand 3.12.2-stable Release PR #4837

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Netgoal: Update netgoal generate flags and variables #4656

Netgoal: Update netgoal generate flags and variables #4656

algobarb commented Oct 17, 2022 •

edited

Loading

codecov bot commented Oct 17, 2022 •

edited

Loading

excalq left a comment

brianolson left a comment

brianolson Oct 24, 2022

algolucky left a comment

algolucky Oct 24, 2022

algobarb Oct 24, 2022

algobarb Oct 24, 2022 •

edited

Loading

algobarb commented Oct 24, 2022

algojack Oct 24, 2022

algobarb Oct 24, 2022

algobarb Oct 24, 2022

egieseke left a comment

algobarb commented Oct 25, 2022 •

edited

Loading

		generateCmd.Flags().IntVarP(&npnAlgodNodes, "npn-algod-nodes", "x", 0, "Total non-participation algod nodes to generate")
		generateCmd.Flags().IntVarP(&npnHostMachines, "npn-host-machines", "X", 0, "Host machines to generate for non-participation algod nodes, default=npn-algod-nodes")

Netgoal: Update netgoal generate flags and variables #4656

Netgoal: Update netgoal generate flags and variables #4656

Conversation

algobarb commented Oct 17, 2022 • edited Loading

Summary

Important - Functional Changes

Test Plan

codecov bot commented Oct 17, 2022 • edited Loading

Codecov Report

excalq left a comment

Choose a reason for hiding this comment

brianolson left a comment

Choose a reason for hiding this comment

brianolson Oct 24, 2022

Choose a reason for hiding this comment

algolucky left a comment

Choose a reason for hiding this comment

algolucky Oct 24, 2022

Choose a reason for hiding this comment

algobarb Oct 24, 2022

Choose a reason for hiding this comment

algobarb Oct 24, 2022 • edited Loading

Choose a reason for hiding this comment

algobarb commented Oct 24, 2022

algojack Oct 24, 2022

Choose a reason for hiding this comment

algobarb Oct 24, 2022

Choose a reason for hiding this comment

algobarb Oct 24, 2022

Choose a reason for hiding this comment

egieseke left a comment

Choose a reason for hiding this comment

algobarb commented Oct 25, 2022 • edited Loading

algobarb commented Oct 17, 2022 •

edited

Loading

codecov bot commented Oct 17, 2022 •

edited

Loading

algobarb Oct 24, 2022 •

edited

Loading

algobarb commented Oct 25, 2022 •

edited

Loading