feat: add dynamic realm and shard to AccoundIds #1783
Conversation
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
# Conflicts: # src/commands/node/tasks.ts
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1783 +/- ##
==========================================
+ Coverage 82.18% 82.37% +0.19%
==========================================
Files 243 244 +1
Lines 34816 35216 +400
Branches 2956 3075 +119
==========================================
+ Hits 28612 29009 +397
+ Misses 6051 6038 -13
- Partials 153 169 +16
... and 20 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
jeromy-cannon
added
PR: Checks Failed
src/core/helpers.ts
Outdated
| @@ -567,3 +550,12 @@ export async function getAppleSiliconChipset(logger: SoloLogger) { | |||
| return ['unknown']; | |||
| } | |||
| } | |||
|
|
|||
| export async function requiresJavaSveFix(logger: SoloLogger) { | |||
| const chipSet = await getAppleSiliconChipset(logger); | |||
There was a problem hiding this comment.
This is incorrect in certain scenarios. For example, if a Hashsphere SRE uses Solo from his personal Macbook to deploy to a AWS cluster, then it will fail. Solo will be on a Macbook, whereas the Mirror Node, or anything else using this getAppleSiliconChipset() function will incorrectly assume that because Solo is running with a Silicon chipset, that the K8s cluster is also running on a Silicon chipset, which is not true in all cases.
We should probably delete the getAppleSiliconChipset() function, and instead write logic to query the cluster to get the information required to determine the chipset.
test/data/v0.35.1-local-config.yaml
Outdated
| @@ -6,10 +6,14 @@ deployments: | |||
| - e2e-cluster-1 | |||
| - e2e-cluster-2 | |||
| namespace: dual-cluster-full | |||
| realm: 1 | |||
There was a problem hiding this comment.
this file is meant to represent v0.35.1, v0.35.1 did not have a realm or shard property in it.
I see you are trying to use this for testing the migration. I recommend cloning this file, and have the clone named schema-v1-local-config.yaml. The only thing is that it also should not have realm and shard, since realm and shard don't appear until schema version 2.
| argv.setArg(flags.realm, 0); | ||
| argv.setArg(flags.shard, 0); |
There was a problem hiding this comment.
can we set these to non-zero values to ensure the functionality is working correctly?
There was a problem hiding this comment.
Non-zero realms and shards require a bump to the network-node version, which breaks most of our e2e tests.
| realm: 0, | ||
| shard: 0, |
There was a problem hiding this comment.
can we set these to non-zero values to help verify more scenarios?
There was a problem hiding this comment.
I will add scenarios with non-zero values in a following PR
src/commands/node/tasks.ts
Outdated
| @@ -2341,14 +2366,18 @@ export class NodeCommandTasks { | |||
|
|
|||
| const k8 = this.k8Factory.getK8(context); | |||
|
|
|||
| const archiveCommand = (await requiresJavaSveFix(this.logger)) | |||
There was a problem hiding this comment.
I was wondering why we can just always use zip, instead of using jar? Additionally, since we own this container we could just update the container to install zip during the docker build process.
There was a problem hiding this comment.
I'm not sure, maybe we should discuss this in the next meeting.
jeromy-cannon
added
the
PR: Unresolved Comments
…r-accounts # Conflicts: # docs/site/content/User/StepByStepGuide.md
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
Ivo-Yankov
removed
the
PR: Merge Conflicts
Signed-off-by: Ivo Yankov <[email protected]>
Ivo-Yankov
added
PR: Needs Team Approval
Signed-off-by: Ivo Yankov <[email protected]>
…ero realm Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
Signed-off-by: Ivo Yankov <[email protected]>
jeromy-cannon
added
PR: Unresolved Comments
Description
soloside. A newer version of Consensus Node (>= v0.61.0) is required to create such a deployment, but that introduces an issue withNodeUpdateTransaction, which breaks most of our E2E tests. The bump to theConsensus NodeandMirror Nodeversions should be done in a separate PR to unblock the merging of this one.solo deployment create:--shardand--realmwith default values0.LocalConfigto store theshardandrealmvalues, which are now used to build all EntityIds.LocalConfigRelated Issues
Pull request (PR) checklist
package.jsonchanges have been explained to and approved by a repository managerTesting
The following manual testing was done:
The following was not tested:
Commit message guidelines
We use 'Conventional Commits' to ensure that our commit messages are easy to read, follow a consistent format, and for automated release note generation. Please follow the guidelines below when writing your commit messages: