From 5679e189e76fa19d7619ccf0de71b6d512d77164 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Fri, 22 Nov 2024 16:55:15 +0100 Subject: [PATCH] Removed capabilites from coil and moved them to coild Signed-off-by: Patryk Strusiewicz-Surmacki --- .github/workflows/ci.yaml | 5 +- docs/setup.md | 13 - v2/cmd/coil/main.go | 13 +- v2/cmd/coil/rpc.go | 13 +- v2/cmd/coild/sub/root.go | 46 +-- v2/cmd/coild/sub/run.go | 35 +- v2/e2e/netconf/netconf-kindnet-v4.json | 6 +- v2/e2e/netconf/netconf-kindnet-v6.json | 6 +- v2/netconf.json | 6 +- v2/pkg/config/config.go | 51 +++ v2/pkg/constants/constants.go | 17 +- v2/runners/coild_server.go | 78 ++-- v2/runners/coild_server_test.go | 490 ++++++++++++++----------- 13 files changed, 402 insertions(+), 377 deletions(-) create mode 100644 v2/pkg/config/config.go diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 0be9e8dd..0530b043 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -13,6 +13,9 @@ env: jobs: test: name: Small test + strategy: + matrix: + test-ipam: ["true", "false"] runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v4 @@ -28,7 +31,7 @@ jobs: v2/bin v2/include key: cache-${{ env.cache-version }}-go-${{ env.go-version }}-${{ hashFiles('v2/Makefile') }} - - run: make setup + - run: make setup TEST_IPAM=${{ matrix.test-ipam }} TEST_EGRESS=true if: steps.cache-tools.outputs.cache-hit != 'true' - run: make test - run: make test-nodenet diff --git a/docs/setup.md b/docs/setup.md index 73f9075f..cb6b7e5c 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -80,8 +80,6 @@ The following example adds `tuning` and `bandwidth` plugins. { "type": "coil", "socket": "/run/coild.sock", - "ipam": true, - "egress": true }, { "type": "tuning", @@ -271,17 +269,6 @@ To deploy Coil with only egress feature enabled the following changes are requir - name: CNI_CONF_NAME value: "01-coil.conflist" ``` -1. Set coil capabilites in `v2/netconf.json` to: - ```json - { - "type": "coil", - "socket": "/run/coild.sock", - "capabilities": { - "ipam": false, - "egress": true - } - }, - ``` 1. Add configuration of your chosen CNI to `v2/netconf.json` before `coil` related configuration. 1. Deploy `coil` to existing cluster as described in [Compile and apply the manifest](#compile-and-apply-the-manifest). diff --git a/v2/cmd/coil/main.go b/v2/cmd/coil/main.go index f47b8765..18a7ebab 100644 --- a/v2/cmd/coil/main.go +++ b/v2/cmd/coil/main.go @@ -14,9 +14,7 @@ import ( ) const ( - rpcTimeout = 1 * time.Minute - ipamEnableKey = "ipam" - egressEnableKey = "egress" + rpcTimeout = 1 * time.Minute ) func cmdAdd(args *skel.CmdArgs) error { @@ -25,15 +23,6 @@ func cmdAdd(args *skel.CmdArgs) error { return err } - ipamEnablad, exists := conf.Capabilities[ipamEnableKey] - if !exists { - ipamEnablad = true - } - - if ipamEnablad && conf.PrevResult != nil { - return types.NewError(types.ErrInvalidNetworkConfig, "coil must be called as the first plugin when IPAM related features are enabled", "") - } - cniArgs, err := makeCNIArgs(args, conf) if err != nil { return err diff --git a/v2/cmd/coil/rpc.go b/v2/cmd/coil/rpc.go index 72f2e580..0ec38c95 100644 --- a/v2/cmd/coil/rpc.go +++ b/v2/cmd/coil/rpc.go @@ -25,18 +25,7 @@ func makeCNIArgs(args *skel.CmdArgs, conf *PluginConf) (*cnirpc.CNIArgs, error) } argsData := env.Map() - ipamEnablad, exists := conf.Capabilities[ipamEnableKey] - if !exists { - ipamEnablad = true - } - - egressEnabled, exists := conf.Capabilities[egressEnableKey] - if !exists { - egressEnabled = true - } - - argsData[constants.EnableIPAM] = strconv.FormatBool(ipamEnablad) - argsData[constants.EnableEgress] = strconv.FormatBool(egressEnabled) + argsData[constants.IsChained] = strconv.FormatBool(conf.PrevResult != nil) ips := []string{} interfaces := map[string]bool{} diff --git a/v2/cmd/coild/sub/root.go b/v2/cmd/coild/sub/root.go index e25af4f5..a332bee7 100644 --- a/v2/cmd/coild/sub/root.go +++ b/v2/cmd/coild/sub/root.go @@ -1,33 +1,14 @@ package sub import ( - "flag" "fmt" "os" v2 "github.com/cybozu-go/coil/v2" - "github.com/cybozu-go/coil/v2/pkg/constants" + "github.com/cybozu-go/coil/v2/pkg/config" "github.com/spf13/cobra" - "k8s.io/klog/v2" - "sigs.k8s.io/controller-runtime/pkg/log/zap" ) -var config struct { - metricsAddr string - healthAddr string - podTableId int - podRulePrio int - exportTableId int - protocolId int - socketPath string - compatCalico bool - egressPort int - registerFromMain bool - zapOpts zap.Options - enableIPAM bool - enableEgress bool -} - var rootCmd = &cobra.Command{ Use: "coild", Short: "gRPC server running on each node", @@ -42,33 +23,14 @@ coil CNI plugin.`, }, } +var cfg *config.Config + // Execute adds all child commands to the root command and sets flags appropriately. // This is called by main.main(). It only needs to happen once to the rootCmd. func Execute() { + cfg = config.Parse(rootCmd) if err := rootCmd.Execute(); err != nil { fmt.Println(err) os.Exit(1) } } - -func init() { - pf := rootCmd.PersistentFlags() - pf.StringVar(&config.metricsAddr, "metrics-addr", ":9384", "bind address of metrics endpoint") - pf.StringVar(&config.healthAddr, "health-addr", ":9385", "bind address of health/readiness probes") - pf.IntVar(&config.podTableId, "pod-table-id", 116, "routing table ID to which coild registers routes for Pods") - pf.IntVar(&config.podRulePrio, "pod-rule-prio", 2000, "priority with which the rule for Pod table is inserted") - pf.IntVar(&config.exportTableId, "export-table-id", 119, "routing table ID to which coild exports routes") - pf.IntVar(&config.protocolId, "protocol-id", 30, "route author ID") - pf.StringVar(&config.socketPath, "socket", constants.DefaultSocketPath, "UNIX domain socket path") - pf.BoolVar(&config.compatCalico, "compat-calico", false, "make veth name compatible with Calico") - pf.IntVar(&config.egressPort, "egress-port", 5555, "UDP port number for egress NAT") - pf.BoolVar(&config.registerFromMain, "register-from-main", false, "help migration from Coil 2.0.1") - pf.BoolVar(&config.enableIPAM, "enable-ipam", true, "enable IPAM related features") - pf.BoolVar(&config.enableEgress, "enable-egress", true, "enable IPAM related features") - - goflags := flag.NewFlagSet("klog", flag.ExitOnError) - klog.InitFlags(goflags) - config.zapOpts.BindFlags(goflags) - - pf.AddGoFlagSet(goflags) -} diff --git a/v2/cmd/coild/sub/run.go b/v2/cmd/coild/sub/run.go index 61c3cc9e..c79d0a7c 100644 --- a/v2/cmd/coild/sub/run.go +++ b/v2/cmd/coild/sub/run.go @@ -38,13 +38,12 @@ var ( func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(coilv2.AddToScheme(scheme)) - // +kubebuilder:scaffold:scheme } func subMain() error { // coild needs a raw zap logger for grpc_zip. - zapLogger := zap.NewRaw(zap.UseFlagOptions(&config.zapOpts)) + zapLogger := zap.NewRaw(zap.UseFlagOptions(&cfg.ZapOpts)) defer zapLogger.Sync() grpcLogger := zapLogger.Named("grpc") @@ -60,10 +59,10 @@ func subMain() error { Scheme: scheme, LeaderElection: false, Metrics: metricsserver.Options{ - BindAddress: config.metricsAddr, + BindAddress: cfg.MetricsAddr, }, GracefulShutdownTimeout: &timeout, - HealthProbeBindAddress: config.healthAddr, + HealthProbeBindAddress: cfg.HealthAddr, }) if err != nil { return err @@ -76,9 +75,9 @@ func subMain() error { return err } - exporter := nodenet.NewRouteExporter(config.exportTableId, config.protocolId, ctrl.Log.WithName("route-exporter")) + exporter := nodenet.NewRouteExporter(cfg.ExportTableId, cfg.ProtocolId, ctrl.Log.WithName("route-exporter")) nodeIPAM := ipam.NewNodeIPAM(nodeName, ctrl.Log.WithName("node-ipam"), mgr, exporter) - if config.enableIPAM { + if cfg.EnableIPAM { watcher := &controllers.BlockRequestWatcher{ Client: mgr.GetClient(), NodeIPAM: nodeIPAM, @@ -96,20 +95,20 @@ func subMain() error { } podNet := nodenet.NewPodNetwork( - config.podTableId, - config.podRulePrio, - config.protocolId, + cfg.PodTableId, + cfg.PodRulePrio, + cfg.ProtocolId, ipv4, ipv6, - config.compatCalico, - config.registerFromMain, + cfg.CompatCalico, + cfg.RegisterFromMain, ctrl.Log.WithName("pod-network"), - config.enableIPAM) + cfg.EnableIPAM) if err := podNet.Init(); err != nil { return err } - if config.enableIPAM { + if cfg.EnableIPAM { podConfigs, err := podNet.List() if err != nil { return err @@ -125,22 +124,22 @@ func subMain() error { } } - os.Remove(config.socketPath) - l, err := net.Listen("unix", config.socketPath) + os.Remove(cfg.SocketPath) + l, err := net.Listen("unix", cfg.SocketPath) if err != nil { return err } - server := runners.NewCoildServer(l, mgr, nodeIPAM, podNet, runners.NewNATSetup(config.egressPort), grpcLogger) + server := runners.NewCoildServer(l, mgr, nodeIPAM, podNet, runners.NewNATSetup(cfg.EgressPort), cfg, grpcLogger, runners.SetCoilInterfaceAlias) if err := mgr.Add(server); err != nil { return err } - if config.enableEgress { + if cfg.EnableEgress { egressWatcher := &controllers.EgressWatcher{ Client: mgr.GetClient(), NodeName: nodeName, PodNet: podNet, - EgressPort: config.egressPort, + EgressPort: cfg.EgressPort, } if err := egressWatcher.SetupWithManager(mgr); err != nil { return err diff --git a/v2/e2e/netconf/netconf-kindnet-v4.json b/v2/e2e/netconf/netconf-kindnet-v4.json index b9ecaf83..5ff4036b 100644 --- a/v2/e2e/netconf/netconf-kindnet-v4.json +++ b/v2/e2e/netconf/netconf-kindnet-v4.json @@ -21,11 +21,7 @@ }, { "type": "coil", - "socket": "/run/coild.sock", - "capabilities": { - "ipam": false, - "egress": true - } + "socket": "/run/coild.sock" }, { "type": "portmap", diff --git a/v2/e2e/netconf/netconf-kindnet-v6.json b/v2/e2e/netconf/netconf-kindnet-v6.json index 5f64f442..1c28344d 100644 --- a/v2/e2e/netconf/netconf-kindnet-v6.json +++ b/v2/e2e/netconf/netconf-kindnet-v6.json @@ -19,11 +19,7 @@ }, { "type": "coil", - "socket": "/run/coild.sock", - "capabilities": { - "ipam": false, - "egress": true - } + "socket": "/run/coild.sock" }, { "type": "portmap", diff --git a/v2/netconf.json b/v2/netconf.json index d0faa985..d3efb490 100644 --- a/v2/netconf.json +++ b/v2/netconf.json @@ -4,11 +4,7 @@ "plugins": [ { "type": "coil", - "socket": "/run/coild.sock", - "capabilities": { - "ipam": true, - "egress": true - } + "socket": "/run/coild.sock" }, { "type": "portmap", diff --git a/v2/pkg/config/config.go b/v2/pkg/config/config.go new file mode 100644 index 00000000..fffeb0e2 --- /dev/null +++ b/v2/pkg/config/config.go @@ -0,0 +1,51 @@ +package config + +import ( + "flag" + + "github.com/cybozu-go/coil/v2/pkg/constants" + "github.com/spf13/cobra" + "k8s.io/klog/v2" + "sigs.k8s.io/controller-runtime/pkg/log/zap" +) + +type Config struct { + MetricsAddr string + HealthAddr string + PodTableId int + PodRulePrio int + ExportTableId int + ProtocolId int + SocketPath string + CompatCalico bool + EgressPort int + RegisterFromMain bool + ZapOpts zap.Options + EnableIPAM bool + EnableEgress bool +} + +func Parse(rootCmd *cobra.Command) *Config { + config := &Config{} + pf := rootCmd.PersistentFlags() + pf.StringVar(&config.MetricsAddr, "metrics-addr", constants.DefautlMetricsAddr, "bind address of metrics endpoint") + pf.StringVar(&config.HealthAddr, "health-addr", constants.DefautlHealthAddr, "bind address of health/readiness probes") + pf.IntVar(&config.PodTableId, "pod-table-id", constants.DefautlPodTableId, "routing table ID to which coild registers routes for Pods") + pf.IntVar(&config.PodRulePrio, "pod-rule-prio", constants.DefautlPodRulePrio, "priority with which the rule for Pod table is inserted") + pf.IntVar(&config.ExportTableId, "export-table-id", constants.DefautlExportTableId, "routing table ID to which coild exports routes") + pf.IntVar(&config.ProtocolId, "protocol-id", constants.DefautlProtocolId, "route author ID") + pf.StringVar(&config.SocketPath, "socket", constants.DefaultSocketPath, "UNIX domain socket path") + pf.BoolVar(&config.CompatCalico, "compat-calico", constants.DefaultCompatCalico, "make veth name compatible with Calico") + pf.IntVar(&config.EgressPort, "egress-port", constants.DefaultEgressPort, "UDP port number for egress NAT") + pf.BoolVar(&config.RegisterFromMain, "register-from-main", constants.DefaultRegisterFromMain, "help migration from Coil 2.0.1") + pf.BoolVar(&config.EnableIPAM, "enable-ipam", constants.DefaultEnableIPAM, "enable IPAM related features") + pf.BoolVar(&config.EnableEgress, "enable-egress", constants.DefaultEnableEgress, "enable IPAM related features") + + goflags := flag.NewFlagSet("klog", flag.ExitOnError) + klog.InitFlags(goflags) + config.ZapOpts.BindFlags(goflags) + + pf.AddGoFlagSet(goflags) + + return config +} diff --git a/v2/pkg/constants/constants.go b/v2/pkg/constants/constants.go index acc28e87..a632d470 100644 --- a/v2/pkg/constants/constants.go +++ b/v2/pkg/constants/constants.go @@ -58,10 +58,23 @@ const ( ) // Config flags +const ( + IsChained = "IS_CHAINED" +) +// Default config values const ( - EnableIPAM = "ENABLE_IPAM" - EnableEgress = "ENABLE_EGRESS" + DefautlMetricsAddr = ":9384" + DefautlHealthAddr = ":9385" + DefautlPodTableId = 116 + DefautlPodRulePrio = 2000 + DefautlExportTableId = 119 + DefautlProtocolId = 30 + DefaultCompatCalico = false + DefaultEgressPort = 5555 + DefaultRegisterFromMain = false + DefaultEnableIPAM = true + DefaultEnableEgress = true ) // MetricsNS is the namespace for Prometheus metrics diff --git a/v2/runners/coild_server.go b/v2/runners/coild_server.go index c1df1aaa..6faa853a 100644 --- a/v2/runners/coild_server.go +++ b/v2/runners/coild_server.go @@ -12,6 +12,7 @@ import ( current "github.com/containernetworking/cni/pkg/types/100" coilv2 "github.com/cybozu-go/coil/v2/api/v2" "github.com/cybozu-go/coil/v2/pkg/cnirpc" + "github.com/cybozu-go/coil/v2/pkg/config" "github.com/cybozu-go/coil/v2/pkg/constants" "github.com/cybozu-go/coil/v2/pkg/founat" "github.com/cybozu-go/coil/v2/pkg/ipam" @@ -91,7 +92,8 @@ func (n natSetup) Hook(l []GWNets, log *zap.Logger) func(ipv4, ipv6 net.IP) erro } // NewCoildServer returns an implementation of cnirpc.CNIServer for coild. -func NewCoildServer(l net.Listener, mgr manager.Manager, nodeIPAM ipam.NodeIPAM, podNet nodenet.PodNetwork, setup NATSetup, logger *zap.Logger) manager.Runnable { +func NewCoildServer(l net.Listener, mgr manager.Manager, nodeIPAM ipam.NodeIPAM, podNet nodenet.PodNetwork, setup NATSetup, cfg *config.Config, logger *zap.Logger, + aliasFunc func(interfaces map[string]bool, conf *nodenet.PodNetConf, logger *zap.Logger, pod *corev1.Pod) error) manager.Runnable { return &coildServer{ listener: l, apiReader: mgr.GetAPIReader(), @@ -100,6 +102,8 @@ func NewCoildServer(l net.Listener, mgr manager.Manager, nodeIPAM ipam.NodeIPAM, podNet: podNet, natSetup: setup, logger: logger, + cfg: cfg, + aliasFunc: aliasFunc, } } @@ -123,6 +127,8 @@ type coildServer struct { podNet nodenet.PodNetwork natSetup NATSetup logger *zap.Logger + cfg *config.Config + aliasFunc func(interfaces map[string]bool, conf *nodenet.PodNetConf, logger *zap.Logger, pod *corev1.Pod) error } var _ manager.LeaderElectionRunnable = &coildServer{} @@ -173,24 +179,28 @@ func newInternalError(err error, msg string) error { func (s *coildServer) Add(ctx context.Context, args *cnirpc.CNIArgs) (*cnirpc.AddResponse, error) { logger := withCtxFields(ctx, s.logger) - pod, err := s.getPodFromArgs(ctx, args, logger) + isChained, err := getSettings(args) if err != nil { - return nil, newInternalError(err, "failed to get pod") + return nil, newInternalError(fmt.Errorf("runtime error"), "failed to get CNi arguments") } - ipamEnabled, egressEnabled, err := getSettings(args) + if s.cfg.EnableIPAM && isChained { + return nil, newInternalError(fmt.Errorf("configuration error"), "coil must be called as the first plugin when IPAM related features are enabled") + } + + pod, err := s.getPodFromArgs(ctx, args, logger) if err != nil { - return nil, newInternalError(err, "error parsing settings") + return nil, newInternalError(err, "failed to get pod") } - if !ipamEnabled && !egressEnabled { + if !s.cfg.EnableIPAM && !s.cfg.EnableEgress { return nil, newInternalError(fmt.Errorf("configuration error"), "both ipam and egress are disabled") } var ipv4, ipv6 net.IP var poolName string - if ipamEnabled { + if s.cfg.EnableIPAM { ns := &corev1.Namespace{} if err := s.client.Get(ctx, client.ObjectKey{Name: pod.Namespace}, ns); err != nil { logger.Sugar().Errorw("failed to get namespace", "name", pod.Namespace, "error", err) @@ -222,7 +232,7 @@ func (s *coildServer) Add(ctx context.Context, args *cnirpc.CNIArgs) (*cnirpc.Ad PoolName: poolName, } - if ipamEnabled { + if s.cfg.EnableIPAM { result, err = s.podNet.SetupIPAM(args.Netns, pod.Name, pod.Namespace, config) if err != nil { if err := s.nodeIPAM.Free(ctx, args.ContainerId, args.Ifname); err != nil { @@ -233,10 +243,10 @@ func (s *coildServer) Add(ctx context.Context, args *cnirpc.CNIArgs) (*cnirpc.Ad } } - if egressEnabled { - if !ipamEnabled { - if err := setCoilInterfaceAlias(args.Interfaces, config, logger, pod); err != nil { - return nil, newInternalError(err, "failed to set interface alias") + if s.cfg.EnableEgress { + if !s.cfg.EnableIPAM { + if err := s.aliasFunc(args.Interfaces, config, logger, pod); err != nil { + return nil, fmt.Errorf("failed to set interface alias: %w", err) } } @@ -256,7 +266,7 @@ func (s *coildServer) Add(ctx context.Context, args *cnirpc.CNIArgs) (*cnirpc.Ad data, err := json.Marshal(result) if err != nil { - if ipamEnabled { + if s.cfg.EnableIPAM { if err := s.podNet.Destroy(args.ContainerId, args.Ifname); err != nil { logger.Sugar().Warnw("failed to destroy pod network", "error", err) } @@ -270,7 +280,7 @@ func (s *coildServer) Add(ctx context.Context, args *cnirpc.CNIArgs) (*cnirpc.Ad return &cnirpc.AddResponse{Result: data}, nil } -func setCoilInterfaceAlias(interfaces map[string]bool, conf *nodenet.PodNetConf, logger *zap.Logger, pod *corev1.Pod) error { +func SetCoilInterfaceAlias(interfaces map[string]bool, conf *nodenet.PodNetConf, logger *zap.Logger, pod *corev1.Pod) error { ifName := "" for name, isSandbox := range interfaces { if !isSandbox { @@ -281,7 +291,7 @@ func setCoilInterfaceAlias(interfaces map[string]bool, conf *nodenet.PodNetConf, logger.Sugar().Infof("interface selected: %s", ifName) hLink, err := netlink.LinkByName(ifName) if err != nil { - return fmt.Errorf("netlink: failed to look up the host-side veth: %w", err) + return fmt.Errorf("netlink: failed to look up the host-side veth [%s]: %w", ifName, err) } logger.Sugar().Infof("link found: %v", hLink) @@ -313,12 +323,7 @@ func getPodIPs(ips []string) (net.IP, net.IP) { func (s *coildServer) Del(ctx context.Context, args *cnirpc.CNIArgs) (*emptypb.Empty, error) { logger := withCtxFields(ctx, s.logger) - ipamEnabled, _, err := getSettings(args) - if err != nil { - return nil, newInternalError(err, "error parsing settings") - } - - if ipamEnabled { + if s.cfg.EnableIPAM { if err := s.podNet.Destroy(args.ContainerId, args.Ifname); err != nil { logger.Sugar().Errorw("failed to destroy pod network", "error", err) return nil, newInternalError(err, "failed to destroy pod network") @@ -335,17 +340,12 @@ func (s *coildServer) Del(ctx context.Context, args *cnirpc.CNIArgs) (*emptypb.E func (s *coildServer) Check(ctx context.Context, args *cnirpc.CNIArgs) (*emptypb.Empty, error) { logger := withCtxFields(ctx, s.logger) - ipamEnabled, egressEnabled, err := getSettings(args) - if err != nil { - return nil, newInternalError(err, "check failed") - } - - if ipamEnabled { + if s.cfg.EnableIPAM { if err := s.podNet.Check(args.ContainerId, args.Ifname); err != nil { logger.Sugar().Errorw("check failed", "error", err) return nil, newInternalError(err, "check failed") } - } else if egressEnabled { + } else if s.cfg.EnableEgress { pod, err := s.getPodFromArgs(ctx, args, logger) if err != nil { return nil, newInternalError(err, "unable to get pod") @@ -534,22 +534,14 @@ func withCtxFields(ctx context.Context, l *zap.Logger) *zap.Logger { return l.With(toZapFields(logging.ExtractFields(ctx))...) } -func getSettings(args *cnirpc.CNIArgs) (bool, bool, error) { - ipamEnabled := true - egressEnabled := true - +func getSettings(args *cnirpc.CNIArgs) (bool, error) { + isChained := false var err error - if args.Args[constants.EnableIPAM] != "" { - if ipamEnabled, err = strconv.ParseBool(args.Args[constants.EnableIPAM]); err != nil { - return false, false, newInternalError(err, "error parsing bool value for IPAM enable flag") + _, exists := args.Args[constants.IsChained] + if exists { + if isChained, err = strconv.ParseBool(args.Args[constants.IsChained]); err != nil { + return false, newInternalError(err, "error parsing CNI chaining bool value ") } } - - if args.Args[constants.EnableEgress] != "" { - if egressEnabled, err = strconv.ParseBool(args.Args[constants.EnableEgress]); err != nil { - return false, false, newInternalError(err, "error parsing bool value for Egress enable flag") - } - } - - return ipamEnabled, egressEnabled, nil + return isChained, nil } diff --git a/v2/runners/coild_server_test.go b/v2/runners/coild_server_test.go index 41e264cf..d4ed526a 100644 --- a/v2/runners/coild_server_test.go +++ b/v2/runners/coild_server_test.go @@ -14,6 +14,8 @@ import ( current "github.com/containernetworking/cni/pkg/types/100" coilv2 "github.com/cybozu-go/coil/v2/api/v2" "github.com/cybozu-go/coil/v2/pkg/cnirpc" + "github.com/cybozu-go/coil/v2/pkg/config" + "github.com/cybozu-go/coil/v2/pkg/constants" "github.com/cybozu-go/coil/v2/pkg/nodenet" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -30,6 +32,11 @@ import ( metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" ) +const ( + testIPAMKey = "TEST_IPAM" + testEgressKey = "TEST_EGRESS" +) + type mockNodeIPAM struct { nAllocate int nFree int @@ -144,6 +151,19 @@ func (ns *mockNATSetup) Hook(gwnets []GWNets, _ *uberzap.Logger) func(ipv4, ipv6 } var _ = Describe("Coild server", func() { + + testIPAMEnv := os.Getenv(testIPAMKey) + testIPAM := true + if testIPAMEnv != "" && testIPAMEnv != "true" { + testIPAM = false + } + + testEgressEnv := os.Getenv(testEgressKey) + testEgress := true + if testEgressEnv != "" && testEgressEnv != "true" { + testEgress = false + } + tmpFile, err := os.CreateTemp("", "") if err != nil { panic(err) @@ -183,7 +203,26 @@ var _ = Describe("Coild server", func() { natsetup = &mockNATSetup{} logbuf = &bytes.Buffer{} logger := zap.NewRaw(zap.WriteTo(logbuf), zap.StacktraceLevel(zapcore.DPanicLevel)) - serv := NewCoildServer(l, mgr, nodeIPAM, podNet, natsetup, logger) + cfg := &config.Config{ + MetricsAddr: constants.DefautlMetricsAddr, + HealthAddr: constants.DefautlMetricsAddr, + PodTableId: constants.DefautlPodTableId, + PodRulePrio: constants.DefautlPodRulePrio, + ExportTableId: constants.DefautlExportTableId, + ProtocolId: constants.DefautlProtocolId, + SocketPath: constants.DefaultSocketPath, + CompatCalico: constants.DefaultCompatCalico, + EgressPort: constants.DefaultEgressPort, + RegisterFromMain: constants.DefaultRegisterFromMain, + EnableIPAM: testIPAM, + EnableEgress: testEgress, + } + + af := func(interfaces map[string]bool, conf *nodenet.PodNetConf, logger *uberzap.Logger, pod *corev1.Pod) error { + return nil + } + + serv := NewCoildServer(l, mgr, nodeIPAM, podNet, natsetup, cfg, logger, af) err = mgr.Add(serv) Expect(err).ToNot(HaveOccurred()) @@ -259,231 +298,244 @@ var _ = Describe("Coild server", func() { ContainerId: "pod1", Ifname: "eth0", Netns: "/run/netns/foo", + Interfaces: map[string]bool{"eth0": false}, }) - Expect(err).NotTo(HaveOccurred()) - By("checking the result") - result := ¤t.Result{} - err = json.Unmarshal(data.Result, result) - Expect(err).NotTo(HaveOccurred()) - Expect(result.IPs).To(HaveLen(2)) - - By("checking custom tags in gRPC log") - // Expecting JSON output like: - // { - // "grpc.code": "OK", - // "grpc.method": "Add", - // "grpc.request.pod.name": "foo", - // "grpc.request.pod.namespace": "ns1", - // "grpc.service": "pkg.cnirpc.CNI", - // "grpc.start_time": "2020-08-30T10:18:49Z", - // "grpc.time_ms": 102.34100341796875, - // "level": "info", - // "msg": "finished unary call with code OK", - // "peer.address": "@", - // "span.kind": "server", - // "system": "grpc", - // "ts": 1598782729.4605289 - // } - logFields := struct { - Method string `json:"grpc.method"` - Netns string `json:"grpc.request.netns"` - ContainerId string `json:"grpc.request.container_id"` - Ifname string `json:"grpc.request.ifname"` - PodName string `json:"grpc.request.pod.name"` - PodNS string `json:"grpc.request.pod.namespace"` - }{} - err = json.Unmarshal(logbuf.Bytes(), &logFields) - Expect(err).ToNot(HaveOccurred()) - Expect(logFields.Method).To(Equal("Add")) - Expect(logFields.Netns).To(Equal("/run/netns/foo")) - Expect(logFields.ContainerId).To(Equal("pod1")) - Expect(logFields.Ifname).To(Equal("eth0")) - Expect(logFields.PodName).To(Equal("foo")) - Expect(logFields.PodNS).To(Equal("ns1")) - - By("checking metrics for gRPC") - resp, err := http.Get("http://localhost:13449/metrics") - Expect(err).NotTo(HaveOccurred()) - defer resp.Body.Close() - mfs, err := (&expfmt.TextParser{}).TextToMetricFamilies(resp.Body) - Expect(err).NotTo(HaveOccurred()) - Expect(mfs).To(HaveKey("grpc_server_handled_total")) - mf := mfs["grpc_server_handled_total"] - metric := findMetric(mf, map[string]string{ - "grpc_method": "Add", - "grpc_code": "OK", - }) - Expect(metric).NotTo(BeNil()) - Expect(metric.GetCounter().GetValue()).To(BeNumerically("==", 1)) - - By("creating a pod in ns2") - pod = &corev1.Pod{} - pod.Namespace = "ns2" - pod.Name = "bar" - pod.Spec.Containers = []corev1.Container{ - {Name: "nginx", Image: "nginx"}, + if testIPAM || testEgress { + Expect(err).NotTo(HaveOccurred()) + } else { + Expect(err).To(HaveOccurred()) } - err = k8sClient.Create(ctx, pod) - Expect(err).NotTo(HaveOccurred()) - By("calling Add expecting a temporary failure") - podNet.errSetup = true - _, err = cniClient.Add(ctx, &cnirpc.CNIArgs{ - Args: map[string]string{"K8S_POD_NAME": "bar", "K8S_POD_NAMESPACE": "ns2"}, - ContainerId: "dns1", - Ifname: "eth0", - Netns: "/run/netns/bar", - }) - Expect(err).To(HaveOccurred()) - - By("calling Add for ns2/bar") - podNet.errSetup = false - data, err = cniClient.Add(ctx, &cnirpc.CNIArgs{ - Args: map[string]string{"K8S_POD_NAME": "bar", "K8S_POD_NAMESPACE": "ns2"}, - ContainerId: "dns1", - Ifname: "eth0", - Netns: "/run/netns/bar", - }) - Expect(err).NotTo(HaveOccurred()) - - By("checking the result") - result = ¤t.Result{} - err = json.Unmarshal(data.Result, result) - Expect(err).NotTo(HaveOccurred()) - Expect(result.IPs).To(HaveLen(1)) - Expect(result.IPs[0].Address.IP.To4()).To(Equal(net.ParseIP("8.8.8.8").To4())) - - By("creating another pod in ns1") - pod = &corev1.Pod{} - pod.Namespace = "ns1" - pod.Name = "zot" - pod.Spec.Containers = []corev1.Container{ - {Name: "nginx", Image: "nginx"}, + result := ¤t.Result{} + if testIPAM { + By("checking the result") + err = json.Unmarshal(data.Result, result) + Expect(err).NotTo(HaveOccurred()) + Expect(result.IPs).To(HaveLen(2)) } - err = k8sClient.Create(ctx, pod) - Expect(err).NotTo(HaveOccurred()) - - By("calling Add to fail for Allocation failure") - _, err = cniClient.Add(ctx, &cnirpc.CNIArgs{ - Args: map[string]string{"K8S_POD_NAME": "zot", "K8S_POD_NAMESPACE": "ns1"}, - ContainerId: "hoge", - Ifname: "eth0", - Netns: "/run/netns/zot", - }) - Expect(err).To(HaveOccurred()) - By("calling Check") - _, err = cniClient.Check(ctx, &cnirpc.CNIArgs{ - Args: map[string]string{"K8S_POD_NAME": "bar", "K8S_POD_NAMESPACE": "ns2"}, - ContainerId: "dns1", - Ifname: "eth0", - Netns: "/run/netns/bar", - }) - Expect(err).NotTo(HaveOccurred()) - _, err = cniClient.Check(ctx, &cnirpc.CNIArgs{ - Args: map[string]string{"K8S_POD_NAME": "zot", "K8S_POD_NAMESPACE": "ns1"}, - ContainerId: "hoge", - Ifname: "eth0", - Netns: "/run/netns/zot", - }) - Expect(err).To(HaveOccurred()) - - By("calling Del") - _, err = cniClient.Del(ctx, &cnirpc.CNIArgs{ - Args: map[string]string{"K8S_POD_NAME": "bar", "K8S_POD_NAMESPACE": "ns2"}, - ContainerId: "dns1", - Ifname: "eth0", - Netns: "/run/netns/bar", - }) - Expect(err).NotTo(HaveOccurred()) - - nodeIPAM.errFree = true - _, err = cniClient.Del(ctx, &cnirpc.CNIArgs{ - Args: map[string]string{"K8S_POD_NAME": "bar", "K8S_POD_NAMESPACE": "ns2"}, - ContainerId: "dns1", - Ifname: "eth0", - Netns: "/run/netns/bar", - }) - Expect(err).To(HaveOccurred()) - - nodeIPAM.errFree = false - podNet.errDestroy = true - _, err = cniClient.Del(ctx, &cnirpc.CNIArgs{ - Args: map[string]string{"K8S_POD_NAME": "bar", "K8S_POD_NAMESPACE": "ns2"}, - ContainerId: "dns1", - Ifname: "eth0", - Netns: "/run/netns/bar", - }) - Expect(err).To(HaveOccurred()) - }) - - It("should setup Foo-over-UDP NAT", func() { - By("creating pod declaring itself as a NAT client") - pod := &corev1.Pod{} - pod.Namespace = "ns1" - pod.Name = "nat-client1" - pod.Spec.Containers = []corev1.Container{ - {Name: "foo", Image: "nginx"}, - } - pod.Annotations = map[string]string{ - "egress.coil.cybozu.com/ns2": "egress", + if testIPAM || testEgress { + By("checking custom tags in gRPC log") + // Expecting JSON output like: + // { + // "grpc.code": "OK", + // "grpc.method": "Add", + // "grpc.request.pod.name": "foo", + // "grpc.request.pod.namespace": "ns1", + // "grpc.service": "pkg.cnirpc.CNI", + // "grpc.start_time": "2020-08-30T10:18:49Z", + // "grpc.time_ms": 102.34100341796875, + // "level": "info", + // "msg": "finished unary call with code OK", + // "peer.address": "@", + // "span.kind": "server", + // "system": "grpc", + // "ts": 1598782729.4605289 + // } + logFields := struct { + Method string `json:"grpc.method"` + Netns string `json:"grpc.request.netns"` + ContainerId string `json:"grpc.request.container_id"` + Ifname string `json:"grpc.request.ifname"` + PodName string `json:"grpc.request.pod.name"` + PodNS string `json:"grpc.request.pod.namespace"` + }{} + err = json.Unmarshal(logbuf.Bytes(), &logFields) + Expect(err).ToNot(HaveOccurred()) + Expect(logFields.Method).To(Equal("Add")) + Expect(logFields.Netns).To(Equal("/run/netns/foo")) + Expect(logFields.ContainerId).To(Equal("pod1")) + Expect(logFields.Ifname).To(Equal("eth0")) + Expect(logFields.PodName).To(Equal("foo")) + Expect(logFields.PodNS).To(Equal("ns1")) + + By("checking metrics for gRPC") + resp, err := http.Get("http://localhost:13449/metrics") + Expect(err).NotTo(HaveOccurred()) + defer resp.Body.Close() + mfs, err := (&expfmt.TextParser{}).TextToMetricFamilies(resp.Body) + Expect(err).NotTo(HaveOccurred()) + Expect(mfs).To(HaveKey("grpc_server_handled_total")) + mf := mfs["grpc_server_handled_total"] + metric := findMetric(mf, map[string]string{ + "grpc_method": "Add", + "grpc_code": "OK", + }) + Expect(metric).NotTo(BeNil()) + Expect(metric.GetCounter().GetValue()).To(BeNumerically("==", 1)) + + By("creating a pod in ns2") + pod = &corev1.Pod{} + pod.Namespace = "ns2" + pod.Name = "bar" + pod.Spec.Containers = []corev1.Container{ + {Name: "nginx", Image: "nginx"}, + } + err = k8sClient.Create(ctx, pod) + Expect(err).NotTo(HaveOccurred()) + + By("calling Add expecting a temporary failure") + podNet.errSetup = true + _, err = cniClient.Add(ctx, &cnirpc.CNIArgs{ + Args: map[string]string{"K8S_POD_NAME": "bar", "K8S_POD_NAMESPACE": "ns2"}, + ContainerId: "dns1", + Ifname: "eth0", + Netns: "/run/netns/bar", + }) + Expect(err).To(HaveOccurred()) + + By("calling Add for ns2/bar") + podNet.errSetup = false + data, err = cniClient.Add(ctx, &cnirpc.CNIArgs{ + Args: map[string]string{"K8S_POD_NAME": "bar", "K8S_POD_NAMESPACE": "ns2"}, + ContainerId: "dns1", + Ifname: "eth0", + Netns: "/run/netns/bar", + }) + Expect(err).NotTo(HaveOccurred()) + + By("checking the result") + result = ¤t.Result{} + err = json.Unmarshal(data.Result, result) + Expect(err).NotTo(HaveOccurred()) + Expect(result.IPs).To(HaveLen(1)) + Expect(result.IPs[0].Address.IP.To4()).To(Equal(net.ParseIP("8.8.8.8").To4())) + + By("creating another pod in ns1") + pod = &corev1.Pod{} + pod.Namespace = "ns1" + pod.Name = "zot" + pod.Spec.Containers = []corev1.Container{ + {Name: "nginx", Image: "nginx"}, + } + err = k8sClient.Create(ctx, pod) + Expect(err).NotTo(HaveOccurred()) + + By("calling Add to fail for Allocation failure") + _, err = cniClient.Add(ctx, &cnirpc.CNIArgs{ + Args: map[string]string{"K8S_POD_NAME": "zot", "K8S_POD_NAMESPACE": "ns1"}, + ContainerId: "hoge", + Ifname: "eth0", + Netns: "/run/netns/zot", + }) + Expect(err).To(HaveOccurred()) + + By("calling Check") + _, err = cniClient.Check(ctx, &cnirpc.CNIArgs{ + Args: map[string]string{"K8S_POD_NAME": "bar", "K8S_POD_NAMESPACE": "ns2"}, + ContainerId: "dns1", + Ifname: "eth0", + Netns: "/run/netns/bar", + }) + Expect(err).NotTo(HaveOccurred()) + _, err = cniClient.Check(ctx, &cnirpc.CNIArgs{ + Args: map[string]string{"K8S_POD_NAME": "zot", "K8S_POD_NAMESPACE": "ns1"}, + ContainerId: "hoge", + Ifname: "eth0", + Netns: "/run/netns/zot", + }) + Expect(err).To(HaveOccurred()) + + By("calling Del") + _, err = cniClient.Del(ctx, &cnirpc.CNIArgs{ + Args: map[string]string{"K8S_POD_NAME": "bar", "K8S_POD_NAMESPACE": "ns2"}, + ContainerId: "dns1", + Ifname: "eth0", + Netns: "/run/netns/bar", + }) + Expect(err).NotTo(HaveOccurred()) + + nodeIPAM.errFree = true + _, err = cniClient.Del(ctx, &cnirpc.CNIArgs{ + Args: map[string]string{"K8S_POD_NAME": "bar", "K8S_POD_NAMESPACE": "ns2"}, + ContainerId: "dns1", + Ifname: "eth0", + Netns: "/run/netns/bar", + }) + Expect(err).To(HaveOccurred()) + + nodeIPAM.errFree = false + podNet.errDestroy = true + _, err = cniClient.Del(ctx, &cnirpc.CNIArgs{ + Args: map[string]string{"K8S_POD_NAME": "bar", "K8S_POD_NAMESPACE": "ns2"}, + ContainerId: "dns1", + Ifname: "eth0", + Netns: "/run/netns/bar", + }) + Expect(err).To(HaveOccurred()) } - err = k8sClient.Create(ctx, pod) - Expect(err).NotTo(HaveOccurred()) - - By("calling Add without Egress/Service") - _, err := cniClient.Add(ctx, &cnirpc.CNIArgs{ - Args: map[string]string{"K8S_POD_NAME": "nat-client1", "K8S_POD_NAMESPACE": "ns1"}, - ContainerId: "nat-client1", - Ifname: "eth0", - Netns: "/run/netns/nat-client1", - }) - Expect(err).To(HaveOccurred()) - - eg := &coilv2.Egress{} - eg.Namespace = "ns2" - eg.Name = "egress" - eg.Spec.Destinations = []string{"192.168.0.0/16", "fd20::/112"} - eg.Spec.Replicas = 1 - err = k8sClient.Create(ctx, eg) - Expect(err).NotTo(HaveOccurred()) - - By("calling Add without Service") - _, err = cniClient.Add(ctx, &cnirpc.CNIArgs{ - Args: map[string]string{"K8S_POD_NAME": "nat-client1", "K8S_POD_NAMESPACE": "ns1"}, - ContainerId: "nat-client1", - Ifname: "eth0", - Netns: "/run/netns/nat-client1", - }) - Expect(err).To(HaveOccurred()) - - svc := &corev1.Service{} - svc.Namespace = "ns2" - svc.Name = "egress" - // currently, ClusterIP must be picked from 10.0.0.0/24 - // see https://github.com/kubernetes/kubernetes/pull/51249 - svc.Spec.ClusterIP = "10.0.0.5" - svc.Spec.Ports = []corev1.ServicePort{{Port: 8080}} - err = k8sClient.Create(ctx, svc) - Expect(err).NotTo(HaveOccurred()) - time.Sleep(100 * time.Millisecond) + }) - By("calling Add with IPv4 Service") - _, err = cniClient.Add(ctx, &cnirpc.CNIArgs{ - Args: map[string]string{"K8S_POD_NAME": "nat-client1", "K8S_POD_NAMESPACE": "ns1"}, - ContainerId: "nat-client1", - Ifname: "eth0", - Netns: "/run/netns/nat-client1", + if testEgress { + It("should setup Foo-over-UDP NAT", func() { + By("creating pod declaring itself as a NAT client") + pod := &corev1.Pod{} + pod.Namespace = "ns1" + pod.Name = "nat-client1" + pod.Spec.Containers = []corev1.Container{ + {Name: "foo", Image: "nginx"}, + } + pod.Annotations = map[string]string{ + "egress.coil.cybozu.com/ns2": "egress", + } + err = k8sClient.Create(ctx, pod) + Expect(err).NotTo(HaveOccurred()) + + By("calling Add without Egress/Service") + _, err := cniClient.Add(ctx, &cnirpc.CNIArgs{ + Args: map[string]string{"K8S_POD_NAME": "nat-client1", "K8S_POD_NAMESPACE": "ns1"}, + ContainerId: "nat-client1", + Ifname: "eth0", + Netns: "/run/netns/nat-client1", + }) + Expect(err).To(HaveOccurred()) + + eg := &coilv2.Egress{} + eg.Namespace = "ns2" + eg.Name = "egress" + eg.Spec.Destinations = []string{"192.168.0.0/16", "fd20::/112"} + eg.Spec.Replicas = 1 + err = k8sClient.Create(ctx, eg) + Expect(err).NotTo(HaveOccurred()) + + By("calling Add without Service") + _, err = cniClient.Add(ctx, &cnirpc.CNIArgs{ + Args: map[string]string{"K8S_POD_NAME": "nat-client1", "K8S_POD_NAMESPACE": "ns1"}, + ContainerId: "nat-client1", + Ifname: "eth0", + Netns: "/run/netns/nat-client1", + }) + Expect(err).To(HaveOccurred()) + + svc := &corev1.Service{} + svc.Namespace = "ns2" + svc.Name = "egress" + // currently, ClusterIP must be picked from 10.0.0.0/24 + // see https://github.com/kubernetes/kubernetes/pull/51249 + svc.Spec.ClusterIP = "10.0.0.5" + svc.Spec.Ports = []corev1.ServicePort{{Port: 8080}} + err = k8sClient.Create(ctx, svc) + Expect(err).NotTo(HaveOccurred()) + time.Sleep(100 * time.Millisecond) + + By("calling Add with IPv4 Service") + _, err = cniClient.Add(ctx, &cnirpc.CNIArgs{ + Args: map[string]string{"K8S_POD_NAME": "nat-client1", "K8S_POD_NAMESPACE": "ns1"}, + ContainerId: "nat-client1", + Ifname: "eth0", + Netns: "/run/netns/nat-client1", + Interfaces: map[string]bool{"eth0": false}, + }) + Expect(err).NotTo(HaveOccurred()) + + By("checking NAT configurations") + Expect(natsetup.gwnets).To(HaveLen(1)) + gwnets := natsetup.gwnets[0] + Expect(gwnets.Gateway.Equal(net.ParseIP("10.0.0.5"))).To(BeTrue()) + Expect(gwnets.Networks).To(HaveLen(1)) + subnet := gwnets.Networks[0] + Expect(subnet.IP.Equal(net.ParseIP("192.168.0.0"))).To(BeTrue()) }) - Expect(err).NotTo(HaveOccurred()) - - By("checking NAT configurations") - Expect(natsetup.gwnets).To(HaveLen(1)) - gwnets := natsetup.gwnets[0] - Expect(gwnets.Gateway.Equal(net.ParseIP("10.0.0.5"))).To(BeTrue()) - Expect(gwnets.Networks).To(HaveLen(1)) - subnet := gwnets.Networks[0] - Expect(subnet.IP.Equal(net.ParseIP("192.168.0.0"))).To(BeTrue()) - }) + } })