From 609a9be65a844a5e4003d59f35e0ae8052261ceb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20=C4=8Cerm=C3=A1k?= Date: Mon, 11 Dec 2023 10:46:25 +0100 Subject: [PATCH] Gate CNI support behind the `cni` build tag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - add cni build tag to libnetwork/cni - split libnetwork/network into multiple files so that cni support can be made optionally available - add -cni build targets to Makefile and build for amd64 with and without cni - add a simple upgrade mechanism if the user never set the network backend explicitly - add cni build tag to .golangci.yml to prevent false positives See also https://issues.redhat.com/browse/RUN-1943 Signed-off-by: Dan Čermák --- .golangci.yml | 1 + Makefile | 9 +- libnetwork/cni/cni_conversion.go | 2 +- libnetwork/cni/cni_exec.go | 2 +- libnetwork/cni/cni_suite_test.go | 2 +- libnetwork/cni/cni_types.go | 2 +- libnetwork/cni/config.go | 2 +- libnetwork/cni/config_freebsd.go | 2 +- libnetwork/cni/config_linux.go | 2 +- libnetwork/cni/config_test.go | 2 +- libnetwork/cni/network.go | 2 +- libnetwork/cni/run.go | 2 +- libnetwork/cni/run_test.go | 2 +- libnetwork/network/interface.go | 179 ++++++------------ libnetwork/network/interface_cni.go | 121 ++++++++++++ .../network/interface_cni_unsupported.go | 32 ++++ 16 files changed, 233 insertions(+), 131 deletions(-) create mode 100644 libnetwork/network/interface_cni.go create mode 100644 libnetwork/network/interface_cni_unsupported.go diff --git a/.golangci.yml b/.golangci.yml index dc4c46294..e7e09feb4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -7,6 +7,7 @@ run: - systemd - exclude_graphdriver_btrfs - containers_image_openpgp + - cni concurrency: 6 deadline: 5m linters: diff --git a/Makefile b/Makefile index 9eb657b1d..35a5c4cbc 100644 --- a/Makefile +++ b/Makefile @@ -36,15 +36,19 @@ build-cross: $(call go-build,windows,386,${BUILDTAGS}) .PHONY: all -all: build-amd64 build-386 +all: build-amd64 build-386 build-amd64-cni .PHONY: build -build: build-amd64 build-386 +build: build-amd64 build-386 build-amd64-cni .PHONY: build-amd64 build-amd64: GOARCH=amd64 $(GO_BUILD) -tags $(BUILDTAGS) ./... +.PHONY: build-amd64-cni +build-amd64-cni: + GOARCH=amd64 $(GO_BUILD) -tags $(BUILDTAGS),cni ./... + .PHONY: build-386 build-386: ifneq ($(shell uname -s), Darwin) @@ -100,6 +104,7 @@ test: test-unit test-unit: netavark-testplugin go test --tags seccomp,$(BUILDTAGS) -v ./... go test --tags remote,$(BUILDTAGS) -v ./pkg/config + go test --tags cni,$(BUILDTAGS) -v ./libnetwork/cni .PHONY: codespell codespell: diff --git a/libnetwork/cni/cni_conversion.go b/libnetwork/cni/cni_conversion.go index 27bf2c657..ffadae0d2 100644 --- a/libnetwork/cni/cni_conversion.go +++ b/libnetwork/cni/cni_conversion.go @@ -1,4 +1,4 @@ -//go:build linux || freebsd +//go:build (linux || freebsd) && cni package cni diff --git a/libnetwork/cni/cni_exec.go b/libnetwork/cni/cni_exec.go index f42f9ef99..9ccf4eff4 100644 --- a/libnetwork/cni/cni_exec.go +++ b/libnetwork/cni/cni_exec.go @@ -16,7 +16,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build linux || freebsd +//go:build (linux || freebsd) && cni package cni diff --git a/libnetwork/cni/cni_suite_test.go b/libnetwork/cni/cni_suite_test.go index 780f93bee..0a21f8f3a 100644 --- a/libnetwork/cni/cni_suite_test.go +++ b/libnetwork/cni/cni_suite_test.go @@ -1,4 +1,4 @@ -//go:build linux +//go:build (linux || freebsd) && cni package cni_test diff --git a/libnetwork/cni/cni_types.go b/libnetwork/cni/cni_types.go index ee1a4735c..711535ced 100644 --- a/libnetwork/cni/cni_types.go +++ b/libnetwork/cni/cni_types.go @@ -1,4 +1,4 @@ -//go:build linux || freebsd +//go:build (linux || freebsd) && cni package cni diff --git a/libnetwork/cni/config.go b/libnetwork/cni/config.go index f28f936c0..71b7872f1 100644 --- a/libnetwork/cni/config.go +++ b/libnetwork/cni/config.go @@ -1,4 +1,4 @@ -//go:build linux || freebsd +//go:build (linux || freebsd) && cni package cni diff --git a/libnetwork/cni/config_freebsd.go b/libnetwork/cni/config_freebsd.go index b406803d4..ddee6d2e0 100644 --- a/libnetwork/cni/config_freebsd.go +++ b/libnetwork/cni/config_freebsd.go @@ -1,4 +1,4 @@ -//go:build freebsd +//go:build (linux || freebsd) && cni package cni diff --git a/libnetwork/cni/config_linux.go b/libnetwork/cni/config_linux.go index 0cbf99e06..efc920614 100644 --- a/libnetwork/cni/config_linux.go +++ b/libnetwork/cni/config_linux.go @@ -1,4 +1,4 @@ -//go:build linux +//go:build (linux || freebsd) && cni package cni diff --git a/libnetwork/cni/config_test.go b/libnetwork/cni/config_test.go index f62a3738e..f7429844d 100644 --- a/libnetwork/cni/config_test.go +++ b/libnetwork/cni/config_test.go @@ -1,4 +1,4 @@ -//go:build linux +//go:build (linux || freebsd) && cni package cni_test diff --git a/libnetwork/cni/network.go b/libnetwork/cni/network.go index 04f267234..06b78f675 100644 --- a/libnetwork/cni/network.go +++ b/libnetwork/cni/network.go @@ -1,4 +1,4 @@ -//go:build linux || freebsd +//go:build (linux || freebsd) && cni package cni diff --git a/libnetwork/cni/run.go b/libnetwork/cni/run.go index 513481f01..d8fb47759 100644 --- a/libnetwork/cni/run.go +++ b/libnetwork/cni/run.go @@ -1,4 +1,4 @@ -//go:build linux || freebsd +//go:build (linux || freebsd) && cni package cni diff --git a/libnetwork/cni/run_test.go b/libnetwork/cni/run_test.go index 3aa335574..13c3877ea 100644 --- a/libnetwork/cni/run_test.go +++ b/libnetwork/cni/run_test.go @@ -1,4 +1,4 @@ -//go:build linux +//go:build (linux || freebsd) && cni package cni_test diff --git a/libnetwork/network/interface.go b/libnetwork/network/interface.go index 576054079..51c1ae718 100644 --- a/libnetwork/network/interface.go +++ b/libnetwork/network/interface.go @@ -8,13 +8,10 @@ import ( "os" "path/filepath" - "github.com/containers/common/libnetwork/cni" "github.com/containers/common/libnetwork/netavark" "github.com/containers/common/libnetwork/types" "github.com/containers/common/pkg/config" - "github.com/containers/common/pkg/machine" "github.com/containers/storage" - "github.com/containers/storage/pkg/homedir" "github.com/containers/storage/pkg/ioutils" "github.com/containers/storage/pkg/unshare" "github.com/sirupsen/logrus" @@ -23,8 +20,6 @@ import ( const ( // defaultNetworkBackendFileName is the file name for sentinel file to store the backend defaultNetworkBackendFileName = "defaultNetworkBackend" - // cniConfigDirRootless is the directory in XDG_CONFIG_HOME for cni plugins - cniConfigDirRootless = "cni/net.d/" // netavarkBinary is the name of the netavark binary netavarkBinary = "netavark" @@ -52,146 +47,94 @@ func NetworkBackend(store storage.Store, conf *config.Config, syslog bool) (type } } - switch backend { - case types.Netavark: - netavarkBin, err := conf.FindHelperBinary(netavarkBinary, false) - if err != nil { - return "", nil, err - } + return backendFromType(backend, store, conf, syslog) +} - aardvarkBin, _ := conf.FindHelperBinary(aardvarkBinary, false) +func netavarkBackendFromConf(store storage.Store, conf *config.Config, syslog bool) (types.ContainerNetwork, error) { + netavarkBin, err := conf.FindHelperBinary(netavarkBinary, false) + if err != nil { + return nil, err + } - confDir := conf.Network.NetworkConfigDir - if confDir == "" { - confDir = getDefaultNetavarkConfigDir(store) - } + aardvarkBin, _ := conf.FindHelperBinary(aardvarkBinary, false) - // We cannot use the runroot for rootful since the network namespace is shared for all - // libpod instances they also have to share the same ipam db. - // For rootless we have our own network namespace per libpod instances, - // so this is not a problem there. - runDir := netavarkRunDir - if unshare.IsRootless() { - runDir = filepath.Join(store.RunRoot(), "networks") - } + confDir := conf.Network.NetworkConfigDir + if confDir == "" { + confDir = getDefaultNetavarkConfigDir(store) + } - netInt, err := netavark.NewNetworkInterface(&netavark.InitConfig{ - Config: conf, - NetworkConfigDir: confDir, - NetworkRunDir: runDir, - NetavarkBinary: netavarkBin, - AardvarkBinary: aardvarkBin, - Syslog: syslog, - }) - return types.Netavark, netInt, err - case types.CNI: - netInt, err := getCniInterface(conf) - return types.CNI, netInt, err - - default: - return "", nil, fmt.Errorf("unsupported network backend %q, check network_backend in containers.conf", backend) + // We cannot use the runroot for rootful since the network namespace is shared for all + // libpod instances they also have to share the same ipam db. + // For rootless we have our own network namespace per libpod instances, + // so this is not a problem there. + runDir := netavarkRunDir + if unshare.IsRootless() { + runDir = filepath.Join(store.RunRoot(), "networks") } + + netInt, err := netavark.NewNetworkInterface(&netavark.InitConfig{ + Config: conf, + NetworkConfigDir: confDir, + NetworkRunDir: runDir, + NetavarkBinary: netavarkBin, + AardvarkBinary: aardvarkBin, + Syslog: syslog, + }) + return netInt, err } func defaultNetworkBackend(store storage.Store, conf *config.Config) (backend types.NetworkBackend, err error) { - // read defaultNetworkBackend file + err = nil + file := filepath.Join(store.GraphRoot(), defaultNetworkBackendFileName) - b, err := os.ReadFile(file) - if err == nil { - val := string(b) - if val == string(types.Netavark) { - return types.Netavark, nil - } - if val == string(types.CNI) { - return types.CNI, nil - } - return "", fmt.Errorf("unknown network backend value %q in %q", val, file) - } - // fail for all errors except ENOENT - if !errors.Is(err, os.ErrNotExist) { - return "", fmt.Errorf("could not read network backend value: %w", err) - } - // cache the network backend to make sure always the same one will be used - defer func() { + writeBackendToFile := func(backendT types.NetworkBackend) { // only write when there is no error if err == nil { - if err := ioutils.AtomicWriteFile(file, []byte(backend), 0o644); err != nil { + if err := ioutils.AtomicWriteFile(file, []byte(backendT), 0o644); err != nil { logrus.Errorf("could not write network backend to file: %v", err) } } - }() - - _, err = conf.FindHelperBinary("netavark", false) - if err != nil { - // if we cannot find netavark use CNI - return types.CNI, nil - } - - // If there are any containers then return CNI - cons, err := store.Containers() - if err != nil { - return "", err - } - if len(cons) != 0 { - return types.CNI, nil - } - - // If there are any non ReadOnly images then return CNI - imgs, err := store.Images() - if err != nil { - return "", err - } - for _, i := range imgs { - if !i.ReadOnly { - return types.CNI, nil - } } - // If there are CNI Networks then return CNI - cniInterface, err := getCniInterface(conf) + // read defaultNetworkBackend file + b, err := os.ReadFile(file) if err == nil { - nets, err := cniInterface.NetworkList() - // there is always a default network so check > 1 - if err != nil && !errors.Is(err, os.ErrNotExist) { - return "", err - } - - if len(nets) > 1 { - // we do not have a fresh system so use CNI - return types.CNI, nil - } - } - return types.Netavark, nil -} + val := string(b) -func getCniInterface(conf *config.Config) (types.ContainerNetwork, error) { - confDir := conf.Network.NetworkConfigDir - if confDir == "" { - var err error - confDir, err = getDefaultCNIConfigDir() - if err != nil { - return nil, err + // if the network backend has been already set previously, + // handle the values depending on whether CNI is supported and + // whether the network backend is explicitly configured + if val == string(types.Netavark) { + // netavark is always good + return types.Netavark, nil + } else if val == string(types.CNI) { + if cniSupported { + return types.CNI, nil + } + // the user has *not* configured a network + // backend explicitly but used CNI in the past + // => we upgrade them in this case to netavark only + writeBackendToFile(types.Netavark) + logrus.Info("Migrating network backend to netavark as no backend has been configured previously") + return types.Netavark, nil } + return "", fmt.Errorf("unknown network backend value %q in %q", val, file) } - return cni.NewCNINetworkInterface(&cni.InitConfig{ - Config: conf, - CNIConfigDir: confDir, - RunDir: conf.Engine.TmpDir, - IsMachine: machine.IsGvProxyBased(), - }) -} -func getDefaultCNIConfigDir() (string, error) { - if !unshare.IsRootless() { - return cniConfigDir, nil + // fail for all errors except ENOENT + if !errors.Is(err, os.ErrNotExist) { + return "", fmt.Errorf("could not read network backend value: %w", err) } - configHome, err := homedir.GetConfigHome() + backend, err = networkBackendFromStore(store, conf) if err != nil { return "", err } - return filepath.Join(configHome, cniConfigDirRootless), nil + // cache the network backend to make sure always the same one will be used + writeBackendToFile(backend) + + return backend, nil } // getDefaultNetavarkConfigDir return the netavark config dir. For rootful it will diff --git a/libnetwork/network/interface_cni.go b/libnetwork/network/interface_cni.go new file mode 100644 index 000000000..0a8bc4472 --- /dev/null +++ b/libnetwork/network/interface_cni.go @@ -0,0 +1,121 @@ +//go:build (linux || freebsd) && cni +// +build linux freebsd +// +build cni + +package network + +import ( + "errors" + "fmt" + "os" + "path/filepath" + + "github.com/containers/common/libnetwork/cni" + "github.com/containers/common/libnetwork/types" + "github.com/containers/common/pkg/config" + "github.com/containers/common/pkg/machine" + "github.com/containers/storage" + "github.com/containers/storage/pkg/homedir" + "github.com/containers/storage/pkg/unshare" +) + +const ( + // cniConfigDirRootless is the directory in XDG_CONFIG_HOME for cni plugins + cniConfigDirRootless = "cni/net.d/" + + cniSupported = true +) + +func getCniInterface(conf *config.Config) (types.ContainerNetwork, error) { + confDir := conf.Network.NetworkConfigDir + if confDir == "" { + var err error + confDir, err = getDefaultCNIConfigDir() + if err != nil { + return nil, err + } + } + return cni.NewCNINetworkInterface(&cni.InitConfig{ + Config: conf, + CNIConfigDir: confDir, + RunDir: conf.Engine.TmpDir, + IsMachine: machine.IsGvProxyBased(), + }) +} + +func getDefaultCNIConfigDir() (string, error) { + if !unshare.IsRootless() { + return cniConfigDir, nil + } + + configHome, err := homedir.GetConfigHome() + if err != nil { + return "", err + } + + return filepath.Join(configHome, cniConfigDirRootless), nil +} + +func networkBackendFromStore(store storage.Store, conf *config.Config) (backend types.NetworkBackend, err error) { + _, err = conf.FindHelperBinary("netavark", false) + if err != nil { + // if we cannot find netavark use CNI + return types.CNI, nil + } + + // If there are any containers then return CNI + cons, err := store.Containers() + if err != nil { + return "", err + } + if len(cons) != 0 { + return types.CNI, nil + } + + // If there are any non ReadOnly images then return CNI + imgs, err := store.Images() + if err != nil { + return "", err + } + for _, i := range imgs { + if !i.ReadOnly { + return types.CNI, nil + } + } + + // If there are CNI Networks then return CNI + cniInterface, err := getCniInterface(conf) + if err == nil { + nets, err := cniInterface.NetworkList() + // there is always a default network so check > 1 + if err != nil && !errors.Is(err, os.ErrNotExist) { + return "", err + } + + if len(nets) > 1 { + // we do not have a fresh system so use CNI + return types.CNI, nil + } + } + return types.Netavark, nil +} + +func backendFromType(backend types.NetworkBackend, store storage.Store, conf *config.Config, syslog bool) (types.NetworkBackend, types.ContainerNetwork, error) { + switch backend { + case types.Netavark: + netInt, err := netavarkBackendFromConf(store, conf, syslog) + if err != nil { + return "", nil, err + } + return types.Netavark, netInt, err + case types.CNI: + netInt, err := getCniInterface(conf) + if err != nil { + return "", nil, err + } + return types.CNI, netInt, err + + default: + return "", nil, fmt.Errorf("unsupported network backend %q, check network_backend in containers.conf", backend) + } +} diff --git a/libnetwork/network/interface_cni_unsupported.go b/libnetwork/network/interface_cni_unsupported.go new file mode 100644 index 000000000..2f4bb8371 --- /dev/null +++ b/libnetwork/network/interface_cni_unsupported.go @@ -0,0 +1,32 @@ +//go:build (linux || freebsd) && !cni +// +build linux freebsd +// +build !cni + +package network + +import ( + "fmt" + + "github.com/containers/common/libnetwork/types" + "github.com/containers/common/pkg/config" + "github.com/containers/storage" +) + +const ( + cniSupported = false +) + +func networkBackendFromStore(_store storage.Store, _conf *config.Config) (backend types.NetworkBackend, err error) { + return types.Netavark, nil +} + +func backendFromType(backend types.NetworkBackend, store storage.Store, conf *config.Config, syslog bool) (types.NetworkBackend, types.ContainerNetwork, error) { + if backend != types.Netavark { + return "", nil, fmt.Errorf("cni support is not enabled in this build, only netavark. Got unsupported network backend %q", backend) + } + cn, err := netavarkBackendFromConf(store, conf, syslog) + if err != nil { + return "", nil, err + } + return types.Netavark, cn, err +}