Skip to content

Only find MAC address if no command-line flag value given#638

Merged
stuartnelson3 merged 3 commits intomasterfrom
delay-check-for-MAC
Feb 28, 2017
Merged

Only find MAC address if no command-line flag value given#638
stuartnelson3 merged 3 commits intomasterfrom
delay-check-for-MAC

Conversation

@stuartnelson3
Copy link
Contributor

Defaulting to the machine's MAC address fails
sometimes fails and causes a panic. Allow the user to
specify custom address to skip this so they can
run AlertManager.

Addresses #582

@beorn7 @brancz @fabxc this is more or less a quick fix so that users can manually enter something while we decide how to handle this.

Defaulting to the machine's MAC address fails
sometimes fails and causes a panic. Allow the user
to specify custom address to skip this so they can
run AlertManager.
Copy link
Member

@brancz brancz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a small typo and my thought on -mesh.hardware-address, but that would be a breaking change so we should think about it a bit more

README.md Outdated
- `-mesh.nickname` string: peer nickname (default "<machine-hostname>")
- `-mesh.peer` value: initial peers (may be repeated)
- `-mesh.nickname` string: mesh peer nickname (default "<machine-hostname>")
- `-mesh.peer` value: initial initial peers (may be repeated)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initial initial

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woops!

README.md Outdated
`-mesh.*` flags.

- `-mesh.hardware-address` string: MAC address, i.e. mesh peer ID (default "<hardware-mac-address>")
- `-mesh.hardware-address` string: mesh peer ID (default "<hardware-mac-address>")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've thought about this before and was thinking that we should probably change this to -mesh.peer-id and have it default to the mac address, but not call it hardware address.

@stuartnelson3
Copy link
Contributor Author

Sounds good. I can leave this here and we can discuss the breaking change, and if we want to then add an alternative to the mac address as a default.

@beorn7
Copy link
Member

beorn7 commented Feb 27, 2017

If we are reluctant to have a breaking change now, let's just merge this with the functional change but without the flag change.

But perhaps we are not worried about breaking changes at this pre-1.0 point?

@stuartnelson3
Copy link
Contributor Author

stuartnelson3 commented Feb 27, 2017

I think it's a valid point that this is experimental pre-1.0. We aren't cutting a release, so anyone using this would be compiling their own version.

@brancz
Copy link
Member

brancz commented Feb 27, 2017

It's not that I'm reluctant to the breaking change, I just want to agree on it first 🙂

@brian-brazil
Copy link
Contributor

I think a breaking change is okay, this feature is marked experimental and I can't imagine too many people setting this currently.

On the naming, this is very close to -mesh.peer which I can imagine causing confusion. Maybe something like unique-peer-id or my-peer-id?

@fabxc
Copy link
Contributor

fabxc commented Feb 28, 2017

I think mesh.peer-id is just fine. So is the breaking change here in my opinion.

@stuartnelson3
Copy link
Contributor Author

The naming is similar, but I say since it's experimental, we can change if we get user complaints. I'll figure out why the tests are failing and wait for a thumb from someone.

@brancz
Copy link
Member

brancz commented Feb 28, 2017

lgtmog

@stuartnelson3 stuartnelson3 merged commit 24a9a64 into master Feb 28, 2017
@stuartnelson3 stuartnelson3 deleted the delay-check-for-MAC branch February 28, 2017 13:57
hh pushed a commit to ii/alertmanager that referenced this pull request Aug 17, 2017
* Add dockerfile for ppc64le and related changes

* Pass the fill file as DOCKEFILE

* Add the dockerfile name to build msg
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants