Refactor for initial release #5

knisbet · 2019-04-04T15:12:23Z

I realize you haven't really seen the prototype code, so this PR is really the initial commit to a CNI plugin utilizing wireguard. It's built on the initial design doc I shared and updated. It's also been converted to markdown so it can live inside the repo (docs/rfcs/0001-spec.md).

To try and summarize:

The controller uses kubernetes crd objects for exchanging state and keys via the kubernetes API. The controller gets deployed to each node, which watches for these objects, and synchronizes the configuration with wireguard on the host to create a full mesh VPN between each network host and act as a cluster.
The controller also uses a secret object for synchronizing pre-shared-keys for each host-pair. Currently this uses a single object, with last writer wins.
On startup, all wireguard keys are rotated, effectively making the keys used ephemeral, and eliminating a need for long term secrets.
The wireguard kernel module needs to be installed on the host ahead of time, I don't currently know how to load it from a docker container, that supports multiple kernel distros and releases.
I'm currently using the wg cli from within the docker container of the controller to update the wg config. Eventually this should be able to use netlink, it's just updating the go netlink project is more than I want to tackle at the moment.
I'm experimenting with using mage as a build system instead of make. Basically, it allows for writing the build scripting in go, which makes it easier to do certain types of logic. It's an experiment, but let me know what you think.
/pkg/apis - This is the CRD definition, it requires a specific directory layout
/pkg/client - automatically generated code
/pkg/controller - this is the main controller logic
/pkg/iptables - control loop for iptables related configuration
/pkg/wireguard - separate package for controlling wireguard
/hack - standardish kubernetes hack scripts for code generations
/scripts - scripts to embed in the container for interop with gravity (use rigging for upgrde/rollback)

Sorry it's kind of big, but I changed and refactored lots of the internals as I went.

build.go

docs/gravity-wormhole.yaml

r0mant · 2019-04-04T20:15:57Z

pkg/apis/wormhole.gravitational.io/register.go

+limitations under the License.
+*/
+
+package wormholegravitationalio


Is this a common package naming pattern used for Kubernetes 3rd party APIs? Reads weird.

Yes. I believe this is a required format / path structure to match the CRD group name as a path (wormhole.gravitational.io)

pkg/apis/wormhole.gravitational.io/register.go

pkg/apis/wormhole.gravitational.io/v1beta1/wgnode_types.go

pkg/controller/utils.go

r0mant · 2019-04-04T21:08:54Z

pkg/controller/utils.go

+ return trace.Wrap(c.updateNodeNameFromPod(podName, podNamespace))
+ }
+
+ nodeName, err := os.Hostname()


If it's important for the node name to be an actual Kubernetes node name, I think it'll be more reliable to use Nodes API rather than relying on hostname.

Sorry, I'm not sure I follow.

The normal flow is to read the Pod name/namespace from the environment, find the pod object for the controller, and read the node name from the spec.

when not running in a pod though, using the Nodes API, how do I find the Node object that matches the host the process is running on? I think we still need to come up with a name, which in some cases the os.Hostname() covers, and otherwise it needs to be passed as a flag to the process.

pkg/wireguard/utils.go

pkg/wireguard/wireguard.go

a-palchikov · 2019-04-05T08:47:24Z

For me none of the build targets work:

=====> Creating build container...

unable to prepare context: The Dockerfile (/home/deemok/go/src/github.com/gravitational/wormhole/Dockerfile.build) must be within the build context (./assets)
Error: running "docker build --pull --tag wormhole-build:0.0.1-9-g696cc34 --build-arg BUILD_IMAGE=golang:1.12.0 --build-arg GOLANGCI_VER=v1.15.0 -f Dockerfile.build ./assets" failed with exit code 1
exit status 1

is there a specific docker dependency?

Also, I cannot comment on multiple issues since they're not part of the commit.

Dockerfile

README.md

a-palchikov · 2019-04-05T08:53:19Z

cmd/wormhole/controller.go

 }

 ctx, cancel := context.WithCancel(context.Background())
 defer cancel()

- err = controller.Run(ctx, kubeletKubeconfig, wormholeKubeconfig)
+ err = c.Run(ctx)


As far as I understand, Run is blocking - then watching for signals below is not working as expected - something needs to run in a goroutune.

good catch, it originally worked differently and I missed the change. What i've done is move the signal handling to a separate routine, and cancels the context.

cmd/wormhole/controller.go

docs/gravity-wormhole.yaml

pkg/iptables/iptables.go

pkg/wireguard/utils.go

pkg/wireguard/wireguard.go

a-palchikov · 2019-04-05T12:28:56Z

Since I cannot comment on multiple issues which are not part of the commit, I'm going to resort to linking to specific file locations.

https://github.com/gravitational/wormhole/blob/kevin/master/productionalize/cmd/wormhole/controller.go#L149
nit: no need for the buffer > 1 here.

https://github.com/gravitational/wormhole/blob/kevin/master/productionalize/pkg/iptables/iptables.go#L46
there should be a Godoc that mentions that Run starts a new goroutine which will only be cancelled with ctx expiring.

https://github.com/gravitational/wormhole/blob/kevin/master/productionalize/pkg/iptables/iptables.go#L173
I think returning an error also for missing rules will let you get rid of the boolean return.

knisbet · 2019-04-05T15:10:41Z

@a-palchikov Re: the unable to build. It looks like the ability to have the dockerfile outside the build context was added in docker 18.03. I did it this way, just for a small optimization of keeping the build context smaller and not having to send built binaries, etc.

docker/cli#886

a-palchikov · 2019-04-08T14:19:52Z

pkg/wireguard/wireguard.go

+ BytesTX int64
+ // BytesRX is the number of bytes received from the peer
+ BytesRX int64
+ //Keepalive is the number of second to keep the connection alive


Kevin Nisbet added 8 commits March 12, 2019 14:53

checking WIP code

cee6a0a

unit tests + overhaul

0340f55

lint fixes

4ab1c9f

rename node to wgnode

0aa9a48

pre-testing check in

672d3a7

refactor and cleanup

2ecb981

delete before adding wg interface

9f60bfd

dep ensure + makefile update

70fb4f7

knisbet requested review from r0mant and a-palchikov April 4, 2019 15:12

update readme

696cc34

r0mant reviewed Apr 4, 2019

View reviewed changes

a-palchikov reviewed Apr 5, 2019

View reviewed changes

Kevin Nisbet added 3 commits April 5, 2019 12:51

address review feedback

d6f6c1c

make bridge MTU configurable

41f58d5

apply retry logic to updatePeerSecrets

3317f38

a-palchikov approved these changes Apr 8, 2019

View reviewed changes

knisbet merged commit 4daacde into master Apr 22, 2019

knisbet deleted the kevin/master/productionalize branch April 22, 2019 15:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor for initial release #5

Refactor for initial release #5

knisbet commented Apr 4, 2019

r0mant Apr 4, 2019

knisbet Apr 5, 2019

r0mant Apr 4, 2019

knisbet Apr 5, 2019

a-palchikov commented Apr 5, 2019

a-palchikov Apr 5, 2019

knisbet Apr 5, 2019

a-palchikov commented Apr 5, 2019

knisbet commented Apr 5, 2019

a-palchikov Apr 8, 2019

Refactor for initial release #5

Refactor for initial release #5

Conversation

knisbet commented Apr 4, 2019

r0mant Apr 4, 2019

Choose a reason for hiding this comment

knisbet Apr 5, 2019

Choose a reason for hiding this comment

r0mant Apr 4, 2019

Choose a reason for hiding this comment

knisbet Apr 5, 2019

Choose a reason for hiding this comment

a-palchikov commented Apr 5, 2019

a-palchikov Apr 5, 2019

Choose a reason for hiding this comment

knisbet Apr 5, 2019

Choose a reason for hiding this comment

a-palchikov commented Apr 5, 2019

knisbet commented Apr 5, 2019

a-palchikov Apr 8, 2019

Choose a reason for hiding this comment