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

Config refactor burn down #3492

Closed
9 of 10 tasks
slackpad opened this issue Sep 25, 2017 · 5 comments
Closed
9 of 10 tasks

Config refactor burn down #3492

slackpad opened this issue Sep 25, 2017 · 5 comments
Labels
theme/internal-cleanup Used to identify tech debt, testing improvements, code refactoring, and non-impactful optimization type/docs Documentation needs to be created/updated/clarified
Milestone

Comments

@slackpad
Copy link
Contributor

slackpad commented Sep 25, 2017

This captures some small cleanups from #3480, now that it has been merged:

  • Fix config dir merge issue (see Slack context in team-consul for September 26, 12:42 AM PDT.
  • The ui config was renamed to enable_ui, breaking existing configs - Renames enable_ui to ui to keep compatibility with existing configs. #3496.
  • Require .json or .hcl extension for all config files - Enforce json or hcl extension to Consul config files, updated unit tests #3494.
  • Document that network segments configuration for client agents requires port number to be specified and fix the panic (user should get a nice error about the port being required).
  • Need to redact the Token field of checks and services in /v1/agent/self output (sanitize routine should be made recursive, it's only hitting the top level) - Recursive sanitize #3505.
  • Update the main docs with info about multiple addresses.
  • Update the change log with all the deprecations and a note about the HCL feature and multiple bind addresses (note that extensions are required for config files, too).
  • Document/follow up the monkey patches for hashicorp/go-sockaddr and hashicorp/hcl and mitchellh/mapstructure.
  • Decide where to put the documentation that was part of agent/config.Config. We can either restore it to the Config struct, add it to the RuntimeConfig or omit it from code altogether and put it in the public documentation. Plan is to use website as canonical and place link to that in the code. Internal-only configs will be documented in the code, but for all public stuff the website is the source of truth.
  • Check out the panic linked below.
@slackpad slackpad added this to the 1.0 milestone Sep 25, 2017
@slackpad
Copy link
Contributor Author

Caught this panic while running unit tests:

--- FAIL: TestPreparedQuery_Create (0.01s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x1 pc=0x40afcb8]

goroutine 3368 [running]:
testing.tRunner.func1(0xc420e1e4b0)
        /usr/local/go/src/testing/testing.go:711 +0x2d2
panic(0x4d500e0, 0x589ce80)
        /usr/local/go/src/runtime/panic.go:491 +0x283
reflect.StructTag.Lookup(0x1, 0x2, 0x4f58add, 0xc, 0xc4210e9b00, 0x58a5b10, 0xc421014ee0)
        /usr/local/go/src/reflect/type.go:1165 +0x38
reflect.StructTag.Get(0x1, 0x2, 0x4f58add, 0xc, 0xc420c74100, 0x98)
        /usr/local/go/src/reflect/type.go:1148 +0x49
github.com/hashicorp/consul/vendor/github.com/mitchellh/mapstructure.(*Decoder).decodeStruct(0xc420158e70, 0x0, 0x0, 0x4d3a540, 0xc4203e4000, 0x4f433c0, 0xc42067d200, 0x199, 0x20000000000, 0x8)
        /Users/james/projects/_go/src/github.com/hashicorp/consul/vendor/github.com/mitchellh/mapstructure/mapstructure.go:690 +0xf82
github.com/hashicorp/consul/vendor/github.com/mitchellh/mapstructure.(*Decoder).decode(0xc420158e70, 0x0, 0x0, 0x4d3a540, 0xc4203e4000, 0x4f433c0, 0xc42067d200, 0x199, 0x4d3a501, 0xc420d132c0)
        /Users/james/projects/_go/src/github.com/hashicorp/consul/vendor/github.com/mitchellh/mapstructure/mapstructure.go:225 +0x65b
github.com/hashicorp/consul/vendor/github.com/mitchellh/mapstructure.(*Decoder).Decode(0xc420158e70, 0x4d3a540, 0xc4203e4000, 0x0, 0xc4203e4000)
        /Users/james/projects/_go/src/github.com/hashicorp/consul/vendor/github.com/mitchellh/mapstructure/mapstructure.go:180 +0xb3
github.com/hashicorp/consul/agent/config.Parse(0xc4206dc000, 0x3f8, 0x4f4feb8, 0x3, 0xc4201b2900, 0x9, 0x10, 0x9, 0x0, 0x0, ...)
        /Users/james/projects/_go/src/github.com/hashicorp/consul/agent/config/config.go:107 +0x3eb
github.com/hashicorp/consul/agent/config.(*Builder).Build(0xc4203a4e00, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /Users/james/projects/_go/src/github.com/hashicorp/consul/agent/config/builder.go:231 +0x3ee
github.com/hashicorp/consul/agent/config.(*Builder).BuildAndValidate(0xc4203a4e00, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /Users/james/projects/_go/src/github.com/hashicorp/consul/agent/config/builder.go:195 +0x7b
github.com/hashicorp/consul/agent.TestConfig(0xc4210efcd8, 0x3, 0x3, 0x4f552e5)
        /Users/james/projects/_go/src/github.com/hashicorp/consul/agent/testagent.go:357 +0x42a
github.com/hashicorp/consul/agent.(*TestAgent).Start(0xc420ba3280, 0xc420ba3280)
        /Users/james/projects/_go/src/github.com/hashicorp/consul/agent/testagent.go:121 +0x2ab
github.com/hashicorp/consul/agent.NewTestAgent(0x4f67bf3, 0x18, 0x0, 0x0, 0x1)
        /Users/james/projects/_go/src/github.com/hashicorp/consul/agent/testagent.go:91 +0x7f
github.com/hashicorp/consul/agent.TestPreparedQuery_Create(0xc420e1e4b0)
        /Users/james/projects/_go/src/github.com/hashicorp/consul/agent/prepared_query_endpoint_test.go:73 +0x79
testing.tRunner(0xc420e1e4b0, 0x4f9fcc8)
        /usr/local/go/src/testing/testing.go:746 +0xd0
created by testing.(*T).Run
        /usr/local/go/src/testing/testing.go:789 +0x2de
FAIL    github.com/hashicorp/consul/agent       14.081s

@magiconair
Copy link
Contributor

I'll have a look at this one. The hcl code had issues in its decodeStruct path and that might have the same reason.

@magiconair
Copy link
Contributor

Didn't push mapstructure as hard as the hcl code yet.

@magiconair
Copy link
Contributor

Monkey patches:

@slackpad slackpad added type/docs Documentation needs to be created/updated/clarified theme/internal-cleanup Used to identify tech debt, testing improvements, code refactoring, and non-impactful optimization labels Oct 5, 2017
@slackpad
Copy link
Contributor Author

slackpad commented Oct 9, 2017

Leaving the docs just on the website has been working for now (even without x-ref links). Let's keep this way for now and close this out.

@slackpad slackpad closed this as completed Oct 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/internal-cleanup Used to identify tech debt, testing improvements, code refactoring, and non-impactful optimization type/docs Documentation needs to be created/updated/clarified
Projects
None yet
Development

No branches or pull requests

2 participants