Skip to content

feat(remote-config): add support for multiple components of the same type, and change from hard to soft delete #1846

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 87 commits into
base: main
Choose a base branch
from

Conversation

instamenta
Copy link
Contributor

@instamenta instamenta commented Apr 15, 2025

Description

  • Changed remote config names to be indexes which are indexes

Related Issues

Pull request (PR) checklist

  • This PR added tests (unit, integration, and/or end-to-end)
  • This PR updated documentation
  • This PR added no TODOs or commented out code
  • This PR has no breaking changes
  • Any technical debt has been documented as a separate issue and linked to this PR
  • Any package.json changes have been explained to and approved by a repository manager
  • All related issues have been linked to this PR
  • All changes in this PR are included in the description
  • When this PR merges the commits will be squashed and the title will be used as the commit message, the 'commit message guidelines' below have been followed

Testing

  • This PR added unit tests
  • This PR added integration/end-to-end tests
  • These changes required manual testing that is documented below
  • Anything not tested is documented
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:
  1. BREAKING CHANGE: a commit that has a footer BREAKING CHANGE:, or appends a ! after the type/scope, introduces a breaking API change (correlating with MAJOR in Semantic Versioning). A BREAKING CHANGE can be part of commits of any type. NOTE: currently breaking changes will only bump the MAJOR version.
  2. The title is prefixed with one of the following:
Prefix Description Semantic Version Update Captured in Release Notes
feat: a new feature MINOR Yes
fix: a bug fix PATCH Yes
perf: performance PATCH Yes
refactor: code change that isn't feature or fix none No
test: adding missing tests none No
docs: changes to documentation none Yes
build: changes to build process none No
ci: changes to CI configuration none No
style: formatting, missing semi-colons, etc none No
chore: updating grunt tasks etc; no production code change none No

…-config-logic-so-the-components-names-are-always-unique-change-logic-from-hard-delete-to-soft-delete-for-components

# Conflicts:
#	src/core/config/remote/components/consensus-node-component.ts
#	src/core/config/remote/components/relay-component.ts
#	src/core/config/remote/remote-config-manager.ts
Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
Copy link
Contributor

github-actions bot commented Apr 15, 2025

Unit Test Results - Linux

32 tests  ±0   32 ✅ ±0   0s ⏱️ ±0s
16 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 05ffff4. ± Comparison against base commit 1a51877.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Apr 15, 2025

Unit Test Results - Windows

  1 files  ± 0   73 suites   - 12   4s ⏱️ ±0s
279 tests  - 81  279 ✅  - 81  0 💤 ±0  0 ❌ ±0 
283 runs   - 82  283 ✅  - 82  0 💤 ±0  0 ❌ ±0 

Results for commit 05ffff4. ± Comparison against base commit 1a51877.

This pull request removes 89 and adds 8 tests. Note that renamed tests count towards both.
calling toObject() should return a valid data ‑ ConsensusNodeComponent calling toObject() should return a valid data
calling toObject() should return a valid data ‑ EnvoyProxyComponent calling toObject() should return a valid data
calling toObject() should return a valid data ‑ HaProxyComponent calling toObject() should return a valid data
calling toObject() should return a valid data ‑ MirrorNodeComponent calling toObject() should return a valid data
calling toObject() should return a valid data ‑ MirrorNodeExplorerComponent calling toObject() should return a valid data
calling toObject() should return a valid data ‑ RelayComponent calling toObject() should return a valid data
should be able to add new command to history with addCommandToHistory() ‑ RemoteConfigDataWrapper should be able to add new command to history with addCommandToHistory()
should be able to add new component with the .add() method ‑ ComponentsDataWrapper should be able to add new component with the .add() method
should be able to create a instance ‑ RemoteConfigDataWrapper should be able to create a instance
should be able to create new instance of the class with valid data ‑ Migration should be able to create new instance of the class with valid data
…
should be able to add new component with the .addNewComponent() method ‑ ComponentsDataWrapper should be able to add new component with the .addNewComponent() method
should be able to change node state with the .changeNodeState(() ‑ ComponentsDataWrapper should be able to change node state with the .changeNodeState(()
should be able to get components with .getComponent() ‑ ComponentsDataWrapper should be able to get components with .getComponent()
should be able to remove component with the .removeComponent() ‑ ComponentsDataWrapper should be able to remove component with the .removeComponent()
should fail if trying to get component that doesn't exist with .getComponent() ‑ ComponentsDataWrapper should fail if trying to get component that doesn't exist with .getComponent()
should not be able to add new component with the .addNewComponent() method if it already exist ‑ ComponentsDataWrapper should not be able to add new component with the .addNewComponent() method if it already exist
should not be able to edit component with the .editComponent() if it doesn't exist  ‑ ComponentsDataWrapper should not be able to edit component with the .editComponent() if it doesn't exist 
should not be able to remove component with the .removeComponent() if it doesn't exist  ‑ ComponentsDataWrapper should not be able to remove component with the .removeComponent() if it doesn't exist 

♻️ This comment has been updated with latest results.

@instamenta instamenta marked this pull request as ready for review April 15, 2025 09:04
@instamenta instamenta requested review from a team as code owners April 15, 2025 09:04
@instamenta instamenta added the PR: Needs Manager Approval A pull request that needs review from a manager. label Apr 15, 2025
Copy link
Contributor

E2E Test Report

 17 files  ±0  120 suites  +1   1h 30m 29s ⏱️ + 2m 34s
314 tests +4  314 ✅ +4  0 💤 ±0  0 ❌ ±0 
330 runs  +4  330 ✅ +4  0 💤 ±0  0 ❌ ±0 

Results for commit 3292683. ± Comparison against base commit 0088b76.

This pull request removes 12 and adds 16 tests. Note that renamed tests count towards both.
should fail if component is not present ‑ RemoteConfigValidator Consensus Nodes validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator Envoy Proxies validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator HaProxies validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator Mirror Node Components validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator Mirror Node Explorers validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator Relays validation should fail if component is not present
should succeed if component is present ‑ RemoteConfigValidator Consensus Nodes validation should succeed if component is present
should succeed if component is present ‑ RemoteConfigValidator Envoy Proxies validation should succeed if component is present
should succeed if component is present ‑ RemoteConfigValidator HaProxies validation should succeed if component is present
should succeed if component is present ‑ RemoteConfigValidator Mirror Node Components validation should succeed if component is present
…
Should not validate consensus nodes if skipConsensusNodes is enabled ‑ RemoteConfigValidator Additional test cases Should not validate consensus nodes if skipConsensusNodes is enabled
Should not validate consensus nodes if status is non-deployed  ‑ RemoteConfigValidator Additional test cases Should not validate consensus nodes if status is non-deployed 
Should not validate consensus nodes if status is requested  ‑ RemoteConfigValidator Additional test cases Should not validate consensus nodes if status is requested 
Should not validate disabled components ‑ RemoteConfigValidator Additional test cases Should not validate disabled components
should fail if component is not present ‑ RemoteConfigValidator Consensus node validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator Envoy proxy validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator HaProxy validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator Mirror node explorer validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator Mirror node validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator Relay validation should fail if component is not present
…

Copy link
Contributor

github-actions bot commented Apr 15, 2025

E2E Test Report

 17 files  ±0  120 suites  +1   1h 25m 17s ⏱️ - 1m 52s
313 tests +3  313 ✅ +3  0 💤 ±0  0 ❌ ±0 
329 runs  +3  329 ✅ +3  0 💤 ±0  0 ❌ ±0 

Results for commit 3788778. ± Comparison against base commit 484f219.

This pull request removes 12 and adds 15 tests. Note that renamed tests count towards both.
should fail if component is not present ‑ RemoteConfigValidator Consensus Nodes validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator Envoy Proxies validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator HaProxies validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator Mirror Node Components validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator Mirror Node Explorers validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator Relays validation should fail if component is not present
should succeed if component is present ‑ RemoteConfigValidator Consensus Nodes validation should succeed if component is present
should succeed if component is present ‑ RemoteConfigValidator Envoy Proxies validation should succeed if component is present
should succeed if component is present ‑ RemoteConfigValidator HaProxies validation should succeed if component is present
should succeed if component is present ‑ RemoteConfigValidator Mirror Node Components validation should succeed if component is present
…
Should not validate consensus nodes if skipConsensusNodes is enabled ‑ RemoteConfigValidator Additional test cases Should not validate consensus nodes if skipConsensusNodes is enabled
Should not validate consensus nodes if status is requested  ‑ RemoteConfigValidator Additional test cases Should not validate consensus nodes if status is requested 
Should not validate consensus nodes if status is stopped  ‑ RemoteConfigValidator Additional test cases Should not validate consensus nodes if status is stopped 
should fail if component is not present ‑ RemoteConfigValidator Consensus node validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator Envoy proxy validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator HaProxy validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator Mirror node explorer validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator Mirror node validation should fail if component is not present
should fail if component is not present ‑ RemoteConfigValidator Relay validation should fail if component is not present
should succeed if component is present ‑ RemoteConfigValidator Consensus node validation should succeed if component is present
…

♻️ This comment has been updated with latest results.

Copy link

codacy-production bot commented Apr 15, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-0.17% (target: -1.00%) 92.18%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (484f219) 34816 28961 83.18%
Head commit (3788778) 34818 (+2) 28902 (-59) 83.01% (-0.17%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#1846) 895 825 92.18%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Copy link

codecov bot commented Apr 15, 2025

Codecov Report

Attention: Patch coverage is 91.73184% with 74 lines in your changes missing coverage. Please review.

Project coverage is 81.97%. Comparing base (484f219) to head (3788778).

Files with missing lines Patch % Lines
src/core/config/remote/components-data-wrapper.ts 87.37% 25 Missing ⚠️
src/commands/explorer.ts 55.55% 20 Missing ⚠️
src/commands/mirror-node.ts 43.47% 13 Missing ⚠️
src/commands/node/tasks.ts 79.16% 5 Missing ⚠️
...data/schema/model/remote/state/relay-node-state.ts 0.00% 5 Missing ⚠️
...rc/core/config/remote/components/base-component.ts 92.85% 3 Missing ⚠️
src/commands/node/handlers.ts 98.14% 1 Missing ⚠️
src/core/chart-manager.ts 0.00% 1 Missing ⚠️
...c/core/config/remote/components/relay-component.ts 95.83% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1846      +/-   ##
==========================================
- Coverage   82.18%   81.97%   -0.21%     
==========================================
  Files         243      245       +2     
  Lines       34816    34818       +2     
  Branches     2956     2303     -653     
==========================================
- Hits        28612    28543      -69     
- Misses       6051     6165     +114     
+ Partials      153      110      -43     
Files with missing lines Coverage Δ
src/commands/deployment.ts 68.06% <100.00%> (ø)
src/commands/network.ts 79.22% <100.00%> (-0.80%) ⬇️
src/commands/relay.ts 85.83% <100.00%> (+0.27%) ⬆️
src/core/config-manager.ts 100.00% <100.00%> (ø)
src/core/config/remote/cluster.ts 76.00% <100.00%> (-18.37%) ⬇️
...rc/core/config/remote/common-flags-data-wrapper.ts 80.35% <100.00%> (ø)
...core/config/remote/components/component-factory.ts 100.00% <100.00%> (ø)
...nfig/remote/components/consensus-node-component.ts 100.00% <100.00%> (ø)
.../config/remote/components/envoy-proxy-component.ts 100.00% <100.00%> (ø)
...ore/config/remote/components/ha-proxy-component.ts 100.00% <100.00%> (ø)
... and 23 more

... and 72 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…-config-logic-so-the-components-names-are-always-unique-change-logic-from-hard-delete-to-soft-delete-for-components
Signed-off-by: Zhan Milenkov <[email protected]>
… on RelayComponent to consensusNodeIds, removed nodeStates on ConsensusNodeComponent

Signed-off-by: Zhan Milenkov <[email protected]>
@instamenta instamenta added Blocked Further development work is blocked by other item and removed PR: Needs Manager Approval A pull request that needs review from a manager. labels Apr 16, 2025
instamenta and others added 30 commits May 12, 2025 09:12
Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
…-config-logic-so-the-components-names-are-always-unique-change-logic-from-hard-delete-to-soft-delete-for-components

# Conflicts:
#	src/business/runtime-state/local-config-runtime-state.ts
#	src/core/dependency-injection/container-init.ts
#	src/data/backend/impl/yaml-config-map-storage-backend.ts
#	test/unit/data/configuration/impl/local-config-source.test.ts
#	test/unit/data/schema/migration/impl/remote/remote-config-v1-migration.test.ts
#	test/unit/data/schema/model/local/local-config.test.ts
#	test/unit/data/schema/model/remote/remote-config.test.ts
Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Nathan Klick <[email protected]>
Signed-off-by: Nathan Klick <[email protected]>
…ss facade implementations

Signed-off-by: Jeromy Cannon <[email protected]>
… and encapsulatedArray for clarity in mutable facade implementations

Signed-off-by: Jeromy Cannon <[email protected]>
…-config-logic-so-the-components-names-are-always-unique-change-logic-from-hard-delete-to-soft-delete-for-components

# Conflicts:
#	src/core/config/remote/common-flags-data-wrapper.ts
#	src/core/config/remote/interfaces/remote-config-common-flags-struct.ts
#	src/core/config/remote/interfaces/remote-config-metadata-struct.ts
#	src/core/config/remote/metadata.ts
#	src/core/config/remote/remote-config-manager.ts
#	src/core/config/remote/remote-config-validator.ts
#	test/e2e/integration/core/remote-config-validator.test.ts
#	test/unit/core/config/remote/metadata.test.ts
#	test/unit/data/schema/migration/impl/remote/remote-config-v1-migration.test.ts
Signed-off-by: Zhan Milenkov <[email protected]>
…-refactor-remote-config-logic-so-the-components-names-are-always-unique-change-logic-from-hard-delete-to-soft-delete-for-components

# Conflicts:
#	src/commands/deployment.ts
#	src/commands/explorer.ts
#	src/commands/network.ts
#	src/commands/node/configs.ts
#	src/commands/node/tasks.ts
#	src/commands/relay.ts
#	src/core/account-manager.ts
#	src/core/config/remote/interfaces/migration-struct.ts
#	src/core/config/remote/interfaces/remote-config-metadata-struct.ts
#	src/core/config/remote/metadata.ts
#	src/core/config/remote/migration.ts
#	src/core/config/remote/remote-config-manager.ts
#	src/core/config/remote/remote-config-validator.ts
#	src/core/middlewares.ts
#	src/core/profile-manager.ts
#	src/data/configuration/impl/remote-config-source.ts
#	src/data/schema/model/local/deployment-schema.ts
#	src/data/schema/model/remote/remote-config-metadata.ts
#	src/data/schema/model/remote/remote-config.ts
#	src/data/schema/model/remote/state/block-node-state-schema.ts
#	src/data/schema/model/remote/state/consensus-node-state-schema.ts
#	src/data/schema/model/remote/state/envoy-proxy-state-schema.ts
#	src/data/schema/model/remote/state/explorer-state-schema.ts
#	src/data/schema/model/remote/state/haproxy-state-schema.ts
#	src/data/schema/model/remote/state/mirror-node-state.ts
#	src/data/schema/model/remote/state/relay-node-state-schema.ts
#	test/e2e/commands/dual-cluster-full.test.ts
#	test/e2e/integration/core/remote-config-validator.test.ts
#	test/test-utility.ts
#	test/unit/commands/base.test.ts
#	test/unit/commands/network.test.ts
#	test/unit/core/config/remote/metadata.test.ts
#	test/unit/core/config/remote/migration.test.ts
#	test/unit/data/schema/migration/impl/remote/remote-config-v1-migration.test.ts
Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants