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

Move container attachment from sh to Go #2307

Merged
merged 14 commits into from
May 26, 2016
Merged

Move container attachment from sh to Go #2307

merged 14 commits into from
May 26, 2016

Conversation

bboreham
Copy link
Contributor

@bboreham bboreham commented May 23, 2016 •
edited
Loading

Part of #1940

Most of this is refactoring/enhancing the current plugin code to be closer to the script; see also #639 for arping and #1726 for ethtool tx off.

The plugin previously leaked a veth if something went wrong configuring it.

This PR does not mean the proxy can avoid shelling out when attaching a container. Still to do: allocate IP addresses, rewrite hosts file, add to DNS.

@bboreham bboreham added this to the 1.6.0 milestone May 23, 2016
@bboreham bboreham self-assigned this May 23, 2016
@bboreham bboreham force-pushed the issues/1940-remove-proxy-attach-dep branch 2 times, most recently from b2a3401 to 65d9873 Compare May 23, 2016 13:57
@bboreham bboreham changed the title WIP: Move container attachment from sh to Go Move container attachment from sh to Go May 23, 2016
@bboreham bboreham removed their assignment May 23, 2016
@brb brb self-assigned this May 23, 2016
brb
brb reviewed May 23, 2016
View reviewed changes
.gitmodules
@@ -70,3 +70,9 @@
[submodule "vendor/github.com/pkg/profile"]
path = vendor/github.com/pkg/profile
url = https://github.com/pkg/profile
[submodule "vendor/github.com/bboreham/arping"]

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@bboreham bboreham force-pushed the issues/1940-remove-proxy-attach-dep branch from 65d9873 to 1be4bac Compare May 23, 2016 14:38
brb
brb reviewed May 23, 2016
View reviewed changes
common/ethtool.go

copy(request.Name[:], name)

socket, err := syscall.Socket(syscall.AF_INET, syscall.SOCK_DGRAM, 0)

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@bboreham bboreham force-pushed the issues/1940-remove-proxy-attach-dep branch from 4b5e78d to dbb53a2 Compare May 23, 2016 17:46
brb
brb reviewed May 24, 2016
View reviewed changes
prog/weaveutil/attach.go
}

func findPid(containerID string) (int, error) {
c, err := docker.NewVersionedClientFromEnv("1.18")

This comment was marked as abuse.

Sign in to view

@bboreham bboreham force-pushed the issues/1940-remove-proxy-attach-dep branch from 09bceba to 180fa24 Compare May 24, 2016 10:24
brb
brb reviewed May 24, 2016
View reviewed changes
net/veth.go
if err := common.WithNetNS(ns, func() error {
_, err := netlink.LinkByName(ifName)
return err
}); err != nil {

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

brb
brb reviewed May 24, 2016
View reviewed changes
net/veth.go
}
name, peerName := vethPrefix+"pl"+id, vethPrefix+"pg"+id
_, err = CreateAndAttachVeth(name, peerName, bridgeName, mtu, func(veth netlink.Link) error {
common.EthtoolTXOff(peerName) // TODO: do we want to do this under fastdp?

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

@bboreham bboreham force-pushed the issues/1940-remove-proxy-attach-dep branch 3 times, most recently from ae81b08 to 92025bb Compare May 25, 2016 15:18
bboreham and others added 14 commits May 25, 2016 17:17
@bboreham
Extract veth creation and LinkSetUp to net package
f11d91b
@bboreham
Remove veth on error
d59c170
@bboreham
Slight cleanup
c458dea
@bboreham
Obtain IP address before setting up veth
9f8b946
@bboreham
initialize via sub-function
9aaa7f1
@bboreham
Extract basic veth setup
0257274
@bboreham
Extract AttachContainer() function
85023e0 
Also move netlink and netns code from common to net

Creates interface if not already there
Adds addresses
Move the set link 'up' to after addresses are added
@bboreham
Extract AddRoute function
4fc2835
@awh @bboreham
Utility function to disable tx checksum offload
e3ef473
@bboreham
Move attach() and detach() from shell script to Go
ac14b19
@bboreham
Move error detection for attach() into Go
161446d
@bboreham
Use 'peer' and 'veth' instead of 'guest' and 'local', after review co…
8717fd1 
…mments
@bboreham
Improve veth name construction; use a constant instead of "ethwe"
2702b85
@bboreham
Only do ethtool tx off in bridged (pcap) mode.
6dd109a 
Detect the bridge type in the same way the weave script does.
@bboreham bboreham force-pushed the issues/1940-remove-proxy-attach-dep branch from 92025bb to 6dd109a Compare May 25, 2016 16:35
brb
brb reviewed May 25, 2016
View reviewed changes
net/veth.go
return cleanup(`failed to attach %s to device "%s": %s`, name, bridgeName, err)
}
default:
return cleanup(`invalid bridge configuration`)

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@brb brb merged commit f0109c1 into master May 26, 2016
This was referenced May 26, 2016
Open
Closed
@hellt hellt mentioned this pull request Sep 27, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@brb brb

Labels
None yet
Projects
None yet
Milestone
1.6.0
Development

Successfully merging this pull request may close these issues.
3 participants
@bboreham @brb @awh