Replace x/net/websocket with gorilla/websocket. #7013

Merged
merged 2 commits into from Feb 22, 2017

Conversation

Projects
None yet
3 participants
Owner

howbazaar commented Feb 21, 2017

There was a tech-board decision some time in the last two years when we were discussing websocket libraries and why we had two. The decision then was to standardise on gorilla/websocket, but nothing happened.

I'm hoping that using the more standards compliant websocket library will help with some of the leaks we are seeing. Even if they don't help right now, the new library gives the client code access to the websocket ping/pong frames, which the old library didn't. This should allow us to have better recognition when the other end goes away.

QA steps

Deploy an old controller, make sure there are workload agents. Upgrade the controller and look at the logs. All older clients, CLI and agents should continue to work as normal. New juju CLI should work with older controller.

Documentation changes

No user impact at all.

Looks pretty mechanical, can't see anything obvious

+// sendInitialErrorV0 writes out the error as a params.ErrorResult serialized
+// with JSON with a new line character at the end.
+//
+// This is a hangover from the initial debug-log streaming endoing where the
@wallyworld

wallyworld Feb 21, 2017

Owner

Can we add a TODO here

rpc/server.go
@@ -341,13 +341,15 @@ func (conn *Conn) input() {
close(conn.dead)
}
+var errRemoteClosed = errors.New("remote closed connection")
@wallyworld

wallyworld Feb 21, 2017

Owner

appears unused?

Owner

howbazaar commented Feb 22, 2017

$$merge$$

Contributor

jujubot commented Feb 22, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit f06c3e9 into juju:develop Feb 22, 2017

sinzui pushed a commit to sinzui/juju that referenced this pull request Feb 23, 2017

Revert "Merge pull request #7013 from howbazaar/gorilla-websocket"
This reverts commit f06c3e9, reversing
changes made to 3c535e6.

jujubot added a commit that referenced this pull request Feb 23, 2017

Merge pull request #7029 from sinzui/revert-gorilla-sockets
Revert "Merge pull request #7013 from howbazaar/gorilla-websocket"

## Description of change

This reverts commit f06c3e9, reversing
changes made to 3c535e6.

## Please provide the following details to expedite Pull Request review:

The Gorilla websockets change broke restore-backup
The  bootstrap under proxied conditions is also broken,

## QA steps

We expect juju to bootstrap under proxied conditions and restore-backup to work again.

## Documentation changes

None

## Bug reference

https://bugs.launchpad.net/juju/+bug/1666898
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment