Skip to content

Commit b9a2193

Browse files
ofilizofiliz
committed
Improve cleanup on network failure paths
1 parent 879d9e0 commit b9a2193

File tree

4 files changed

+80
-72
lines changed

4 files changed

+80
-72
lines changed

‎network/endpoint.go‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -45,44 +45,51 @@ func (nw *network) newEndpoint(epInfo *EndpointInfo) (*endpoint, error) {
4545
var err error
4646

4747
log.Printf("[net] Creating endpoint %+v in network %v.", epInfo, nw.Id)
48+
defer func() {
49+
if err != nil {
50+
log.Printf("[net] Failed to create endpoint %v, err:%v.", epInfo.Id, err)
51+
}
52+
}()
4853

4954
if nw.Endpoints[epInfo.Id] != nil {
5055
err = errEndpointExists
51-
goto fail
56+
return nil, err
5257
}
5358

5459
// Call the platform implementation.
5560
ep, err = nw.newEndpointImpl(epInfo)
5661
if err != nil {
57-
goto fail
62+
return nil, err
5863
}
5964

6065
nw.Endpoints[epInfo.Id] = ep
6166

6267
log.Printf("[net] Created endpoint %+v.", ep)
6368

6469
return ep, nil
65-
66-
fail:
67-
log.Printf("[net] Creating endpoint %v failed, err:%v.", epInfo.Id, err)
68-
69-
return nil, err
7070
}
7171

7272
// DeleteEndpoint deletes an existing endpoint from the network.
7373
func (nw *network) deleteEndpoint(endpointId string) error {
74+
var err error
75+
7476
log.Printf("[net] Deleting endpoint %v from network %v.", endpointId, nw.Id)
77+
defer func() {
78+
if err != nil {
79+
log.Printf("[net] Failed to delete endpoint %v, err:%v.", endpointId, err)
80+
}
81+
}()
7582

7683
// Look up the endpoint.
7784
ep, err := nw.getEndpoint(endpointId)
7885
if err != nil {
79-
goto fail
86+
return err
8087
}
8188

8289
// Call the platform implementation.
8390
err = nw.deleteEndpointImpl(ep)
8491
if err != nil {
85-
goto fail
92+
return err
8693
}
8794

8895
// Remove the endpoint object.
@@ -91,11 +98,6 @@ func (nw *network) deleteEndpoint(endpointId string) error {
9198
log.Printf("[net] Deleted endpoint %+v.", ep)
9299

93100
return nil
94-
95-
fail:
96-
log.Printf("[net] Deleting endpoint %v failed, err:%v.", endpointId, err)
97-
98-
return err
99101
}
100102

101103
// GetEndpoint returns the endpoint with the given ID.

‎network/endpoint_linux.go‎

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,13 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
5252
return nil, err
5353
}
5454

55+
// On failure, delete the veth pair.
56+
defer func() {
57+
if err != nil {
58+
netlink.DeleteLink(contIfName)
59+
}
60+
}()
61+
5562
//
5663
// Host network interface setup.
5764
//
@@ -60,14 +67,14 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
6067
log.Printf("[net] Setting link %v state up.", hostIfName)
6168
err = netlink.SetLinkState(hostIfName, true)
6269
if err != nil {
63-
goto cleanup
70+
return nil, err
6471
}
6572

6673
// Connect host interface to the bridge.
6774
log.Printf("[net] Setting link %v master %v.", hostIfName, nw.extIf.BridgeName)
6875
err = netlink.SetLinkMaster(hostIfName, nw.extIf.BridgeName)
6976
if err != nil {
70-
goto cleanup
77+
return nil, err
7178
}
7279

7380
//
@@ -77,7 +84,7 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
7784
// Query container network interface info.
7885
containerIf, err = net.InterfaceByName(contIfName)
7986
if err != nil {
80-
goto cleanup
87+
return nil, err
8188
}
8289

8390
// Setup rules for IP addresses on the container interface.
@@ -86,14 +93,14 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
8693
log.Printf("[net] Adding ARP reply rule for IP address %v on %v.", ipAddr.String(), contIfName)
8794
err = ebtables.SetArpReply(ipAddr.IP, nw.getArpReplyAddress(containerIf.HardwareAddr), ebtables.Append)
8895
if err != nil {
89-
goto cleanup
96+
return nil, err
9097
}
9198

9299
// Add MAC address translation rule.
93100
log.Printf("[net] Adding MAC DNAT rule for IP address %v on %v.", ipAddr.String(), contIfName)
94101
err = ebtables.SetDnatForIPAddress(nw.extIf.Name, ipAddr.IP, containerIf.HardwareAddr, ebtables.Append)
95102
if err != nil {
96-
goto cleanup
103+
return nil, err
97104
}
98105
}
99106

@@ -103,23 +110,32 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
103110
log.Printf("[net] Opening netns %v.", epInfo.NetNsPath)
104111
ns, err = OpenNamespace(epInfo.NetNsPath)
105112
if err != nil {
106-
goto cleanup
113+
return nil, err
107114
}
108115
defer ns.Close()
109116

110117
// Move the container interface to container's network namespace.
111118
log.Printf("[net] Setting link %v netns %v.", contIfName, epInfo.NetNsPath)
112119
err = netlink.SetLinkNetNs(contIfName, ns.GetFd())
113120
if err != nil {
114-
goto cleanup
121+
return nil, err
115122
}
116123

117124
// Enter the container network namespace.
118125
log.Printf("[net] Entering netns %v.", epInfo.NetNsPath)
119126
err = ns.Enter()
120127
if err != nil {
121-
goto cleanup
128+
return nil, err
122129
}
130+
131+
// Return to host network namespace.
132+
defer func() {
133+
log.Printf("[net] Exiting netns %v.", epInfo.NetNsPath)
134+
err = ns.Exit()
135+
if err != nil {
136+
log.Printf("[net] Failed to exit netns, err:%v.", err)
137+
}
138+
}()
123139
}
124140

125141
// If a name for the container interface is specified...
@@ -128,22 +144,22 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
128144
log.Printf("[net] Setting link %v state down.", contIfName)
129145
err = netlink.SetLinkState(contIfName, false)
130146
if err != nil {
131-
goto cleanup
147+
return nil, err
132148
}
133149

134150
// Rename the container interface.
135151
log.Printf("[net] Setting link %v name %v.", contIfName, epInfo.IfName)
136152
err = netlink.SetLinkName(contIfName, epInfo.IfName)
137153
if err != nil {
138-
goto cleanup
154+
return nil, err
139155
}
140156
contIfName = epInfo.IfName
141157

142158
// Bring the interface back up.
143159
log.Printf("[net] Setting link %v state up.", contIfName)
144160
err = netlink.SetLinkState(contIfName, true)
145161
if err != nil {
146-
goto cleanup
162+
return nil, err
147163
}
148164
}
149165

@@ -152,7 +168,7 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
152168
log.Printf("[net] Adding IP address %v to link %v.", ipAddr.String(), contIfName)
153169
err = netlink.AddIpAddress(contIfName, ipAddr.IP, &ipAddr)
154170
if err != nil {
155-
goto cleanup
171+
return nil, err
156172
}
157173
}
158174

@@ -169,17 +185,7 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
169185

170186
err = netlink.AddIpRoute(nlRoute)
171187
if err != nil {
172-
goto cleanup
173-
}
174-
}
175-
176-
// If inside the container network namespace...
177-
if ns != nil {
178-
// Return to host network namespace.
179-
log.Printf("[net] Exiting netns %v.", epInfo.NetNsPath)
180-
err = ns.Exit()
181-
if err != nil {
182-
goto cleanup
188+
return nil, err
183189
}
184190
}
185191

@@ -194,12 +200,6 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
194200
}
195201

196202
return ep, nil
197-
198-
cleanup:
199-
// Roll back the changes for the endpoint.
200-
netlink.DeleteLink(contIfName)
201-
202-
return nil, err
203203
}
204204

205205
// deleteEndpointImpl deletes an existing endpoint from the network.

‎network/network.go‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,11 @@ func (nm *networkManager) newNetwork(nwInfo *NetworkInfo) (*network, error) {
121121
var err error
122122

123123
log.Printf("[net] Creating network %+v.", nwInfo)
124+
defer func() {
125+
if err != nil {
126+
log.Printf("[net] Failed to create network %v, err:%v.", nwInfo.Id, err)
127+
}
128+
}()
124129

125130
// Set defaults.
126131
if nwInfo.Mode == "" {
@@ -131,19 +136,19 @@ func (nm *networkManager) newNetwork(nwInfo *NetworkInfo) (*network, error) {
131136
extIf := nm.findExternalInterfaceBySubnet(nwInfo.Subnets[0].Prefix.String())
132137
if extIf == nil {
133138
err = errSubnetNotFound
134-
goto fail
139+
return nil, err
135140
}
136141

137142
// Make sure this network does not already exist.
138143
if extIf.Networks[nwInfo.Id] != nil {
139144
err = errNetworkExists
140-
goto fail
145+
return nil, err
141146
}
142147

143148
// Call the OS-specific implementation.
144149
nw, err = nm.newNetworkImpl(nwInfo, extIf)
145150
if err != nil {
146-
goto fail
151+
return nil, err
147152
}
148153

149154
// Add the network object.
@@ -152,37 +157,36 @@ func (nm *networkManager) newNetwork(nwInfo *NetworkInfo) (*network, error) {
152157

153158
log.Printf("[net] Created network %v on interface %v.", nwInfo.Id, extIf.Name)
154159
return nw, nil
155-
156-
fail:
157-
log.Printf("[net] Failed to create network %v, err:%v.", nwInfo.Id, err)
158-
return nil, err
159160
}
160161

161162
// DeleteNetwork deletes an existing container network.
162163
func (nm *networkManager) deleteNetwork(networkId string) error {
164+
var err error
165+
163166
log.Printf("[net] Deleting network %v.", networkId)
167+
defer func() {
168+
if err != nil {
169+
log.Printf("[net] Failed to delete network %v, err:%v.", networkId, err)
170+
}
171+
}()
164172

165173
// Find the network.
166174
nw, err := nm.getNetwork(networkId)
167175
if err != nil {
168-
goto fail
176+
return err
169177
}
170178

171179
// Call the OS-specific implementation.
172180
err = nm.deleteNetworkImpl(nw)
173181
if err != nil {
174-
goto fail
182+
return err
175183
}
176184

177185
// Remove the network object.
178186
delete(nw.extIf.Networks, networkId)
179187

180188
log.Printf("[net] Deleted network %+v.", nw)
181189
return nil
182-
183-
fail:
184-
log.Printf("[net] Failed to delete network %v, err:%v.", networkId, err)
185-
return err
186190
}
187191

188192
// GetNetwork returns the network with the given ID.

0 commit comments

Comments
 (0)