Merge pull request #6 from p-strusiewiczsurmacki-mobica/standalone-eg…

…ress-settings-fix

Standalone egress settings fix

.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

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).
 

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

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{}

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)
-}

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

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",

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",

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",

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
+}

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