Skip to content

feat(remote-config): make sure relay, mirror node, explorers and block nodes can be deployed multiple times #2009

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

Open
wants to merge 175 commits into
base: main
Choose a base branch
from

Conversation

instamenta
Copy link
Contributor

@instamenta instamenta commented May 23, 2025

Description

Related Issues

  • Closes #
  • Related to #

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

The following manual testing was done:

  • TBD

The following was not tested:

  • TBD
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]>
…-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]>
…-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]>
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]>
…-config-logic-so-the-components-names-are-always-unique-change-logic-from-hard-delete-to-soft-delete-for-components

# Conflicts:
#	src/commands/explorer.ts
#	src/commands/mirror-node.ts
#	src/commands/network.ts
#	src/commands/relay.ts
#	src/core/config/remote/types.ts
#	test/unit/commands/network.test.ts
Signed-off-by: Zhan Milenkov <[email protected]>
…ion layer related to the remote config

Signed-off-by: Zhan Milenkov <[email protected]>
instamenta added 21 commits May 30, 2025 09:32
Signed-off-by: Zhan Milenkov <[email protected]>
…-mirror-node-explorer-and-block-nodes-can-be-deployed-multiple-times
Signed-off-by: Zhan Milenkov <[email protected]>
… logic for chart manager's isChartInstalled method

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]>
Signed-off-by: Zhan Milenkov <[email protected]>
Signed-off-by: Zhan Milenkov <[email protected]>
…-mirror-node-explorer-and-block-nodes-can-be-deployed-multiple-times
@instamenta instamenta marked this pull request as ready for review May 30, 2025 17:03
@instamenta instamenta requested review from a team as code owners May 30, 2025 17:03
Signed-off-by: Zhan Milenkov <[email protected]>
npm run solo-test -- explorer destroy --deployment "${SOLO_DEPLOYMENT}" --force
npm run solo-test -- relay destroy -i node1,node2 --deployment "${SOLO_DEPLOYMENT}" --cluster-ref kind-${SOLO_CLUSTER_NAME}
npm run solo-test -- mirror-node destroy --deployment "${SOLO_DEPLOYMENT}" --force
npm run solo-test -- explorer destroy --id 1 --deployment "${SOLO_DEPLOYMENT}" --force
Copy link
Contributor

Choose a reason for hiding this comment

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

for adding these IDs, I think if there is only one of them, they shouldn't have to specify which one to delete. This way it will be backwards compatible.

Copy link
Contributor

Choose a reason for hiding this comment

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

src/data/schema/model/remote/state/component-ids-shema.ts
misspelled schema

import {Exclude, Expose} from 'class-transformer';

@Exclude()
export class ComponentIdsShema {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export class ComponentIdsShema {
export class ComponentIdsSchema {

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this class is needed. I see that we already have a id inside of ComponentStateMetadataSchema. I think we talked about when deleting components to leave the component within the remote config (or did we get rid of that idea to not keep unused objects around? I don't see a deleted or similar in the DeploymentPhase...).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we got rid of soft delete

);

return new HAProxyStateSchema(metadata);
return new HAProxyStateSchema(this.getMetadata(ComponentTypes.HaProxy, clusterReference, namespace));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return new HAProxyStateSchema(this.getMetadata(ComponentTypes.HaProxy, clusterReference, namespace));
return new HaProxyStateSchema(this.getMetadata(ComponentTypes.HaProxy, clusterReference, namespace));

@@ -5,6 +5,7 @@ import {type ComponentTypes} from '../enumerations/component-types.js';
import {type DeploymentPhase} from '../../../../data/schema/model/remote/deployment-phase.js';
import {type ClusterReference, type ComponentId} from '../../../../types/index.js';
import {type DeploymentStateSchema} from '../../../../data/schema/model/remote/deployment-state-schema.js';
import {type ComponentIdsShema} from '../../../../data/schema/model/remote/state/component-ids-shema.js';
Copy link
Contributor

Choose a reason for hiding this comment

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

the only place in the business layer that should reference or have access to the data layer schemas is the runtime state. the runtime state should transfer the schema contents into a business layer object and also handle converting the business layer object into a schema object. the schema objects are really only for reading and writing (and migrating)

Comment on lines +184 to +189
const secrets: {
data: Record<string, string>;
name: string;
namespace: string;
type: string;
labels: Record<string, string>;
Copy link
Contributor

Choose a reason for hiding this comment

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

we should create a src/integration/kube/resources/secret/secret.ts, it was missed during our creation of the kube rewrite.

constName: 'id',
name: 'id',
definition: {
describe: 'ID',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
describe: 'ID',
describe: 'The numeric identifier for the component',

@jeromy-cannon jeromy-cannon added the PR: Unresolved Comments A pull request where there are comments and they need to be resolved. label May 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Unresolved Comments A pull request where there are comments and they need to be resolved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make sure relay, mirror node, explorer and block nodes can be deployed multiple times
3 participants