Skip to content
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

feat: solo deployment create should use the context and cluster provided for where to save the remote config #1142

Merged

Conversation

instamenta
Copy link
Contributor

Description

Addresses issue preventing passing email address via flags

Related Issues

Copy link
Contributor

github-actions bot commented Jan 13, 2025

Unit Test Results - Linux

  1 files  ±0   59 suites  ±0   3s ⏱️ ±0s
233 tests ±0  233 ✅ ±0  0 💤 ±0  0 ❌ ±0 
238 runs  ±0  238 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 4282698. ± Comparison against base commit ab018a7.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jan 13, 2025

Unit Test Results - Windows

  1 files  ±0   59 suites  ±0   14s ⏱️ -2s
233 tests ±0  233 ✅ ±0  0 💤 ±0  0 ❌ ±0 
238 runs  ±0  238 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 4282698. ± Comparison against base commit ab018a7.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jan 13, 2025

E2E Test Report

 17 files  ±0  126 suites  ±0   1h 27m 38s ⏱️ + 1m 32s
258 tests ±0  258 ✅ ±0  0 💤 ±0  0 ❌ ±0 
269 runs  ±0  269 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 4282698. ± Comparison against base commit ab018a7.

♻️ This comment has been updated with latest results.

Copy link

codacy-production bot commented Jan 13, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.05% (target: -1.00%) 90.99%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (ab018a7) 21308 17829 83.67%
Head commit (4282698) 21366 (+58) 17888 (+59) 83.72% (+0.05%)

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 (#1142) 111 101 90.99%

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

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

Copy link

codecov bot commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 90.99099% with 10 lines in your changes missing coverage. Please review.

Project coverage is 82.94%. Comparing base (ab018a7) to head (4282698).
Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
src/core/config/local_config.ts 0.00% 7 Missing ⚠️
src/commands/cluster/handlers.ts 0.00% 2 Missing ⚠️
src/core/k8.ts 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1142      +/-   ##
==========================================
+ Coverage   82.91%   82.94%   +0.03%     
==========================================
  Files          77       77              
  Lines       21335    21366      +31     
  Branches     1808     1455     -353     
==========================================
+ Hits        17690    17723      +33     
- Misses       3559     3608      +49     
+ Partials       86       35      -51     
Files with missing lines Coverage Δ
src/commands/cluster/configs.ts 84.55% <100.00%> (+1.35%) ⬆️
src/commands/cluster/tasks.ts 88.80% <100.00%> (+0.34%) ⬆️
src/core/k8.ts 85.27% <0.00%> (+0.40%) ⬆️
src/commands/cluster/handlers.ts 84.56% <0.00%> (ø)
src/core/config/local_config.ts 65.72% <0.00%> (-1.36%) ⬇️

... and 23 files with indirect coverage changes

Impacted file tree graph

Copy link
Contributor

@jeromy-cannon jeromy-cannon left a comment

Choose a reason for hiding this comment

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

❯ rm -Rf ~/.solo
❯ kind create cluster -n solo-e2e
Creating cluster "solo-e2e" ...
 ✓ Ensuring node image (kindest/node:v1.27.3) 🖼
 ✓ Preparing nodes 📦  
 ✓ Writing configuration 📜 
 ✓ Starting control-plane 🕹️ 
 ✓ Installing CNI 🔌 
 ✓ Installing StorageClass 💾 
Set kubectl context to "kind-solo-e2e"
You can now use your cluster with:

kubectl cluster-info --context kind-solo-e2e

Have a question, bug, or feature request? Let us know! https://kind.sigs.k8s.io/#community 🙂
❯ kind create cluster -n solo
Creating cluster "solo" ...
 ✓ Ensuring node image (kindest/node:v1.27.3) 🖼
 ✓ Preparing nodes 📦  
 ✓ Writing configuration 📜 
 ✓ Starting control-plane 🕹️ 
 ✓ Installing CNI 🔌 
 ✓ Installing StorageClass 💾 
Set kubectl context to "kind-solo"
You can now use your cluster with:

kubectl cluster-info --context kind-solo

Have a nice day! 👋
❯ git status
On branch 00976-solo-deployment-create-should-use-the-context
Your branch is up to date with 'origin/00976-solo-deployment-create-should-use-the-context'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)
        examples/address-book/

nothing added to commit but untracked files present (use "git add" to track)
❯ git pull
Already up to date.
❯ npm run build

> @hashgraph/[email protected] build
> rm -Rf dist && tsc && node resources/post-build-script.js

❯ npm run solo -- deployment create -n jeromy-solo --deployment-clusters kind-solo-e2e  --context-cluster kind-solo-e2e=kind-solo-e2e

> @hashgraph/[email protected] solo
> node --no-deprecation --no-warnings dist/solo.js deployment create -n jeromy-solo --deployment-clusters kind-solo-e2e --context-cluster kind-solo-e2e=kind-solo-e2e


******************************* Solo *********************************************
Version                 : 0.32.0
Kubernetes Context      : kind-solo
Kubernetes Cluster      : kind-solo
Kubernetes Namespace    : jeromy-solo
**********************************************************************************
✔ Initialize
  ✔ Acquire lease - lease acquired successfully, attempt: 1/10
⠴ Prompt local configuration
◼ Validate cluster connections
◼ Create remote config
◼ Read local configuration settings
◼ Update local configuration

? Select kubectl context to be associated with cluster: kind-solo-e2e … 
❯ docker
  docker-desktop
  gke-alex
  gke-edge
  gke-fullstack
  gke-jeromy
  gke-secure
  re-lat
  kind-solo-e2e
  kind-solo

It is prompting me for the cluster, even though I supplied the flag with the same information.

Then when I select it, I get this error:

******************************* Solo *********************************************
Version                 : 0.32.0
Kubernetes Context      : kind-solo
Kubernetes Cluster      : kind-solo
Kubernetes Namespace    : jeromy-solo
**********************************************************************************
✔ Initialize
  ✔ Acquire lease - lease acquired successfully, attempt: 1/10
✖ ENOENT: no such file or directory, open '/Users/user/.solo/cache/local-config.yaml'
◼ Validate cluster connections
◼ Create remote config
◼ Read local configuration settings
◼ Update local configuration
*********************************** ERROR *****************************************
Error installing chart solo-deployment
***********************************************************************************

I think Ivo moved the init method that creates the directories into the basecommand, you just need to call it at the beginning of the deployment command. this is because I did rm -Rf on the ~/.solo directory.

if I run solo init and then run the command again, I now get a different error:

❯ npm run solo -- init

> @hashgraph/[email protected] solo
> node --no-deprecation --no-warnings dist/solo.js init


******************************* Solo *********************************************
Version                 : 0.32.0
Kubernetes Context      : kind-solo
Kubernetes Cluster      : kind-solo
**********************************************************************************
✔ Setup home directory and cache
✔ Check dependencies [11s]
  ✔ Check dependency: helm [OS: darwin, Release: 23.6.0, Arch: arm64] [11s]
✔ Setup chart manager [1s]
✔ Copy templates in '/Users/user/.solo/cache'


***************************************************************************************
Note: solo stores various artifacts (config, logs, keys etc.) in its home directory: /Users/user/.solo
If a full reset is needed, delete the directory or relevant sub-directories before running 'solo init'.
***************************************************************************************
❯ npm run solo -- deployment create -n jeromy-solo --deployment-clusters kind-solo-e2e  --context-cluster kind-solo-e2e=kind-solo-e2e

> @hashgraph/[email protected] solo
> node --no-deprecation --no-warnings dist/solo.js deployment create -n jeromy-solo --deployment-clusters kind-solo-e2e --context-cluster kind-solo-e2e=kind-solo-e2e


******************************* Solo *********************************************
Version                 : 0.32.0
Kubernetes Context      : kind-solo
Kubernetes Cluster      : kind-solo
Kubernetes Namespace    : jeromy-solo
**********************************************************************************
✔ Initialize
  ✔ Acquire lease - lease acquired successfully, attempt: 1/10
✔ Prompt local configuration [7s]
✔ Validate cluster connections
  ✔ Testing connection to cluster: kind-solo-e2e
✖ Failed to read remote config from cluster
◼ Read local configuration settings
◼ Update local configuration
*********************************** ERROR *****************************************
failed to read existing leases, unexpected server response of '500' received
***********************************************************************************

@instamenta , this is still happening after I pulled your latest.

@jeromy-cannon jeromy-cannon added PR: Needs Approval A pull request that needs reviews and approvals. PR: Unresolved Comments A pull request where there are comments and they need to be resolved. and removed PR: Needs Approval A pull request that needs reviews and approvals. labels Jan 15, 2025
…icky, addeded logic to test context and not the cluster and return readable error message

Signed-off-by: instamenta <[email protected]>
Signed-off-by: instamenta <[email protected]>
@jeromy-cannon jeromy-cannon added the PR: Merge Conflicts A pull request that has merge conflicts that need to be resolved. label Jan 16, 2025
@jeromy-cannon jeromy-cannon self-requested a review January 16, 2025 13:54
@jeromy-cannon jeromy-cannon removed the PR: Unresolved Comments A pull request where there are comments and they need to be resolved. label Jan 16, 2025
@jeromy-cannon
Copy link
Contributor

@instamenta , please resolve merge conflicts

…-create-should-use-the-context

# Conflicts:
#	src/commands/cluster/handlers.ts
#	src/commands/cluster/tasks.ts
#	src/commands/deployment.ts
#	src/core/k8.ts
#	test/unit/commands/cluster.test.ts
Signed-off-by: instamenta <[email protected]>
Signed-off-by: instamenta <[email protected]>
@jeromy-cannon jeromy-cannon added PR: Needs Approval A pull request that needs reviews and approvals. and removed PR: Merge Conflicts A pull request that has merge conflicts that need to be resolved. labels Jan 17, 2025
@jeromy-cannon jeromy-cannon added PR: Ready to Merge A pull request that is ready to merge. and removed PR: Needs Approval A pull request that needs reviews and approvals. labels Jan 17, 2025
@jeromy-cannon jeromy-cannon merged commit fe42edd into main Jan 18, 2025
42 checks passed
@jeromy-cannon jeromy-cannon deleted the 00976-solo-deployment-create-should-use-the-context branch January 18, 2025 08:22
swirlds-automation added a commit that referenced this pull request Jan 24, 2025
## [0.34.0](v0.33.0...v0.34.0) (2025-01-24)

### Features

* `solo deployment create` should use the context and cluster provided for where to save the remote config ([#1142](#1142)) ([fe42edd](fe42edd))
* Connect to multicluster deployments and validate remoteConfigs ([#1141](#1141)) ([c78e226](c78e226))
* for v0.59.x or greater set the internal IP address to 127.0.0.1 to avoid an ISS, in config.txt ([#1162](#1162)) ([4ca488b](4ca488b))
* node upgrade command and new e2e tests ([#1133](#1133)) ([1cf5893](1cf5893))
* separate explorer from mirror node install/uninstall ([#1177](#1177)) ([0887fa6](0887fa6))
* solo network destroy should update remote-config ([#1155](#1155)) ([98b028f](98b028f))
* Update solo to load remote config near entry point ([#1176](#1176)) ([473a650](473a650))

### Bug Fixes

* merge error due to change of remote_config_tasks ([#1197](#1197)) ([9d1a8cb](9d1a8cb))
* Normalize mirror node resources path ([#1175](#1175)) ([ab018a7](ab018a7))
* Refactor RemoteConfigTasks class ([#1185](#1185)) ([66cfc4d](66cfc4d))
* remove k8.getKubeConfig ([#1182](#1182)) ([89c557a](89c557a))
* Set mirror node importer startDate ([#1174](#1174)) ([9d9ef53](9d9ef53))
* update hedera explorer chart version and location ([#1188](#1188)) ([0c415ef](0c415ef))
* use "double fork" to invoke port forward within Taskfile ([#1148](#1148)) ([d662d3f](d662d3f))
@swirlds-automation
Copy link
Contributor

🎉 This PR is included in version 0.34.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Ready to Merge A pull request that is ready to merge. released
Projects
None yet
3 participants