Merge pull request #5370 from npinaeva/gateway-refactor

Gateway refactoring: preparation work for Layer2 with router

Author: npinaeva

go-controller/Makefile

@@ -93,9 +93,17 @@ clean:
 
 lint:
 ifeq ($(CONTAINER_RUNNABLE), 0)
-	@GOPATH=${GOPATH} ./hack/lint.sh $(CONTAINER_RUNTIME)
+	@GOPATH=${GOPATH} ./hack/lint.sh $(CONTAINER_RUNTIME) || { echo "lint failed! Try running 'make lint-fix'"; exit 1; }
 else
-	echo "linter can only be run within a container since it needs a specific golangci-lint version"
+	echo "linter can only be run within a container since it needs a specific golangci-lint version"; exit 1
+endif
+
+lint-fix:
+ifeq ($(CONTAINER_RUNNABLE), 0)
+	@GOPATH=${GOPATH} ./hack/lint.sh $(CONTAINER_RUNTIME) fix || { echo "ERROR: lint fix failed! There is a bug that changes file ownership to root \
+	when this happens. To fix it, simply run 'chown -R <user>:<group> *' from the repo root."; exit 1; }
+else
+	echo "linter can only be run within a container since it needs a specific golangci-lint version"; exit 1
 endif
 
 gofmt:

go-controller/hack/lint.sh

@@ -1,14 +1,18 @@
 #!/usr/bin/env bash
-
 VERSION=v1.60.3
+extra_flags=""
 if [ "$#" -ne 1 ]; then
+  if [ "$#" -eq 2 ] && [ "$2" == "fix" ]; then
+    extra_flags="--fix"
+  else
     echo "Expected command line argument - container runtime (docker/podman) got $# arguments: $@"
     exit 1
+  fi
 fi
 
 $1 run --security-opt label=disable --rm \
   -v  ${HOME}/.cache/golangci-lint:/cache -e GOLANGCI_LINT_CACHE=/cache \
   -v $(pwd):/app -w /app -e GO111MODULE=on docker.io/golangci/golangci-lint:${VERSION} \
 	golangci-lint run --verbose --print-resources-usage \
-	--modules-download-mode=vendor --timeout=15m0s && \
-	echo "lint OK!"
+	--modules-download-mode=vendor --timeout=15m0s ${extra_flags} && \
+	echo "lint OK!"

go-controller/pkg/libovsdb/ops/options.go

@@ -0,0 +1,18 @@
+package ops
+
+// This is a list of options used for OVN operations.
+// Started with adding only some of them, feel free to continue extending this list.
+// Eventually we expect to have no string options in the code.
+const (
+	// RequestedTnlKey can be used by LogicalSwitch, LogicalSwitchPort, LogicalRouter and LogicalRouterPort
+	// for distributed switches/routers
+	RequestedTnlKey = "requested-tnl-key"
+	// RequestedChassis can be used by LogicalSwitchPort and LogicalRouterPort.
+	// It specifies the chassis (by name or hostname) that is allowed to bind this port.
+	RequestedChassis = "requested-chassis"
+	// RouterPort can be used by LogicalSwitchPort to specify a connection to a logical router.
+	RouterPort = "router-port"
+	// GatewayMTU can be used by LogicalRouterPort to specify the MTU for the gateway port.
+	// If set, logical flows will be added to router pipeline to check packet length.
+	GatewayMTU = "gateway_mtu"
+)

go-controller/pkg/ovn/base_network_controller.go

@@ -425,7 +425,7 @@ func (bnc *BaseNetworkController) syncNodeClusterRouterPort(node *corev1.Node, h
 	enableGatewayMTU := util.ParseNodeGatewayMTUSupport(node)
 	if enableGatewayMTU {
 		lrpOptions = map[string]string{
-			"gateway_mtu": strconv.Itoa(config.Default.MTU),
+			libovsdbops.GatewayMTU: strconv.Itoa(config.Default.MTU),
 		}
 	}
 	logicalRouterPort := nbdb.LogicalRouterPort{
@@ -560,7 +560,7 @@ func (bnc *BaseNetworkController) createNodeLogicalSwitch(nodeName string, hostS
 		Type:      "router",
 		Addresses: []string{"router"},
 		Options: map[string]string{
-			"router-port": types.RouterToSwitchPrefix + switchName,
+			libovsdbops.RouterPort: types.RouterToSwitchPrefix + switchName,
 		},
 	}
 	if bnc.IsDefault() {

go-controller/pkg/ovn/base_network_controller_pods.go

@@ -535,7 +535,7 @@ func (bnc *BaseNetworkController) addLogicalPortToNetwork(pod *corev1.Pod, nadNa
 	// rescheduled.
 
 	if !config.Kubernetes.DisableRequestedChassis {
-		lsp.Options["requested-chassis"] = pod.Spec.NodeName
+		lsp.Options[libovsdbops.RequestedChassis] = pod.Spec.NodeName
 	}
 
 	// let's calculate if this network controller's role for this pod

go-controller/pkg/ovn/egressgw.go

@@ -649,30 +649,27 @@ func deletePodSNATOps(nbClient libovsdbclient.Client, ops []ovsdb.Operation, gwR
 // addOrUpdatePodSNAT adds or updates per pod SNAT rules towards the nodeIP that are applied to the GR where the pod resides
 // used when disableSNATMultipleGWs=true
 func addOrUpdatePodSNAT(nbClient libovsdbclient.Client, gwRouterName string, extIPs, podIfAddrs []*net.IPNet) error {
-	nats, err := buildPodSNAT(extIPs, podIfAddrs, "")
+	ops, err := addOrUpdatePodSNATOps(nbClient, gwRouterName, extIPs, podIfAddrs, nil)
 	if err != nil {
 		return err
 	}
-	logicalRouter := nbdb.LogicalRouter{
-		Name: gwRouterName,
-	}
-	if err := libovsdbops.CreateOrUpdateNATs(nbClient, &logicalRouter, nats...); err != nil {
-		return fmt.Errorf("failed to update SNAT for pods of router %s: %v", logicalRouter.Name, err)
+	if _, err = libovsdbops.TransactAndCheck(nbClient, ops); err != nil {
+		return fmt.Errorf("failed to update SNAT for pods of router %s: %v", gwRouterName, err)
 	}
 	return nil
 }
 
 // addOrUpdatePodSNATOps returns the operation that adds or updates per pod SNAT rules towards the nodeIP that are
 // applied to the GR where the pod resides
 // used when disableSNATMultipleGWs=true
-func addOrUpdatePodSNATOps(nbClient libovsdbclient.Client, gwRouterName string, extIPs, podIfAddrs []*net.IPNet, match string, ops []ovsdb.Operation) ([]ovsdb.Operation, error) {
-	router := &nbdb.LogicalRouter{Name: gwRouterName}
-	nats, err := buildPodSNAT(extIPs, podIfAddrs, match)
+func addOrUpdatePodSNATOps(nbClient libovsdbclient.Client, gwRouterName string, extIPs, podIfAddrs []*net.IPNet, ops []ovsdb.Operation) ([]ovsdb.Operation, error) {
+	gwRouter := &nbdb.LogicalRouter{Name: gwRouterName}
+	nats, err := buildPodSNAT(extIPs, podIfAddrs, "")
 	if err != nil {
 		return nil, err
 	}
-	if ops, err = libovsdbops.CreateOrUpdateNATsOps(nbClient, ops, router, nats...); err != nil {
-		return nil, fmt.Errorf("failed to update SNAT for pods of router: %s, error: %v", gwRouterName, err)
+	if ops, err = libovsdbops.CreateOrUpdateNATsOps(nbClient, ops, gwRouter, nats...); err != nil {
+		return nil, fmt.Errorf("failed to create ops to update SNAT for pods of router: %s, error: %v", gwRouterName, err)
 	}
 	return ops, nil
 }