Skip to content
This repository was archived by the owner on Jun 20, 2024. It is now read-only.

[proxy] weavewait rewrites /etc/hosts so weave ip is returned first from 'hostname -i' #1079

Merged
merged 2 commits into from
Jul 13, 2015

Conversation

paulbellamy
Copy link
Contributor

Partial fix for #68. Won't help with dynamic attach/detach or the weave script.

@rade
Copy link
Member

rade commented Jul 6, 2015

I think this should close #68 and we should open a new issue (or issues) covering the remaining cases.

@bboreham would you mind reviewing this?

@rade rade assigned bboreham and unassigned rade Jul 6, 2015
@paulbellamy
Copy link
Contributor Author

I've tried this with Java's InetAddress.getLocalHost(), and it returns the weave address.

$ weave launch
$ docker $(weave proxy-config) run -ti --rm my-java-app
2c80b6ca84b7/10.128.0.2
$ docker run -ti --rm my-java-app
fc551c2b55a7/172.17.0.5

with code:

import java.net.InetAddress;

public class Main {
  public static void main(String[] args) {
    try {
      System.out.println(InetAddress.getLocalHost());
    } catch (java.net.UnknownHostException e) {
      System.out.println(e);
    }
  }
}

checkErr(err)
var oldHosts, newHosts bytes.Buffer
for _, addr := range addrs {
fmt.Fprint(&newHosts, strings.Split(addr.String(), "/")[0], "\t", hostname, "\n")

This comment was marked as abuse.

This comment was marked as abuse.

@paulbellamy paulbellamy force-pushed the 68-rewrite-etc-hosts branch from 93e934a to 8652ba2 Compare July 6, 2015 13:40
@@ -19,4 +19,7 @@ COMMITTED_IMAGE=$(proxy docker_on $HOST1 commit c1)
assert_raises "proxy docker_on $HOST1 run --name c2 $COMMITTED_IMAGE"
assert "entrypoint c2" "$(entrypoint $COMMITTED_IMAGE)"

# Check weave IP is first ip returned, so java (etc) prefer weave.
assert "proxy docker_on $HOST1 run -e 'WEAVE_CIDR=10.2.1.1/24' $BASE_IMAGE hostname -i | cut -d' ' -f1" "10.2.1.1"

This comment was marked as abuse.

This comment was marked as abuse.

@bboreham bboreham assigned paulbellamy and unassigned bboreham Jul 6, 2015
@paulbellamy
Copy link
Contributor Author

As @bboreham points out above, afaict hostname -i makes no guarantees about order. Nor does Java.

If we want to add the ip hostname into /etc/hosts we could use docker's --add-hosts, but as hostname -I already returns the weave IP, I don't see the point of that.

@paulbellamy paulbellamy force-pushed the 68-rewrite-etc-hosts branch from 8652ba2 to 0e74e46 Compare July 6, 2015 15:14
@rade
Copy link
Member

rade commented Jul 6, 2015

--add-host was considered in #68. It is of no use. The whole point of #68 is make hostname -i return the weave address. first.

We could remove the host entry for the docker ip.

@paulbellamy
Copy link
Contributor Author

Yeah. removing the docker ip seems a bit naughty..

@rade
Copy link
Member

rade commented Jul 6, 2015

Yeah. removing the docker ip seems a bit naughty.

Meh. The docker ip should still show up with hostname -I. Provided that both inbound and outbound docker links still work that is fine, I reckon. And we can add a flag for disabling that behaviour.

@paulbellamy
Copy link
Contributor Author

Tried out links with the docker IP removed from /etc/hosts, and they still work fine. Not a big fan of adding yet another flag, but...

@paulbellamy paulbellamy force-pushed the 68-rewrite-etc-hosts branch from eb8ed82 to ea52cfd Compare July 7, 2015 10:41
@paulbellamy paulbellamy assigned rade and unassigned paulbellamy Jul 7, 2015
@rade rade removed their assignment Jul 7, 2015
@paulbellamy paulbellamy force-pushed the 68-rewrite-etc-hosts branch from ea52cfd to 386e358 Compare July 7, 2015 14:50
args = os.Args[1:]
notInExec = true
rewriteHosts = true
)

if args[0] == "-s" {

This comment was marked as abuse.

This comment was marked as abuse.

@paulbellamy paulbellamy force-pushed the 68-rewrite-etc-hosts branch 2 times, most recently from ce441c9 to e445837 Compare July 10, 2015 12:57
…rom 'hostname -i'

Adds a --no-rewrite-hosts flag to the proxy, and be default, removes the
docker IP from hosts. We only discard the docker host when it exists, so
--net=none works.
writeHosts(hosts)
}

func parseHosts() map[string][]string {

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

@tomwilkie
Copy link
Contributor

LGTM


fields := strings.Fields(line)
if len(fields) > 0 {
ips[fields[0]] = fields[1:]

This comment was marked as abuse.

@bboreham bboreham assigned paulbellamy and unassigned bboreham Jul 10, 2015
@paulbellamy paulbellamy assigned bboreham and unassigned paulbellamy Jul 10, 2015
bboreham added a commit that referenced this pull request Jul 13, 2015
@bboreham bboreham merged commit 5e43f0f into master Jul 13, 2015
@paulbellamy paulbellamy deleted the 68-rewrite-etc-hosts branch July 13, 2015 10:19
@rade rade modified the milestones: current, 1.1.0 Jul 13, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants