Trident: Trident Operator: unable to register trident node with CSI controller, causing PV mounts to fail.

[JosephGJ]

Describe the bug
Unable to mount PVs into pods, Kubernetes events for the pod shows:

Events:
  Type     Reason              Age               From                     Message
  ----     ------              ----              ----                     -------
  Normal   Scheduled           <unknown>         default-scheduler        Successfully assigned trident-pvc-test/task-pv-pod to gueabetk8sappworkertst95
  Warning  FailedAttachVolume  5s (x6 over 21s)  attachdetach-controller  AttachVolume.Attach failed for volume "pvc-d4b9e15a-197b-45b4-9024-683653f6a434" : rpc error: code = NotFound desc = node gueabetk8sappworkertst95 was not found

When deploying trident with the operator, after deploying the trident provisioner manifest, trident nodes are unable to register with the trident CSI controller. Requests to the controller are returning a 400 bad request with the error invalid JSON: unexpected end of JSON input. The JSON being sent is the list of Global Unicast IP addresses that trident collects. I've run it through a linter and it comes out as valid json, and it's a somewhat long list as we have around 90+ nodes in the cluster. It seems like nodes are initially able to register, but repeated updates to the node cache are throwing these errors.

Environment

• Trident version: 20.07.0
• Trident installation flags used: custom image + registry defined in provisioner spec. debug flag in operator.
• Container runtime: 19.03.11
• Kubernetes version: 1.18.3
• Kubernetes orchestrator: none
• Kubernetes enabled feature gates: All required trident feature gates should be enabled by default in this kube version.
• OS: Fedora CoreOS 32.20200824.3.0
• NetApp backend types: ONTAP-SAN

To Reproduce
Deploy trident operator following steps in documentation. After deploying the trident provisioner manifest, trident nodes will be repeatedly failing to register with CSI controller. Continue with deployment, create backend, storageclass etc.. and create a PVC. The PVC is able to be bound to a PV with the backend, then attempt to mount the PV into the pod. This may be hard to replicate similar to our environment.

Expected behavior
This has previously been deployed to one of our development clusters and has been successful. I expect the same thing to happen the cluster I'm attempting to deploy to, where trident nodes are registered with the controller and PVs are successfully mounted to pods.

[JosephGJ]

What we think is happening here is that it is due to the way Trident gets the IPs the node has (https://github.com/NetApp/trident/blob/4c0959c5c086c3bdf74bb298b806ee72a50a0455/utils/osutils.go#L339). This method also picks up the IPVS IPs on the cluster. This is due to IPVS being an interface that has all IPs of all the services attached.

This is called when Trident attempts to register the node with the controller (https://github.com/NetApp/trident/blob/4c0959c5c086c3bdf74bb298b806ee72a50a0455/frontend/csi/rest.go#L119). The resulting payload is too large and causes the call to fail.
We have proven that the current method of getting the node IPs includes all the IPVS IPs using this simple go program, and a possible way to fix this:

package main
import (
    "fmt"
    "net"
    "strings"
)
// Current implementation
func getAddresses1() ([]net.Addr, error) {
    return net.InterfaceAddrs()
}
// New implementation
func getAddresses2() ([]net.Addr, error) {
    ifs, err := net.Interfaces()
    if err != nil {
        return nil, err
    }
    addrs := []net.Addr{}
    for _, iface := range ifs {
        if isValidInterface(iface) {
            addrsToAdd, err := iface.Addrs()
            if err != nil {
                return nil, err
            } 
            addrs = append(addrs, addrsToAdd...)
        }
    }
    return addrs, nil
}
func isValidInterface(iface net.Interface) bool {
    // Could this be a flag? Maybe regex? /^(docker|flannel|cali|kube-ipvs).*/
    switch {
    case strings.HasPrefix(iface.Name, "docker"):
        return false
    case strings.HasPrefix(iface.Name, "flannel"):
        return false
    case strings.HasPrefix(iface.Name, "cali"):
        return false
    case strings.HasPrefix(iface.Name, "kube-ipvs"):
        return false
    }
    return true
}
func main() {
    fmt.Println(getAddresses1()) // Prints all the IPVS/docker/flannel/cali IPs too
    fmt.Println(getAddresses2()) // Prints just the IPs that trident could possibly use
}

[gnarl]

Hi @JosephGJ,

Thanks for providing additional details on the issue you are encountering. We'll look into providing a fix although it won't make the Trident 20.10.0 release.

[clintonk]

@JosephGJ, does this still occur if you set autoExportCIDRs in your backend config?

https://netapp-trident.readthedocs.io/en/stable-v20.07/kubernetes/operations/tasks/backends/ontap/ontap-nas/dynamic-export-policies.html?highlight=autoExportCIDRs

[JosephGJ]

@clintonk This still occurs if the autoExportCIDRs option with our worker subnet range is added to the backend. The issue with worker pods bein unable to register with the trident-main containers CSI controller occurs when you deploy the tridentprovisioner CR object, which is before any backends are configured anyway.

[clintonk]

@JosephGJ Thanks for your help thus far. It's clear that you are correct that the IPVS interface lists global unicast addresses for every K8S service, and that is overwhelming the node CR where Trident keeps this data. The string matching approach to filtering interfaces could work, but given the number of overlay networks available for K8S we would be tweaking the regex for a long time. So it'd be ideal to find a better way to eliminate the service addresses. The Trident node daemonset doesn't access the K8S API, and we'd like to keep it that way if possible, so filtering addresses by interface & address properties would be preferable. I've prototyped a couple approaches, one that removes dummy interfaces as used by IPVS, and one that considers only interfaces used by default routes. We could even do a union of those two result sets. Here is your sample code modified to test those ideas, and if you could let us know how it performs in your environment that would be most helpful.

package main

import (
    "net"
    "strings"

    log "github.com/sirupsen/logrus"
    "github.com/vishvananda/netlink"
)

// Current implementation
func getAddresses1() []net.Addr {
    addrs, _ := net.InterfaceAddrs()
    return addrs
}

// New implementation (JosephGJ)
func getAddresses2() []net.Addr {
    ifs, err := net.Interfaces()
    if err != nil {
        log.Fatal(err)
    }
    addrs := []net.Addr{}
    for _, iface := range ifs {
        if isValidInterface(iface) {
            addrsToAdd, err := iface.Addrs()
            if err != nil {
                log.Fatal(err)
            }
            addrs = append(addrs, addrsToAdd...)
        }
    }
    return addrs
}

func getAddresses3() []net.Addr {

    links, err := netlink.LinkList()
    if err != nil {
        log.Fatal(err)
    }

    addrs := make([]net.Addr, 0)

    for _, link := range links {

        log.Debugf("Interface %+v, type = %s\n", link, link.Type())

        linkAddrs, err := netlink.AddrList(link, netlink.FAMILY_ALL)
        if err != nil {
            log.Warningf("   Could not get addresses for interface %s; %v\n", link.Attrs().Name, err)
            continue
        }

        for _, linkAddr := range linkAddrs {

            if link.Type() == "dummy" {
                log.Debugf("   Address %s for dummy interface %s; skipping\n", linkAddr.String(), link.Attrs().Name)
                continue
            }

            ipNet := linkAddr.IPNet
            if ipNet == nil {
                log.Debugf("   Nil IPNet for address %s; skipping\n", linkAddr.String())
                continue
            }

            if !ipNet.IP.IsGlobalUnicast() {
                log.Debugf("   Address %s is not global unicast; skipping\n", linkAddr.String())
                continue
            }

            log.Debugf("   Valid address %s\n", linkAddr.String())
            addrs = append(addrs, ipNet)
        }
    }

    return addrs
}

func getAddresses4() []net.Addr {

    // Get all default routes (nil destination)
    routes, err := netlink.RouteListFiltered(netlink.FAMILY_ALL, &netlink.Route{}, netlink.RT_FILTER_DST)
    if err != nil {
        log.Fatal(err)
    }

    // Get deduplicated set of links associated with default routes
    intfIndexMap := make(map[int]bool)
    for _, route := range routes {
        log.Debugf("%+v", route)
        intfIndexMap[route.LinkIndex] = true
    }

    links := make([]netlink.Link, 0)
    for linkIndex := range intfIndexMap {
        if link, err := netlink.LinkByIndex(linkIndex); err != nil {
            log.Fatal(err)
        } else {
            links = append(links, link)
        }
    }

    addrs := make([]net.Addr, 0)

    for _, link := range links {

        log.Debugf("Interface %s, type = %s\n", link.Attrs().Name, link.Type())

        linkAddrs, err := netlink.AddrList(link, netlink.FAMILY_ALL)
        if err != nil {
            log.Warningf("   Could not get addresses for interface %s; %v\n", link.Attrs().Name, err)
            continue
        }

        for _, linkAddr := range linkAddrs {

            if link.Type() == "dummy" {
                log.Debugf("   Address %s for dummy interface %s; skipping\n", linkAddr.String(), link.Attrs().Name)
                continue
            }

            ipNet := linkAddr.IPNet
            if ipNet == nil {
                log.Debugf("   Nil IPNet for address %s; skipping\n", linkAddr.String())
                continue
            }

            if !ipNet.IP.IsGlobalUnicast() {
                log.Debugf("   Address %s is not global unicast; skipping\n", linkAddr.String())
                continue
            }

            log.Debugf("   Valid address %s\n", linkAddr.String())
            addrs = append(addrs, ipNet)
        }
    }

    return addrs
}

func getAddresses5() []net.Addr {

    addressMap := make(map[string]net.Addr)

    for _, addr := range getAddresses3() {
        addressMap[addr.String()] = addr
    }
    for _, addr := range getAddresses4() {
        addressMap[addr.String()] = addr
    }

    addrs := make([]net.Addr, 0)

    for _, addr := range addressMap {
        addrs = append(addrs, addr)
    }

    return addrs
}

func isValidInterface(iface net.Interface) bool {
    // Could this be a flag? Maybe regex? /^(docker|flannel|cali|kube-ipvs).*/
    switch {
    case strings.HasPrefix(iface.Name, "docker"):
        return false
    case strings.HasPrefix(iface.Name, "flannel"):
        return false
    case strings.HasPrefix(iface.Name, "cali"):
        return false
    case strings.HasPrefix(iface.Name, "kube-ipvs"):
        return false
    }
    return true
}

func main() {

    log.SetLevel(log.InfoLevel)

    // Prints all the global unicast addresses from all interfaces (IPVS/docker/flannel/cali IPs too)
    log.Info("1. All interfaces: ", getAddresses1())

    // Prints the IPs from interfaces whose names match a regex (/^(docker|flannel|cali|kube-ipvs).*/)
    log.Info("2. Regex-filtered interfaces: ", getAddresses2())

    // Prints all the global unicast addresses from all non-dummy interfaces
    log.Info("3. Non-dummy interfaces: ", getAddresses3())

    // Prints all the global unicast addresses from interfaces with default routes
    log.Info("4. Default-route interfaces: ", getAddresses4())

    // Prints the union of sets 3 & 4
    log.Info("5. Non-dummy plus default-route interfaces: ", getAddresses5())
}

[JosephGJ]

@clintonk I've tested these implementations on our hosts and these are the outputs from the code. The interface needed for registering the tested node with the CSI controller is 172.16.244.128/24, so it seems any of the suggested implementations should work.

Output:

1. All interfaces:  [127.0.0.1/8 172.16.244.128/24 172.16.191.128/24 172.17.0.1/16 10.227.81.225/32 172.16.243.78/32 10.227.131.42/32 10.227.94.221/32 ... (There's a lot in here) ] 
2. Regex-filtered interfaces:  [127.0.0.1/8 ::1/128 172.16.244.128/24 fe80::5b06:4f1b:7be9:a999/64 172.16.191.128/24 fe80::ee88:1834:8c35:c2dd/64]
3. Non-dummy interfaces:  [172.16.244.128/24 172.16.191.128/24 172.17.0.1/16 10.226.22.0/32]
4. Default-route interfaces:  [172.16.244.128/24]
5. Non-dummy plus default-route interfaces:  [172.16.244.128/24 172.16.191.128/24 172.17.0.1/16 10.226.22.0/32]

[clintonk]

@JosephGJ Thank you! We're working on getting implementation #5 into the next Trident update, and your confirmation is very helpful. Your help understanding root cause for this issue was instrumental.