Skip to content

Commit 5fd2e3e

Browse files
authored
feat: Implementation of enable_icc option (#69)
* feat: implementation of enable_icc option + code refactor * address review comments and add support for iptables v6 * add cleanup method if create fails during post process * Add a per-network mutex lock to make Create and Remove calls thread safe * Also track active-reqs to prevent multiple lock creation for the same * Add additional unit-test for HandleCreateOptions Signed-off-by: Swagat Bora <[email protected]> --------- Signed-off-by: Swagat Bora <[email protected]>
1 parent 818fdd6 commit 5fd2e3e

File tree

16 files changed

+1292
-160
lines changed

16 files changed

+1292
-160
lines changed

api/types/network_types.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,6 @@ type NetworkCreateRequest struct {
156156
IPAM IPAM `json:"IPAM"`
157157

158158
// EnableIPv6 specifies to enable IPv6 on the network.
159-
//
160-
// IPv6 networks are not currently supported.
161159
EnableIPv6 bool `json:"EnableIPv6"`
162160

163161
// Options specifies network specific options to be used by the drivers.

go.mod

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
module github.com/runfinch/finch-daemon
22

3-
go 1.22
3+
go 1.22.0
4+
5+
toolchain go1.22.7
46

57
require (
68
github.com/containerd/cgroups/v3 v3.0.3
@@ -13,6 +15,7 @@ require (
1315
github.com/containerd/platforms v0.2.1
1416
github.com/containerd/typeurl/v2 v2.2.0
1517
github.com/containernetworking/cni v1.2.2
18+
github.com/coreos/go-iptables v0.7.0
1619
github.com/coreos/go-systemd/v22 v22.5.0
1720
github.com/distribution/reference v0.6.0
1821
github.com/docker/cli v26.0.0+incompatible
@@ -65,14 +68,14 @@ require (
6568
github.com/containerd/typeurl v1.0.3-0.20220422153119-7f6e6d160d67 // indirect
6669
github.com/containernetworking/plugins v1.4.1 // indirect
6770
github.com/containers/ocicrypt v1.1.10 // indirect
68-
github.com/coreos/go-iptables v0.7.0 // indirect
69-
github.com/cyphar/filepath-securejoin v0.2.4 // indirect
70-
github.com/davecgh/go-spew v1.1.1 // indirect
71+
github.com/cyphar/filepath-securejoin v0.3.1 // indirect
72+
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
7173
github.com/djherbis/times v1.6.0 // indirect
7274
github.com/docker/docker-credential-helpers v0.8.1 // indirect
7375
github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c // indirect
7476
github.com/docker/go-units v0.5.0 // indirect
7577
github.com/fahedouch/go-logrotate v0.2.0 // indirect
78+
github.com/fatih/color v1.17.0 // indirect
7679
github.com/felixge/httpsnoop v1.0.4 // indirect
7780
github.com/fluent/fluent-logger-golang v1.9.0 // indirect
7881
github.com/getlantern/mockconn v0.0.0-20200818071412-cb30d065a848 // indirect

go.sum

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,12 @@ github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSV
7878
github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
7979
github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY=
8080
github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
81-
github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg=
82-
github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4=
81+
github.com/cyphar/filepath-securejoin v0.3.1 h1:1V7cHiaW+C+39wEfpH6XlLBQo3j/PciWFrgfCLS8XrE=
82+
github.com/cyphar/filepath-securejoin v0.3.1/go.mod h1:F7i41x/9cBF7lzCrVsYs9fuzwRZm4NQsGTBdpp6mETc=
8383
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
84-
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
8584
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
85+
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=
86+
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
8687
github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk=
8788
github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E=
8889
github.com/djherbis/times v1.6.0 h1:w2ctJ92J8fBvWPxugmXIv7Nz7Q3iDMKNx9v5ocVH20c=
@@ -105,8 +106,8 @@ github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1m
105106
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
106107
github.com/fahedouch/go-logrotate v0.2.0 h1:UR9Fv8MDVfWwnkirmFHck+tRSWzqOwRjVRLMpQgSxaI=
107108
github.com/fahedouch/go-logrotate v0.2.0/go.mod h1:1RL/yr7LntS4zadAC6FT6yB/C1CQt3V6eHAZzymfwzE=
108-
github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM=
109-
github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE=
109+
github.com/fatih/color v1.17.0 h1:GlRw1BRJxkpqUCBKzKOw098ed57fEsKeNjpTe3cSjK4=
110+
github.com/fatih/color v1.17.0/go.mod h1:YZ7TlrGPkiz6ku9fK3TLD/pl3CpsiFyu8N92HLgmosI=
110111
github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg=
111112
github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
112113
github.com/fluent/fluent-logger-golang v1.9.0 h1:zUdY44CHX2oIUc7VTNZc+4m+ORuO/mldQDA7czhWXEg=

internal/service/network/create.go

Lines changed: 68 additions & 134 deletions
Original file line numberDiff line numberDiff line change
@@ -5,65 +5,42 @@ package network
55

66
import (
77
"context"
8-
"encoding/json"
98
"fmt"
10-
"os"
11-
"path/filepath"
129
"strings"
1310

14-
"github.com/containerd/nerdctl/pkg/lockutil"
1511
"github.com/containerd/nerdctl/pkg/netutil"
1612

1713
"github.com/runfinch/finch-daemon/api/types"
14+
"github.com/runfinch/finch-daemon/internal/service/network/driver"
1815
"github.com/runfinch/finch-daemon/pkg/errdefs"
1916
"github.com/runfinch/finch-daemon/pkg/utility/maputility"
2017
)
2118

2219
// Create implements the logic to turn a network create request to the back-end nerdctl create network calls.
2320
func (s *service) Create(ctx context.Context, request types.NetworkCreateRequest) (types.NetworkCreateResponse, error) {
24-
// enable_ip_masquerade, host_binding_ipv4, and bridge name network options are not supported by nerdctl.
25-
// So we must filter out any unsupported options which would prevent the network from being created and accept the defaults.
26-
bridge := ""
27-
filterUnsupportedOptions := func(original map[string]string) map[string]string {
28-
options := map[string]string{}
29-
for k, v := range original {
30-
switch k {
31-
case "com.docker.network.bridge.enable_ip_masquerade":
32-
// must be true
33-
if v != "true" {
34-
s.logger.Warnf("network option com.docker.network.bridge.enable_ip_masquerade is set to %s, but it must be true", v)
35-
}
36-
case "com.docker.network.bridge.host_binding_ipv4":
37-
// must be 0.0.0.0
38-
if v != "0.0.0.0" {
39-
s.logger.Warnf("network option com.docker.network.bridge.host_binding_ipv4 is set to %s, but it must be 0.0.0.0", v)
40-
}
41-
case "com.docker.network.bridge.enable_icc":
42-
s.logger.Warnf("network option com.docker.network.bridge.enable_icc is not currently supported in nerdctl", v)
43-
case "com.docker.network.bridge.name":
44-
bridge = v
45-
default:
46-
options[k] = v
47-
}
21+
var bridgeDriver driver.DriverHandler
22+
var err error
23+
24+
createOptionsFrom := func(request types.NetworkCreateRequest) (netutil.CreateOptions, error) {
25+
// Default to "bridge" driver if request does not specify a driver
26+
networkDriver := request.Driver
27+
if networkDriver == "" {
28+
networkDriver = "bridge"
4829
}
49-
return options
50-
}
5130

52-
createOptionsFrom := func(r types.NetworkCreateRequest) netutil.CreateOptions {
5331
options := netutil.CreateOptions{
54-
Name: r.Name,
55-
Driver: "bridge",
32+
Name: request.Name,
33+
Driver: networkDriver,
5634
IPAMDriver: "default",
57-
IPAMOptions: r.IPAM.Options,
58-
Options: filterUnsupportedOptions(r.Options),
59-
Labels: maputility.Flatten(r.Labels, maputility.KeyEqualsValueFormat),
60-
}
61-
if r.Driver != "" {
62-
options.Driver = r.Driver
35+
IPAMOptions: request.IPAM.Options,
36+
Labels: maputility.Flatten(request.Labels, maputility.KeyEqualsValueFormat),
37+
IPv6: request.EnableIPv6,
6338
}
64-
if r.IPAM.Driver != "" {
65-
options.IPAMDriver = r.IPAM.Driver
39+
40+
if request.IPAM.Driver != "" {
41+
options.IPAMDriver = request.IPAM.Driver
6642
}
43+
6744
if len(request.IPAM.Config) != 0 {
6845
options.Subnets = []string{}
6946
if subnet, ok := request.IPAM.Config[0]["Subnet"]; ok {
@@ -76,7 +53,21 @@ func (s *service) Create(ctx context.Context, request types.NetworkCreateRequest
7653
options.Gateway = gateway
7754
}
7855
}
79-
return options
56+
57+
// Handle driver-specific options
58+
switch networkDriver {
59+
case "bridge":
60+
bridgeDriver, err = driver.NewBridgeDriver(s.netClient, s.logger, options.IPv6)
61+
if err != nil {
62+
return options, err
63+
}
64+
options, err = bridgeDriver.HandleCreateOptions(request, options)
65+
return options, err
66+
default:
67+
options.Options = request.Options
68+
}
69+
70+
return options, nil
8071
}
8172

8273
if config, err := s.getNetwork(request.Name); err == nil {
@@ -92,8 +83,21 @@ func (s *service) Create(ctx context.Context, request types.NetworkCreateRequest
9283
return response, nil
9384
}
9485

95-
net, err := s.netClient.CreateNetwork(createOptionsFrom(request))
96-
warning := ""
86+
options, err := createOptionsFrom(request)
87+
if err != nil {
88+
return types.NetworkCreateResponse{}, err
89+
}
90+
91+
// Ensure thread-safety for network operations using a per-network mutex.
92+
// Operations on different network IDs can proceed concurrently.
93+
netMu := s.ensureLock(request.Name)
94+
95+
netMu.Lock()
96+
defer netMu.Unlock()
97+
defer s.releaseLock(request.Name)
98+
99+
// Create network
100+
net, err := s.netClient.CreateNetwork(options)
97101
if err != nil && strings.Contains(err.Error(), "unsupported cni driver") {
98102
return types.NetworkCreateResponse{}, errdefs.NewNotFound(errPluginNotFound)
99103
} else if err != nil {
@@ -104,103 +108,33 @@ func (s *service) Create(ctx context.Context, request types.NetworkCreateRequest
104108
return types.NetworkCreateResponse{}, errNetworkIDNotFound
105109
}
106110

107-
// Since nerdctl currently does not support custom bridge names,
108-
// we explicitly override bridge name in the conflist file for the network that was just created
109-
if bridge != "" {
110-
if err = s.setBridgeName(net, bridge); err != nil {
111-
warning = fmt.Sprintf("Failed to set network bridge name %s: %s", bridge, err)
111+
// Add cleanup func to remove the network if an error is encountered during post processing
112+
cleanup := func(ctx context.Context, name string) {
113+
if cleanupErr := s.Remove(ctx, name); cleanupErr != nil {
114+
// ignore if the network does not exist
115+
if !errdefs.IsNotFound(cleanupErr) {
116+
s.logger.Errorf("cleanup failed in defer %s: %v", name, cleanupErr)
117+
}
112118
}
113119
}
114120

115-
return types.NetworkCreateResponse{
116-
ID: *net.NerdctlID,
117-
Warning: warning,
118-
}, nil
119-
}
120-
121-
// setBridgeName will override the bridge name in an existing CNI config file for a network.
122-
func (s *service) setBridgeName(net *netutil.NetworkConfig, bridge string) error {
123-
return lockutil.WithDirLock(s.netClient.NetconfPath(), func() error {
124-
// first, make sure that the bridge name is not used by any of the existing bridge networks
125-
bridgeNet, err := s.getNetworkByBridgeName(bridge)
121+
defer func() {
126122
if err != nil {
127-
return err
128-
}
129-
if bridgeNet != nil {
130-
return fmt.Errorf("bridge name %s already in use by network %s", bridge, bridgeNet.Name)
123+
cleanup(ctx, request.Name)
131124
}
125+
}()
132126

133-
// load the CNI config file and set bridge name
134-
configFilename := s.getConfigPathForNetworkName(net.Name)
135-
configFile, err := os.Open(configFilename)
127+
// Handle post network create actions
128+
warning := ""
129+
if bridgeDriver != nil {
130+
warning, err = bridgeDriver.HandlePostCreate(net)
136131
if err != nil {
137-
return err
138-
}
139-
defer configFile.Close()
140-
var netJSON interface{}
141-
if err = json.NewDecoder(configFile).Decode(&netJSON); err != nil {
142-
return err
143-
}
144-
netMap, ok := netJSON.(map[string]interface{})
145-
if !ok {
146-
return fmt.Errorf("network config file %s is not a valid map", configFilename)
147-
}
148-
plugins, ok := netMap["plugins"]
149-
if !ok {
150-
return fmt.Errorf("could not find plugins in network config file %s", configFilename)
151-
}
152-
pluginsMap, ok := plugins.([]interface{})
153-
if !ok {
154-
return fmt.Errorf("could not parse plugins in network config file %s", configFilename)
155-
}
156-
for _, plugin := range pluginsMap {
157-
pluginMap, ok := plugin.(map[string]interface{})
158-
if !ok {
159-
continue
160-
}
161-
if pluginMap["type"] == "bridge" {
162-
pluginMap["bridge"] = bridge
163-
data, err := json.MarshalIndent(netJSON, "", " ")
164-
if err != nil {
165-
return err
166-
}
167-
return os.WriteFile(configFilename, data, 0o644)
168-
}
169-
}
170-
return fmt.Errorf("bridge plugin not found in network config file %s", configFilename)
171-
})
172-
}
173-
174-
// From https://github.com/containerd/nerdctl/blob/v1.5.0/pkg/netutil/netutil.go#L186-L188
175-
func (s *service) getConfigPathForNetworkName(netName string) string {
176-
return filepath.Join(s.netClient.NetconfPath(), "nerdctl-"+netName+".conflist")
177-
}
178-
179-
type bridgePlugin struct {
180-
Type string `json:"type"`
181-
Bridge string `json:"bridge"`
182-
}
183-
184-
func (s *service) getNetworkByBridgeName(bridge string) (*netutil.NetworkConfig, error) {
185-
networks, err := s.netClient.FilterNetworks(func(*netutil.NetworkConfig) bool {
186-
return true
187-
})
188-
if err != nil {
189-
return nil, err
190-
}
191-
for _, network := range networks {
192-
for _, plugin := range network.Plugins {
193-
if plugin.Network.Type != "bridge" {
194-
continue
195-
}
196-
var bridgeJSON bridgePlugin
197-
if err = json.Unmarshal(plugin.Bytes, &bridgeJSON); err != nil {
198-
continue
199-
}
200-
if bridgeJSON.Bridge == bridge {
201-
return network, nil
202-
}
132+
return types.NetworkCreateResponse{}, err
203133
}
204134
}
205-
return nil, nil
135+
136+
return types.NetworkCreateResponse{
137+
ID: *net.NerdctlID,
138+
Warning: warning,
139+
}, nil
206140
}

0 commit comments

Comments
 (0)