cardano-testnet | configurable SPO and relays count, enable parallel execution of the test suite #6007

carbolymer · 2024-10-10T13:25:09Z

Description

This PR changes default testnet configuration to start with 1 SPO and 2 Relay nodes. This PR also makes the testnet nodes count and role (SPO/relay) configurable.

It also reenables previously disabled tests, and runs them in parallel, reducing test suite execution time.

Things still not working yet:

tests requiring more than SPO

[BUG] - cardano-testnet sometimes hangs indefinitely #5762

Checklist

Commit sequence broadly makes sense and commits have useful messages
New tests are added if needed and existing tests are updated. These may include:
- golden tests
- property tests
- roundtrip tests
- integration tests
  See Runnings tests for more details
Any changes are noted in the CHANGELOG.md for affected package
The version bounds in .cabal files are updated
CI passes. See note on CI. The following CI checks are required:
- Code is linted with hlint. See .github/workflows/check-hlint.yml to get the hlint version
- Code is formatted with stylish-haskell. See .github/workflows/stylish-haskell.yml to get the stylish-haskell version
- Code builds on Linux, MacOS and Windows for ghc-8.10.7 and ghc-9.2.7
Self-reviewed the diff

Note on CI

If your PR is from a fork, the necessary CI jobs won't trigger automatically for security reasons.
You will need to get someone with write privileges. Please contact IOG node developers to do this
for you.

cardano-testnet/src/Testnet/Components/Configuration.hs

Jimbo4350

A few comments. Please break this PR up into smaller commits with informative commit messages.

Jimbo4350 · 2024-10-10T14:23:03Z

cardano-testnet/src/Parsers/Cardano.hs

+    <>  OA.showDefault
+    <>  OA.value 1)
+  where
+    defaultSpoOptions = TestnetNodeOptions TestnetNodeRoleSpo Nothing []

 _pSpo :: Parser TestnetNodeOptions


We can delete this in a follow up PR.

FYI the removal is done in #6008

cardano-testnet/src/Testnet/Defaults.hs

cardano-testnet/src/Testnet/Start/Cardano.hs

Jimbo4350 · 2024-10-10T14:32:27Z

cardano-testnet/src/Testnet/Start/Cardano.hs

+    . encode $ Defaults.defaultByronProtocolParamsJsonValue
+
+  -- Because in Conway the overlay schedule and decentralization parameter
+  -- are deprecated, we must use the "create-staked" cli command to create


We can remove this comment in a follow up PR.

removed in: #6008

Jimbo4350 · 2024-10-10T14:45:58Z

cardano-testnet/src/Testnet/Start/Cardano.hs

+    let paymentAddrFile = tmpAbsPath </> "utxo-keys" </> "utxo" <> show idx </> "utxo.addr"
+
+    execCli_
+      [ "address", "build"


This will cause failures in the 9.3 release branch if merged to master. address was removed from the top level however we are going to reintroduce it as it is era agnostic 🫠 .

Thanks. I'll keep an eye on it.

cardano-testnet/src/Testnet/Start/Types.hs

smelc · 2024-10-11T11:53:08Z

FYI I'll merge #6008 after this one. The two are highly conflictual.

Jimbo4350

LGTM!

Jimbo4350 · 2024-10-11T14:57:22Z

cardano-testnet/src/Testnet/Start/Cardano.hs

 -- > ├── delegate-keys
 -- > │   ├── delegate{1,2,3}
 -- > │   │   ├── kes.{skey,vkey}
 -- > │   │   ├── key.{skey,vkey}
 -- > │   │   ├── opcert.{cert,counter}
-- > │   │   └── vrf.{skey,vkey}
+-- > │   │   ├── vrf.{skey,vkey}


Jimbo4350 · 2024-10-11T14:57:55Z

cardano-testnet/src/Testnet/Start/Cardano.hs

+-- > │   │   ├── staking.{skey,vkey}
+-- > ├── utxo-keys
+-- > │   ├── utxo{1,2,3}
+-- > │   │   ├── utxo.{addr,skey,vkey}


Did you manually update these changes?

Yes I did.

@smelc any tips how to make this less error-prone?

@carbolymer> I ran tree on the content of a sandbox created by running the tests.

I did the same, but the de-duplication part is what's annoying and error-prone.

Jimbo4350 · 2024-10-11T14:59:20Z

cardano-testnet/src/Testnet/Start/Cardano.hs

+  _ <- createSPOGenesisAndFiles nPools nDReps maxSupply asbe shelleyGenesis alonzoGenesis conwayGenesis (TmpAbsolutePath tmpAbsPath)
+
+  -- TODO: This should come from the configuration!
+  let makePathsAbsolute :: (Element a ~ FilePath, MonoFunctor a) => a -> a


Nice. How did you come across MonoFunctor?

I don't remember now. I think I saw a presentation mentioning it some time ago.

Jimbo4350 · 2024-10-11T15:02:46Z

cardano-testnet/src/Testnet/Types.hs

-  , poolKeys :: PoolNodeKeys
+data TestnetNode = TestnetNode
+  { testnetNodeRuntime :: !NodeRuntime
+  , poolKeys :: Maybe SpoNodeKeys -- ^ Keys are only present for SPO nodes


We should move poolKeys into NodeRuntime if possible.

I wasn't sure about that. NodeRuntime is more about the system process itself like sockets, system handles etc. TestnetNode is rather for describing logical layer, like keys here.

With the amount of information we have now it can be one datatype I guess.

smelc · 2024-10-14T08:19:36Z

cardano-testnet/src/Testnet/Start/Cardano.hs

-- > │       └── drep.{skey,vkey}
-- > ├── genesis.{alonzo,conway}.spec.json
+-- > │   ├── drep{1,2,3}
+-- > │   │   └── drep.{skey,drep.vkey}


Suggested change

-- > │ │ └── drep.{skey,drep.vkey}

-- > │ │ └── drep.{skey,vkey}

The drep.{skey,vkey} notation actually means drep.skey, drep.vkey; when you unfold the brackets (as would bash do).

Oh right, sorry. A typo got in where I was doing those changes without thinking much.

removed in: #6008

smelc · 2024-10-14T08:21:59Z

cardano-testnet/src/Testnet/Start/Types.hs

+  , isSpoNodeOptions
+  , isRelayNodeOptions


Personally I am not a big fan of exporting small helper functions like this. Users could just pattern match on the constructors, instead of using those functions.

Optional.

Those functions are more readable than pattern matching on the constructor. This is more readable to me

when (isSpoNodeOptions nodeOptions) $ do ...

rather than more noisy case:

case nodeOptions of RelayNodeOptions{} -> pure () SpoNodeOptions{} -> do ...

Additionally, if you have a dedicated function, you can compose it.

smelc · 2024-10-14T08:31:15Z

cardano-testnet/test/cardano-testnet-test/Cardano/Testnet/Test/Cli/LeadershipSchedule.hs

+        , cardanoNodes =
+          [ SpoNodeOptions Nothing []
+          , SpoNodeOptions Nothing []
+          , SpoNodeOptions Nothing []
+          ]


Suggested change

, cardanoNodes =

[ SpoNodeOptions Nothing []

, SpoNodeOptions Nothing []

, SpoNodeOptions Nothing []

]

, cardanoNodes = repeat 3 $ SpoNodeOptions Nothing []

smelc · 2024-10-14T08:31:54Z

cardano-testnet/src/Testnet/Types.hs

@@ -79,6 +85,13 @@ data KeyPair k = KeyPair
  , signingKey :: forall dir. (File (SKey k) dir)
  }

+type instance Element (KeyPair k) = FilePath
+instance MonoFunctor (KeyPair k) where


TIL MonoFunctor 😉 (it's nice!)

smelc · 2024-10-14T08:32:35Z

cardano-testnet/test/cardano-testnet-test/Cardano/Testnet/Test/Gov/CommitteeAddNew.hs

+      -- replicate votes requested number of times
+      mkVotes :: [(Int, String)] -- ^ [(count, vote)]
+              -> [(String, Int)] -- ^ [(vote, ordering number)]
+      mkVotes votes = zip (concatMap (uncurry replicate) votes) [1..]


Nice change 👍

smelc · 2024-10-14T08:33:22Z

cardano-testnet/test/cardano-testnet-test/Cardano/Testnet/Test/Gov/ProposeNewConstitutionSPO.hs

+        , cardanoNodes =
+          [ SpoNodeOptions Nothing []
+          , SpoNodeOptions Nothing []
+          , SpoNodeOptions Nothing []
+          ]


Suggested change

, cardanoNodes =

[ SpoNodeOptions Nothing []

, SpoNodeOptions Nothing []

, SpoNodeOptions Nothing []

]

, cardanoNodes = repeat 3 $ SpoNodeOptions Nothing []

carbolymer changed the title ~~Mgalazyn/feature/enable configurable testnet topology~~ cardano-testnet | configurable SPO and relays count Oct 10, 2024

carbolymer changed the title ~~cardano-testnet | configurable SPO and relays count~~ cardano-testnet | configurable SPO and relays count, enable parallel execution of the test suite Oct 10, 2024

carbolymer force-pushed the mgalazyn/feature/enable-configurable-testnet-topology branch 3 times, most recently from bc21882 to f6be47e Compare October 10, 2024 13:42

carbolymer marked this pull request as ready for review October 10, 2024 14:01

carbolymer requested a review from a team as a code owner October 10, 2024 14:01

carbolymer commented Oct 10, 2024

View reviewed changes

cardano-testnet/src/Testnet/Components/Configuration.hs Outdated Show resolved Hide resolved

Jimbo4350 reviewed Oct 10, 2024

View reviewed changes

carbolymer force-pushed the mgalazyn/feature/enable-configurable-testnet-topology branch 10 times, most recently from 11db08a to 130551d Compare October 11, 2024 09:23

Jimbo4350 approved these changes Oct 11, 2024

View reviewed changes

carbolymer force-pushed the mgalazyn/feature/enable-configurable-testnet-topology branch from 130551d to 8ec596d Compare October 14, 2024 06:38

carbolymer enabled auto-merge October 14, 2024 06:38

carbolymer added 4 commits October 14, 2024 10:04

Enable running testnet tests using all threads

20ad401

Add default node and keys directories to Testnet.Defaults

713e099

Make testnet pools and relays configurable

9f76a8e

Update and reenable tests working with a single SPO

c4d12d1

carbolymer force-pushed the mgalazyn/feature/enable-configurable-testnet-topology branch from 8ec596d to c4d12d1 Compare October 14, 2024 08:04

smelc reviewed Oct 14, 2024

View reviewed changes

carbolymer added this pull request to the merge queue Oct 14, 2024

smelc mentioned this pull request Oct 14, 2024

cardano-testnet: QoL changes #6008

Merged

Merged via the queue into master with commit 6beea61 Oct 14, 2024
23 of 26 checks passed

carbolymer deleted the mgalazyn/feature/enable-configurable-testnet-topology branch October 14, 2024 09:21

carbolymer mentioned this pull request Oct 14, 2024

cardano-testnet | Remove NodeRuntime type #6013

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cardano-testnet | configurable SPO and relays count, enable parallel execution of the test suite #6007

cardano-testnet | configurable SPO and relays count, enable parallel execution of the test suite #6007

carbolymer commented Oct 10, 2024 •

edited

Loading

Jimbo4350 left a comment

Jimbo4350 Oct 10, 2024

smelc Oct 10, 2024

Jimbo4350 Oct 10, 2024

carbolymer Oct 14, 2024

Jimbo4350 Oct 10, 2024

carbolymer Oct 11, 2024

smelc commented Oct 11, 2024

Jimbo4350 left a comment

Jimbo4350 Oct 11, 2024

Jimbo4350 Oct 11, 2024

carbolymer Oct 14, 2024

smelc Oct 14, 2024

carbolymer Oct 14, 2024

Jimbo4350 Oct 11, 2024

carbolymer Oct 14, 2024

Jimbo4350 Oct 11, 2024

carbolymer Oct 14, 2024

carbolymer Oct 14, 2024

smelc Oct 14, 2024

carbolymer Oct 14, 2024 •

edited

Loading

carbolymer Oct 14, 2024

smelc Oct 14, 2024

carbolymer Oct 14, 2024 •

edited

Loading

smelc Oct 14, 2024

smelc Oct 14, 2024

smelc Oct 14, 2024

smelc Oct 14, 2024

	-- > │ │ └── drep.{skey,drep.vkey}
	-- > │ │ └── drep.{skey,vkey}

cardano-testnet | configurable SPO and relays count, enable parallel execution of the test suite #6007

cardano-testnet | configurable SPO and relays count, enable parallel execution of the test suite #6007

Conversation

carbolymer commented Oct 10, 2024 • edited Loading

Description

Checklist

Note on CI

Jimbo4350 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smelc commented Oct 11, 2024

Jimbo4350 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carbolymer Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carbolymer Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carbolymer commented Oct 10, 2024 •

edited

Loading

carbolymer Oct 14, 2024 •

edited

Loading

carbolymer Oct 14, 2024 •

edited

Loading