31 Commits

Author SHA1 Message Date
Manuel Rüger
1d37130447 Fix linting 2022-10-17 11:37:07 -05:00
Aaron U'Ren
6dbd3a1d6d fix(NSC): don't check protocol on DSR svcs
DSR IPVS inherently don't have a protocol, so don't check for protocol
with these services and ensure that they are cleaned up correctly.

Fixes #1328
2022-07-29 17:19:07 -05:00
Aaron U'Ren
4b6cf6c896 fact(protocol): standardize protocol conversions 2022-02-11 17:34:10 -06:00
Aaron U'Ren
28aab6ea20 fact(service_endpoints_sync): simplify external IP logic
This is an attempt to make the external IP logic easier to follow and
more straight forward for future changes like consolidating the iptables
logic.
2022-02-11 17:34:10 -06:00
Aaron U'Ren
5101a4fe81 fix(nsc): remove error for lookupFWMarkByService
lookupFWMarkByService() was previous returning an error when no fwmark
was found in the tracking map for a given service. However, this isn't
really an error condition and shouldn't be treated as such. When it was
treated as an error condition users got a lot of confusing errors in the
logs.
2021-12-03 11:49:28 +01:00
Aaron U'Ren
c3f90c54b3
Fix Misc DSR Issues (#1174)
* fact(NSC): consolidate constants to top

* fix(NSC): increase IPVS add service logging

* fix(NSC): improve logging for FWMark IPVS entries

* fix(NSC): add missing parameter to logging

* feat(NSC): generate unique FW marks

Because we trim the 32-bit FNV-1a hash to 16 bits there is the potential
for FW marks to collide with each other even for unique inputs of IP,
protocol, and port. This reduces that chance up to the 16-bit max by
keeping track of which FW marks we've already allocated and what IP,
protocol, port combo they've been allocated for.

Fixes #1045

* fact(NSC): move utility funcs to utils

* fix(NSC): reduce IPVS service shell outs

This also aligns it more with the almost identical function used for
non-FWmarked services ipvsAddService() which is also called from
setupExternalIPServices and passes in this same list of ipvsServices.

* fix(NSC): fix & consolidate DSR cleanup code

A lot of this is refactor work, but its important to know why the DSR
mangle tables were not being cleaned up in the first place. When we
transitioned to iptables-save to look over the mangle rules, we didn't
realize that iptables-save changes the format of the marks from integer
values (which is what the CLI works with) to hexadecimal.

This made it so that we were never actually matching on a mangle rule,
which left them all behind. When these mangle rules were left, it meant
that IPs that used to be part of a DSR service were essentially
black-holed on the system and were no longer route-able.

Fixes #1167

* doc(dsr): expand DSR documentation

fixes #1055

* ensure active service map is updated for non DSR services

Co-authored-by: Murali Reddy <muralimmreddy@gmail.com>
2021-10-14 16:14:05 +05:30
Aaron U'Ren
85f28411dc feat(.golangci.yml): enable long lines linter and remediate 2021-09-11 16:20:07 -05:00
Aaron U'Ren
6208bfac46 feat(.golangci.yml): enable gomnd and remediate 2021-09-11 16:20:07 -05:00
Aaron U'Ren
f52fddddee feat(.golangci.yml): enable gocritic and remediate 2021-09-11 16:20:07 -05:00
Aaron U'Ren
d6ccc22519 feat(.golangci.yml): enable goconst and remediate 2021-09-11 16:20:07 -05:00
Aaron U'Ren
c5f4c00d63 feat(.golangci.yml): enable dupl and remediate 2021-09-11 16:20:07 -05:00
Billie Cleek
d5a18cac67
remove IPVS metrics (#1133)
* remove IPVS metrics

Remove metrics for IPVS services when the IPVS service is deleted so
that the number of metrics does not grow without bound.

Fixes #734

* delete metricsMap key when IPVS service is removed

Delete the key in NetworkServicesController.metricsMap when the
respective IPVS configuration is removed.

Remove a period from a comment to conform to kube-router norms

* cleanup stale metrics in a distinct method

* remove unnecessary error return value on cleanupStaleMetrics
2021-07-31 01:25:58 +05:30
Aaron U'Ren
4306e5d47c feat(DSR): make TCPMSS based on primary link MTU 2021-05-17 16:33:15 -05:00
Aaron U'Ren
be01f317c7 fact: other misc cleanups 2021-04-14 16:23:59 -05:00
Aaron U'Ren
53cfbe30eb fix: return early when we might be holding nil references 2021-04-14 16:23:59 -05:00
Aaron U'Ren
4efa5ccc48 fact: remove function parameters that are never referenced 2021-04-14 16:23:59 -05:00
Aaron U'Ren
96675e620b fix: don't capitalize error messages
It is standard practice in Go to not capitalize error messages:
https://github.com/golang/go/wiki/CodeReviewComments#error-strings
2021-04-14 16:23:59 -05:00
Manuel Rüger
7d47aefe7d Replace github.com/golang/glog with k8s.io/klog/v2
glog is effectively unmaintained and the kubernetes ecosystem is mainly
using its fork klog

Fixes: #1051
2021-04-11 13:16:03 -05:00
Murali Reddy
d1e1923b63 prevent iptable command calls when necessary rules already exists 2021-03-18 09:21:22 -05:00
ep4eg
ca2008e576
feat: simple CRI implementation in addition to Docker, required for DSR functionality. CRI compliant runtimes support (e.g. containerd, cri-o, etc.) (#1027)
* feat: simple CRI implementation in addition to Docker, required for DSR functionality. CRI compliant runtimes support (e.g. containerd, cri-o, etc.)

* upd: dependencies

* cleanup

* feat: cleanup gRPC connections after we did the job

* upd: go.sum
2021-02-08 20:04:13 +05:30
Murali Reddy
3c734fb96a
merge gobgp-update into master (#982)
* merge gobgp-update into master

* update travis.yaml go version:

* go get github.com/osrg/gobgp to build gobgp

* install git as go get needs it
2020-09-07 10:27:58 +05:30
Qingkun Li
7613a735de
add IfaceHasNoAddr check for external ip delete error (#971) 2020-08-14 19:41:09 +05:30
Murali Reddy
a33089d292
[testing] run go linters (#943)
* run go linters for static code checking

* fix(lint): fix all goimports linting errors

* fix(lint): fix all golint errors

* fix(lint): fix all spelling errors

Co-authored-by: Aaron U'Ren <aauren@gmail.com>
2020-07-28 23:52:41 +05:30
eta
a2ac2f0054 fix unintentional Sprint of two-argument generateFwmark() call 2020-06-14 15:02:32 +01:00
Manuel Rüger
12674d5f8b
Add golangci-lint support (#895)
* Makefile: Add lint using golangci-lint

* build/travis-test.sh: Run lint step

* metrics_controller: Lint

pkg/metrics/metrics_controller.go:150:2: `mu` is unused (structcheck)
        mu          sync.Mutex
        ^
pkg/metrics/metrics_controller.go:151:2: `nodeIP` is unused (structcheck)
        nodeIP      net.IP
        ^

* network_service_graceful: Lint

pkg/controllers/proxy/network_service_graceful.go:21:6: `gracefulQueueItem` is unused (deadcode)
type gracefulQueueItem struct {
     ^
pkg/controllers/proxy/network_service_graceful.go:22:2: `added` is unused (structcheck)
        added   time.Time
        ^
pkg/controllers/proxy/network_service_graceful.go:23:2: `service` is unused (structcheck)
        service *ipvs.Service
        ^

* network_services_controller_test: Lint

pkg/controllers/proxy/network_services_controller_test.go:80:6: func `logf` is unused (unused)

* ecmp_vip: Lint

pkg/controllers/routing/ecmp_vip.go:208:4: S1023: redundant `return` statement (gosimple)
                        return
                        ^

* bgp_peers: Lint

pkg/controllers/routing/bgp_peers.go:331:4: S1023: redundant `return` statement (gosimple)
                        return
                        ^

* bgp_policies: Lint

pkg/controllers/routing/bgp_policies.go:80:3: S1011: should replace loop with `externalBgpPeers = append(externalBgpPeers, nrc.nodePeerRouters...)` (gosimple)
                for _, peer := range nrc.nodePeerRouters {
                ^
pkg/controllers/routing/bgp_policies.go:23:20: ineffectual assignment to `err` (ineffassign)
        podCidrPrefixSet, err := table.NewPrefixSet(config.PrefixSet{
                          ^
pkg/controllers/routing/bgp_policies.go:42:22: ineffectual assignment to `err` (ineffassign)
        clusterIPPrefixSet, err := table.NewPrefixSet(config.PrefixSet{
                            ^
pkg/controllers/routing/bgp_policies.go:33:30: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck)
                nrc.bgpServer.AddDefinedSet(podCidrPrefixSet)
                                           ^
pkg/controllers/routing/bgp_policies.go:48:30: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck)
                nrc.bgpServer.AddDefinedSet(clusterIPPrefixSet)
                                           ^
pkg/controllers/routing/bgp_policies.go:69:31: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck)
                        nrc.bgpServer.AddDefinedSet(iBGPPeerNS)
                                                   ^
pkg/controllers/routing/bgp_policies.go:108:31: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck)
                        nrc.bgpServer.AddDefinedSet(ns)
                                                   ^
pkg/controllers/routing/bgp_policies.go:120:30: Error return value of `nrc.bgpServer.AddDefinedSet` is not checked (errcheck)
                nrc.bgpServer.AddDefinedSet(ns)
                                           ^
                                                   ^

* network_policy_controller: Lint

pkg/controllers/netpol/network_policy_controller.go:35:2: `networkPolicyAnnotation` is unused (deadcode)
        networkPolicyAnnotation      = "net.beta.kubernetes.io/network-policy"
        ^
pkg/controllers/netpol/network_policy_controller.go:1047:4: SA9003: empty branch (staticcheck)
                        if err != nil {
                        ^
pkg/controllers/netpol/network_policy_controller.go:969:10: SA4006: this value of `err` is never used (staticcheck)
        chains, err := iptablesCmdHandler.ListChains("filter")
                ^
pkg/controllers/netpol/network_policy_controller.go:1568:4: SA4006: this value of `err` is never used (staticcheck)
                        err = iptablesCmdHandler.Delete("filter", "FORWARD", strconv.Itoa(i-realRuleNo))
                        ^
pkg/controllers/netpol/network_policy_controller.go:1584:4: SA4006: this value of `err` is never used (staticcheck)
                        err = iptablesCmdHandler.Delete("filter", "OUTPUT", strconv.Itoa(i-realRuleNo))
                        ^

* network_services_controller: Lint

pkg/controllers/proxy/network_services_controller.go:66:2: `h` is unused (deadcode)
        h      *ipvs.Handle
        ^
pkg/controllers/proxy/network_services_controller.go:879:23: SA1019: client.NewEnvClient is deprecated: use NewClientWithOpts(FromEnv)  (staticcheck)
        dockerClient, err := client.NewEnvClient()
                             ^
pkg/controllers/proxy/network_services_controller.go:944:5: unreachable: unreachable code (govet)
                                glog.V(3).Infof("Waiting for tunnel interface %s to come up in the pod, retrying", KUBE_TUNNEL_IF)
                                ^
pkg/controllers/proxy/network_services_controller.go:1289:5: S1002: should omit comparison to bool constant, can be simplified to `!hasHairpinChain` (gosimple)
        if hasHairpinChain != true {
           ^
pkg/controllers/proxy/network_services_controller.go:1237:43: S1019: should use make(map[string][]string) instead (gosimple)
        rulesNeeded := make(map[string][]string, 0)
                                                 ^
pkg/controllers/proxy/network_services_controller.go:1111:4: S1023: redundant break statement (gosimple)
                        break
                        ^
pkg/controllers/proxy/network_services_controller.go:1114:4: S1023: redundant break statement (gosimple)
                        break
                        ^
pkg/controllers/proxy/network_services_controller.go:1117:4: S1023: redundant break statement (gosimple)
                        break
                        ^
pkg/controllers/proxy/network_services_controller.go:445:21: Error return value of `nsc.publishMetrics` is not checked (errcheck)
                nsc.publishMetrics(nsc.serviceMap)
                                  ^
pkg/controllers/proxy/network_services_controller.go:1609:9: Error return value of `h.Write` is not checked (errcheck)
        h.Write([]byte(ip + "-" + protocol + "-" + port))
               ^
pkg/controllers/proxy/network_services_controller.go:912:13: Error return value of `netns.Set` is not checked (errcheck)
                        netns.Set(hostNetworkNamespaceHandle)
                                 ^
pkg/controllers/proxy/network_services_controller.go:926:13: Error return value of `netns.Set` is not checked (errcheck)
                        netns.Set(hostNetworkNamespaceHandle)
                                 ^
pkg/controllers/proxy/network_services_controller.go:950:13: Error return value of `netns.Set` is not checked (errcheck)
                        netns.Set(hostNetworkNamespaceHandle)
                                 ^
pkg/controllers/proxy/network_services_controller.go:641:9: SA4006: this value of `err` is never used (staticcheck)
        addrs, err := getAllLocalIPs()
               ^

* network_routes_controller: Lint

pkg/controllers/routing/network_routes_controller.go:340:2: S1000: should use for range instead of for { select {} } (gosimple)
        for {
        ^
pkg/controllers/routing/network_routes_controller.go:757:22: Error return value of `nrc.bgpServer.Stop` is not checked (errcheck)
                        nrc.bgpServer.Stop()
                                          ^
pkg/controllers/routing/network_routes_controller.go:770:22: Error return value of `nrc.bgpServer.Stop` is not checked (errcheck)
                        nrc.bgpServer.Stop()
                                          ^
pkg/controllers/routing/network_routes_controller.go:782:23: Error return value of `nrc.bgpServer.Stop` is not checked (errcheck)
                                nrc.bgpServer.Stop()
                                                  ^
pkg/controllers/routing/network_routes_controller.go:717:12: Error return value of `g.Serve` is not checked (errcheck)
        go g.Serve()

* ipset: Lint

pkg/utils/ipset.go:243:23: Error return value of `entry.Set.Parent.Save` is not checked (errcheck)
        entry.Set.Parent.Save()
                             ^

* pkg/cmd/kube-router: Lint

pkg/cmd/kube-router.go:214:26: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck)
                fmt.Fprintf(os.Stderr, output)
                                       ^
pkg/cmd/kube-router.go:184:15: SA1017: the channel used with signal.Notify should be buffered (staticcheck)
        signal.Notify(ch, syscall.SIGINT, syscall.SIGTERM)
                     ^
pkg/cmd/kube-router.go:94:17: Error return value of `hc.RunServer` is not checked (errcheck)
        go hc.RunServer(stopCh, &wg)
                       ^
pkg/cmd/kube-router.go:112:16: Error return value of `hc.RunCheck` is not checked (errcheck)
        go hc.RunCheck(healthChan, stopCh, &wg)
                      ^
pkg/cmd/kube-router.go:121:12: Error return value of `mc.Run` is not checked (errcheck)
                go mc.Run(healthChan, stopCh, &wg)
                         ^

* cmd/kube-router/kube-router: Lint

cmd/kube-router/kube-router.go:31:24: Error return value of `flag.CommandLine.Parse` is not checked (errcheck)
        flag.CommandLine.Parse([]string{})
                              ^
cmd/kube-router/kube-router.go:33:10: Error return value of `flag.Set` is not checked (errcheck)
        flag.Set("logtostderr", "true")
                ^
cmd/kube-router/kube-router.go:34:10: Error return value of `flag.Set` is not checked (errcheck)
        flag.Set("v", config.VLevel)
                ^
cmd/kube-router/kube-router.go:62:27: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck)
                        fmt.Fprintf(os.Stdout, http.ListenAndServe("0.0.0.0:6060", nil).Error())
                                               ^

* kube-router_test: Lint

cmd/kube-router/kube-router_test.go:21:10: Error return value of `io.Copy` is not checked (errcheck)
                io.Copy(stderrBuf, stderrR)
                       ^
cmd/kube-router/kube-router_test.go:40:17: Error return value of `docBuf.ReadFrom` is not checked (errcheck)
        docBuf.ReadFrom(docF)
                       ^

* service_endpoints_sync: Lint

pkg/controllers/proxy/service_endpoints_sync.go:460:2: ineffectual assignment to `ipvsSvcs` (ineffassign)
        ipvsSvcs, err := nsc.ln.ipvsGetServices()
        ^
pkg/controllers/proxy/service_endpoints_sync.go:311:5: SA4006: this value of `err` is never used (staticcheck)
                                err = nsc.ln.ipAddrDel(dummyVipInterface, externalIP)
                                ^

* node: Lint

pkg/utils/node.go:19:16: SA1019: clientset.Core is deprecated: please explicitly pick a version if possible.  (staticcheck)
                node, err := clientset.Core().Nodes().Get(nodeName, metav1.GetOptions{})
                             ^
pkg/utils/node.go:27:15: SA1019: clientset.Core is deprecated: please explicitly pick a version if possible.  (staticcheck)
        node, err := clientset.Core().Nodes().Get(hostName, metav1.GetOptions{})
                     ^
pkg/utils/node.go:34:15: SA1019: clientset.Core is deprecated: please explicitly pick a version if possible.  (staticcheck)
                node, err = clientset.Core().Nodes().Get(hostnameOverride, metav1.GetOptions{})
                            ^

* aws: Lint

pkg/controllers/routing/aws.go:31:8: SA4006: this value of `err` is never used (staticcheck)
                URL, err := url.Parse(providerID)
                     ^

* health_controller: Lint

pkg/healthcheck/health_controller.go:54:10: Error return value of `w.Write` is not checked (errcheck)
                w.Write([]byte("OK\n"))
                       ^
pkg/healthcheck/health_controller.go:68:10: Error return value of `w.Write` is not checked (errcheck)
                w.Write([]byte("Unhealthy"))
                       ^
pkg/healthcheck/health_controller.go:159:2: S1000: should use a simple channel send/receive instead of `select` with a single case (gosimple)
        select {
        ^

* network_routes_controller_test: Lint

pkg/controllers/routing/network_routes_controller_test.go:1113:37: Error return value of `testcase.nrc.bgpServer.Stop` is not checked (errcheck)
                        defer testcase.nrc.bgpServer.Stop()
                                                         ^
pkg/controllers/routing/network_routes_controller_test.go:1314:37: Error return value of `testcase.nrc.bgpServer.Stop` is not checked (errcheck)
                        defer testcase.nrc.bgpServer.Stop()
                                                         ^
pkg/controllers/routing/network_routes_controller_test.go:2327:37: Error return value of `testcase.nrc.bgpServer.Stop` is not checked (errcheck)
                        defer testcase.nrc.bgpServer.Stop()
                                                         ^

* .golangci.yml: Increase timeout

Default is 1m, increase to 5m otherwise travis might fail

* Makefile: Update golangci-lint to 1.27.0

* kube-router_test.go: defer waitgroup

Co-authored-by: Aaron U'Ren <aauren@users.noreply.github.com>

* network_routes_controller: Incorporate review

* bgp_policies: Incorporate review

* network_routes_controller: Incorporate review

* bgp_policies: Log error instead

* network_services_controller: Incorporate review

Co-authored-by: Aaron U'Ren <aauren@users.noreply.github.com>
2020-06-03 22:29:06 +02:00
bumyongchoi
f5db29e36d
honor the ClientIP session affinity timeout when set. (#882)
* honor the ClientIP session affinity timeout

* update moq file

* Fix unit test failure due to adding a new arg to ipvsAddService

Co-authored-by: Bumyong Choi <bchoi@digitalocean.com>
2020-04-23 17:10:30 +05:30
Murali Reddy
0857436ec3
use endpoint (IP, port) tuple to track active endpoints of a service in use. Currently only endpoint IP (#842)
used so any change in port of the endpoint leaves stale ipvs server config

Fixes #841
2020-03-02 23:28:14 +05:30
Arthur Outhenin-Chalandre
97c682e6f2
Ignore deletion of unknown IPVS rules (#830)
* add a --excluded-cidrs
* ignore deletion of ipvs rules with address in excluded cidrs

Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>
2020-02-17 01:39:28 +05:30
Murali Reddy
230ff155dd
restrict externalTrafficPolicy=Local interpretation only to NodePort and LoadBalancer services (#836)
* restrict externalTrafficPolicy=Local interpretation only to NodePort and LoadBalancer services

Fixes #818

* addressing review comments
2020-01-28 10:27:56 +05:30
Murali Reddy
f01a9a579e
Revert "restrict externalTrafficPolicy=Local interpretation only to NodePort and LoadBalancer services (#819)" (#835)
This reverts commit 27ec314e969fedc512de27ff1c519fa4186cf4ff.
2020-01-22 06:37:42 +05:30
Murali Reddy
27ec314e96
restrict externalTrafficPolicy=Local interpretation only to NodePort and LoadBalancer services (#819)
* restrict externalTrafficPolicy=Local interpretation only to NodePort and LoadBalancer services

Fixes #818

* refactoring service controller sync() logic to be more modular
2020-01-22 06:36:19 +05:30