-
Notifications
You must be signed in to change notification settings - Fork 24
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
Implement auto-config #48
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added 2 comments. PTAL. Other than that, it looks good.
} | ||
|
||
private static boolean azureDnsServerConfigured() { | ||
return readFileContents("/etc/resolv.conf").contains("168.63.129.16"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried today creating two random ubuntu instances on Azure and unfortunately, they didn't have 168.63.129.16
in /etc/resolv.conf
:( I wonder, maybe we should change 168.63.129.16
to bx.internal.cloudapp.net
. What do you think? Could you make some more checks on that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is weird, I always had them even with different linux distros. Can you tell me the specifications of the OS and which region you deployed it? I will try to recreate the same. I will check the VMs with bx.internal.cloudapp.net
but right now I am unable to find one that doesn't have it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, actually I remember checking last week and all my instances had 168.63.129.16
. But today I did exactly the same, created a default ubuntu instance, in default region (US East) with all default parameters and 168.63.129.16
was not there. It makes me think that maybe this bx.internal.cloudapp.net
can be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see right now I checked one of the VMs and its content is:
# Dynamic resolv.conf(5) file for glibc resolver(3) generated by resolvconf(8)
# DO NOT EDIT THIS FILE BY HAND -- YOUR CHANGES WILL BE OVERWRITTEN
nameserver 168.63.129.16
search dhmodtvjlweejkiq3y3w53ku4a.gx.internal.cloudapp.net
Maybe *.internal.cloudapp.net
is a better choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, good idea 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if /etc/resolv.conf
does not exist at all? think of Windows, etc.
Also I assume we should touch filesystem under the j.s.PrivilegedAction
wrapper otherwise it will blow-up when Java Security is enabled? @kwart knows details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non-existing /etc/resolv.conf
is not an issue, because if anything non-expected happens in the auto-detection we just log it in FINE level and this strategy is just not used.
About Java Security, I don't know, I guess it should be similar. But @kwart can comment on that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we don't support the Java Security Manager in Hazelcast yet, we don't need to wrap the calls.
Ref: https://hazelcast.atlassian.net/wiki/spaces/PM/pages/398753822/
…y.java Co-authored-by: Rafał Leszko <[email protected]>
We used to check a name server ip "168.63.129.16", but it seems this is not present in every Azure VM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Good work @mtyazici
No description provided.