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

cmd: fix create cluster bug #2854

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

gsora
Copy link
Collaborator

@gsora gsora commented Feb 5, 2024

Cherrypick of 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

//.....
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

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
@github-actions github-actions bot added the branch-release PR or Issue linked to a release branch (not main) label Feb 5, 2024
Copy link

sonarcloud bot commented Feb 5, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

codecov bot commented Feb 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8483677) 53.25% compared to head (67f576a) 53.23%.

Additional details and impacted files
@@              Coverage Diff               @@
##           main-v0.19    #2854      +/-   ##
==============================================
- Coverage       53.25%   53.23%   -0.03%     
==============================================
  Files             200      200              
  Lines           27697    27700       +3     
==============================================
- Hits            14750    14745       -5     
- Misses          11120    11126       +6     
- Partials         1827     1829       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@xenowits xenowits added the merge when ready Indicates bulldozer bot may merge when all checks pass label Feb 5, 2024
@xenowits xenowits changed the title cmd: fix create cluster bug (#2852) cmd: fix create cluster bug Feb 5, 2024
@obol-bulldozer obol-bulldozer bot merged commit 155f0db into main-v0.19 Feb 5, 2024
16 of 18 checks passed
@obol-bulldozer obol-bulldozer bot deleted the gsora/main-v0.19/cherrypick_#2852 branch February 5, 2024 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-release PR or Issue linked to a release branch (not main) merge when ready Indicates bulldozer bot may merge when all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants