cmd: fix create cluster bug #2852
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Could you drop a couple lines on how this PR fixes the specified bug? |
|
|
Before the fix: After the fix: |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2852 +/- ##
==========================================
+ Coverage 53.44% 53.46% +0.02%
==========================================
Files 200 200
Lines 28011 28014 +3
==========================================
+ Hits 14971 14979 +8
+ Misses 11180 11177 -3
+ Partials 1860 1858 -2 ☔ View full report in Codecov by Sentry. |
gsora
approved these changes
Feb 5, 2024
xenowits
added
the
merge when ready
gsora
pushed a commit
that referenced
this pull request
Feb 5, 2024
Fix bug in `create cluster` command. -- From @xenowits's explanation on slack: > When create cluster command is run twice in the same directory, it doesn’t error out with “Fatal error: existing node directory found, please delete it before running this command” while it should > Explanation: When we provide a definition file to the create cluster command, we don’t set the "conf.NumNodes" field when verifying the config ```go //..... func validateCreateConfig(ctx context.Context, conf clusterConfig) error { if conf.NumNodes == 0 && conf.DefFile == "" { // if there's a definition file, infer this value from it later return errors.New("missing --nodes flag") } // Check for valid network configuration. if err := validateNetworkConfig(conf); err != nil { return errors.Wrap(err, "get network config") } // BUG: conf.NumNodes is zero here!! if err := detectNodeDirs(conf.ClusterDir, conf.NumNodes); err != nil { return err } ``` category: bug ticket: #2851
obol-bulldozer bot
pushed a commit
that referenced
this pull request
Feb 5, 2024
Cherrypick of [2852](#2852) on `main-v0.19`. -- Fix bug in `create cluster` command. -- From @xenowits's explanation on slack: > When create cluster command is run twice in the same directory, it doesn’t error out with “Fatal error: existing node directory found, please delete it before running this command” while it should > Explanation: When we provide a definition file to the create cluster command, we don’t set the "conf.NumNodes" field when verifying the config ```go //..... func validateCreateConfig(ctx context.Context, conf clusterConfig) error { if conf.NumNodes == 0 && conf.DefFile == "" { // if there's a definition file, infer this value from it later return errors.New("missing --nodes flag") } // Check for valid network configuration. if err := validateNetworkConfig(conf); err != nil { return errors.Wrap(err, "get network config") } // BUG: conf.NumNodes is zero here!! if err := detectNodeDirs(conf.ClusterDir, conf.NumNodes); err != nil { return err } ``` category: bug ticket: #2851
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix bug in
create clustercommand.--
From @xenowits's explanation on slack:
category: bug
ticket: #2851
Closes #2851