cli: add config command and config validate subcommand to nomad CLI #9198

th0m · 2020-10-27T23:38:13Z

This command is similar to the consul validate command and can be used to perform configuration validation before actually restarting Nomad and risking a failure if there is an issue in the configuration.

Also added three test cases to the command test suite:

Empty directory which is acceptable
Working configuration
Non-working configuration

jrasell

Thanks very much for this @th0m.

I would be curious of your thoughts, as well as others, around the command nomad config validate rather than nomad config-validate? In my mind, this allows for additional config based sub-commands in the future thus providing future flexibility while keeping the experience consistent.

th0m · 2020-10-28T20:43:43Z

Makes sense to me @jrasell, happy to change it.

jrasell

Thanks for the update. I have suggested a couple of changes along with a question regarding the error handling which I would be keen to get your feedback on as a target user for this command.

jrasell · 2020-11-02T07:58:08Z

command/config_validate_test.go

+	}
+	defer os.Remove(fh)
+
+	ui := new(cli.MockUi)


Suggested change

ui := new(cli.MockUi)

ui := cli.NewMockUi()

jrasell · 2020-11-02T07:58:21Z

command/config_validate_test.go

+		t.Fatalf("err: %s", err)
+	}
+
+	ui := new(cli.MockUi)


Suggested change

ui := new(cli.MockUi)

ui := cli.NewMockUi()

jrasell · 2020-11-02T07:58:30Z

command/config_validate_test.go

+		t.Fatalf("err: %s", err)
+	}
+
+	ui := new(cli.MockUi)


Suggested change

ui := new(cli.MockUi)

ui := cli.NewMockUi()

jrasell · 2020-11-02T08:11:49Z

command/config_validate.go

+		if err != nil {
+			c.Ui.Error(fmt.Sprintf(
+				"Error loading configuration from %s: %s", path, err))
+			return 1


I am curious on your thoughts here around the use of multierror? In the event of multiple files having validation issues, the command can output all the errors in a single run, rather than needing the operator to run the command multiple times to fix each issue which is displayed.

Great idea, I'll use multierror.

dadgar · 2020-11-07T00:17:49Z

@jrasell @th0m It may be good to refactor the isValidConfig call so that it can be used here. Currently, the command just validates the config can be parsed, but it may have settings that will cause Nomad not to be able to start:

th0m · 2021-12-13T21:03:23Z

Thanks for the review @dadgar and @jrasell.
I just rebased my branch and will follow up on your comments shortly.

th0m · 2022-01-20T21:24:55Z

@dadgar following your suggestion I've made the isValidConfig function visible to other packages to be able to use it here and perform config validation in addition to parsability validation.
I've added a test that catches a bad configuration that is otherwise parsable.

@jrasell I think I have addressed all your comments, can you please re-review?

Thanks both.

th0m · 2022-01-28T00:05:21Z

@jrasell sorry about the repeated ping but in case you missed my previous message this PR is now ready for review 🙂
Thank you!

th0m · 2022-02-07T16:54:42Z

@jrasell @tgross may I have a review here when you have a minute? Thank you so much!

tgross

Looking pretty good @th0m! I've left some comments but mostly it's just tightening up language at this point.

command/config_validate.go

th0m · 2022-02-07T19:58:37Z

Thank you @tgross, I have committed both of your suggestions and squashed the three commits into one.

tgross

LGTM

These issues were addressed.

th0m · 2022-02-09T00:27:38Z

Thanks @tgross!

github-actions · 2022-10-29T02:38:05Z

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

th0m changed the title ~~Add config-validate command to nomad CLI~~ cli: add config-validate command to nomad CLI Oct 27, 2020

jrasell reviewed Oct 28, 2020

View reviewed changes

jrasell added theme/cli type/enhancement labels Oct 28, 2020

vercel bot had a problem deploying to Preview October 28, 2020 12:54 Failure

th0m force-pushed the tlefebvre/add-config-validate-command branch from 501babd to 85b7249 Compare October 28, 2020 20:58

th0m changed the title ~~cli: add config-validate command to nomad CLI~~ cli: add config command and config validate subcommand to nomad CLI Oct 28, 2020

jrasell previously requested changes Nov 2, 2020

View reviewed changes

jrasell self-assigned this Nov 2, 2020

tgross added the stage/waiting-reply label Feb 19, 2021

Base automatically changed from master to main March 8, 2021 19:25

th0m force-pushed the tlefebvre/add-config-validate-command branch from 85b7249 to 0993c5a Compare December 13, 2021 20:46

vercel bot temporarily deployed to Preview – nomad December 13, 2021 20:46 Inactive

vercel bot deployed to Preview – nomad-storybook-and-ui December 13, 2021 20:46 View deployment

th0m force-pushed the tlefebvre/add-config-validate-command branch from 0993c5a to 793f7e8 Compare December 13, 2021 22:50

vercel bot temporarily deployed to Preview – nomad December 13, 2021 22:50 Inactive

vercel bot deployed to Preview – nomad-storybook-and-ui December 13, 2021 22:50 View deployment

th0m requested a review from jrasell January 20, 2022 18:41

th0m force-pushed the tlefebvre/add-config-validate-command branch from 793f7e8 to 45ec419 Compare January 20, 2022 21:19

vercel bot temporarily deployed to Preview – nomad January 20, 2022 21:19 Inactive

vercel bot deployed to Preview – nomad-storybook-and-ui January 20, 2022 21:19 View deployment

tgross self-requested a review February 7, 2022 16:58

tgross self-assigned this Feb 7, 2022

tgross removed the stage/waiting-reply label Feb 7, 2022

tgross requested changes Feb 7, 2022

View reviewed changes

command/config_validate.go Outdated Show resolved Hide resolved

command/config_validate.go Outdated Show resolved Hide resolved

vercel bot temporarily deployed to Preview – nomad February 7, 2022 19:54 Inactive

vercel bot deployed to Preview – nomad-storybook-and-ui February 7, 2022 19:54 View deployment

vercel bot temporarily deployed to Preview – nomad February 7, 2022 19:54 Inactive

Add config command and config validate subcommand to nomad CLI

0cfb338

th0m force-pushed the tlefebvre/add-config-validate-command branch from c8f821f to 0cfb338 Compare February 7, 2022 19:57

vercel bot temporarily deployed to Preview – nomad February 7, 2022 19:57 Inactive

vercel bot deployed to Preview – nomad-storybook-and-ui February 7, 2022 19:59 View deployment

Merge branch 'hashicorp:main' into tlefebvre/add-config-validate-command

39730d3

vercel bot deployed to Preview – nomad-storybook-and-ui February 7, 2022 20:49 View deployment

vercel bot deployed to Preview – nomad February 7, 2022 20:49 View deployment

tgross approved these changes Feb 8, 2022

View reviewed changes

tgross merged commit 41f84c6 into hashicorp:main Feb 8, 2022

tgross mentioned this pull request Feb 8, 2022

docs and changelog for nomad config validate #12031

Merged

github-actions bot locked as resolved and limited conversation to collaborators Oct 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli: add config command and config validate subcommand to nomad CLI #9198

cli: add config command and config validate subcommand to nomad CLI #9198

th0m commented Oct 27, 2020

jrasell left a comment •

edited

Loading

th0m commented Oct 28, 2020

jrasell left a comment

jrasell Nov 2, 2020

jrasell Nov 2, 2020

jrasell Nov 2, 2020

jrasell Nov 2, 2020

th0m Dec 13, 2021

dadgar commented Nov 7, 2020

th0m commented Dec 13, 2021

th0m commented Jan 20, 2022

th0m commented Jan 28, 2022 •

edited

Loading

th0m commented Feb 7, 2022

tgross left a comment

th0m commented Feb 7, 2022

tgross left a comment

th0m commented Feb 9, 2022

github-actions bot commented Oct 29, 2022

cli: add config command and config validate subcommand to nomad CLI #9198

cli: add config command and config validate subcommand to nomad CLI #9198

Conversation

th0m commented Oct 27, 2020

jrasell left a comment • edited Loading

Choose a reason for hiding this comment

th0m commented Oct 28, 2020

jrasell left a comment

Choose a reason for hiding this comment

jrasell Nov 2, 2020

Choose a reason for hiding this comment

jrasell Nov 2, 2020

Choose a reason for hiding this comment

jrasell Nov 2, 2020

Choose a reason for hiding this comment

jrasell Nov 2, 2020

Choose a reason for hiding this comment

th0m Dec 13, 2021

Choose a reason for hiding this comment

dadgar commented Nov 7, 2020

th0m commented Dec 13, 2021

th0m commented Jan 20, 2022

th0m commented Jan 28, 2022 • edited Loading

th0m commented Feb 7, 2022

tgross left a comment

Choose a reason for hiding this comment

th0m commented Feb 7, 2022

tgross left a comment

Choose a reason for hiding this comment

th0m commented Feb 9, 2022

github-actions bot commented Oct 29, 2022

jrasell left a comment •

edited

Loading

th0m commented Jan 28, 2022 •

edited

Loading