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

[proxy] use a container-local unix socket to get the address, instead of logs #1474

Merged
merged 2 commits into from
Sep 29, 2015
Merged

[proxy] use a container-local unix socket to get the address, instead of logs #1474

merged 2 commits into from
Sep 29, 2015

Conversation

paulbellamy
Copy link
Contributor

Fixes #1453 and, #1472

rade
rade reviewed Sep 27, 2015
View reviewed changes
weave
return 1
fi
fractional_sleep 3.1
done

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@rade rade assigned paulbellamy and unassigned rade Sep 27, 2015
rade
rade reviewed Sep 28, 2015
View reviewed changes
weave
shift 4
output=$(docker exec $container curl -s -S -X $http_verb --unix-socket $socket "$@" http:$url)
[[ -n "$output" ]] || return 1
echo $output

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@paulbellamy paulbellamy assigned rade and unassigned paulbellamy Sep 28, 2015
@rade rade assigned paulbellamy and unassigned rade Sep 28, 2015
@rade
Copy link
Member

rade commented Sep 28, 2015

The CircleCI tests are timing out.

Please review my refactor commit. The only behavioural change is the retry reduction. I think that is correct; the 3s connect timeout in the normal status case doesn't usually kick in; if there is nothing listening then curl instantly fails.

@paulbellamy
Copy link
Contributor Author

The retry reduction means we're doing 10 docker execs/second until the proxy starts. that seems a bit excessive to me.

@rade
Copy link
Member

rade commented Sep 28, 2015

It's not excessive and exactly what happens with the router. We don't a delay of several seconds in weave launch. There was an issue/PR to address precisely that.

@paulbellamy
Copy link
Contributor Author

With the router we're just doing an http request, not a docker exec, though.

@rade
Copy link
Member

rade commented Sep 28, 2015

meh

@paulbellamy paulbellamy assigned rade and unassigned paulbellamy Sep 28, 2015
@paulbellamy
Copy link
Contributor Author

I had accidentally used [[ which broke the --local test.

rade added a commit that referenced this pull request Sep 29, 2015
@rade
Merge pull request #1474 from weaveworks/1472-weave-env-debug
5b1d0c5 
use a container-local unix socket to get the address, instead of logs

Fixes #1453. Fixes #1472.
@rade rade merged commit 5b1d0c5 into master Sep 29, 2015
@paulbellamy paulbellamy deleted the 1472-weave-env-debug branch September 29, 2015 10:09
@rade rade added this to the 1.2.0 milestone Oct 1, 2015
@rade rade mentioned this pull request Oct 9, 2015
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@rade rade

Labels
None yet
Projects
None yet
Milestone
1.2.0
Development

Successfully merging this pull request may close these issues.
2 participants
@paulbellamy @rade