search

calvn / chocolatey-consul

Chocolatey package for Consul
4 stars 27 forks source link

Removes default extraArgs #2

Closed anguswilliams closed 7 years ago

anguswilliams commented 7 years ago

Hi there,

I just got caught out by this setting which seems to have appeared in commit a67d3f25c83dd8e9b8bdcddb47312dc31a06e8c2. I was using this package to configure my windows nodes as consul agents, and managed to put my cluster in a bad state due to all my agents becoming servers all of a sudden! Would be good to remove as default and users can add if needed.

calvn commented 7 years ago

@anguswilliams sorry about that, I certainly agree and will push out a new realease! I'd recommend pinning the package to specific versions when installing to prevent potential breakage from updates in the future.

anguswilliams commented 7 years ago

@calvn no worries, I do have the version pinned, and was running upgrade tests in a staging environment which is where I caught this luckily. Thanks for reviewing so quickly!

calvn commented 7 years ago

@arledesma this PR removes the default arguments from the install, which I think makes sense. Thought I should let you know.

Edit: I think we should use $env:chocolateyPackageParameters to pass in arguments during install. See https://github.com/chocolatey/choco/wiki/How-To-Parse-PackageParameters-Argument.

calvn commented 7 years ago

Related: https://github.com/calvn/chocolatey-packages/pull/8

Maybe we should pull in that PR. I do like the ability to give users the option to change defaults.

anguswilliams commented 7 years ago

Yep, that PR makes more sense.

calvn commented 7 years ago

I am going to revert the extra args changes for now, since that can be set from a configuration file, but not the other way around. I'll see if @123BLiN is willing to do a PR against this repo first.