eBay / go-ovn

A Go library for OVN Northbound/Southbound DB access using native OVSDB protocol
Apache License 2.0
108 stars 59 forks source link

add ForceReconnect API #130

Closed winsopc closed 3 years ago

winsopc commented 3 years ago

Signed-off-by: Zhen Wang zhewang@nvidia.com

vtolstov commented 3 years ago

LGTM, but can you describe why you need it ?

winsopc commented 3 years ago

this API benefits for goovn user ovn-kubernetes to support ovndb TLS certFile and privkeyFile update on purpose. https://github.com/ovn-org/ovn-kubernetes/blob/master/go-controller/pkg/util/go_ovn.go L87-97.

With new API, I can use fsnotify watcher to watch the file change and update the tlsconfig.cert and call new API without worry about high layer use goovn.client.

winsopc commented 3 years ago

example code to use the api: `// watchForChanges exits if the configuration file changed. func updateSslKeyPair(certFile, privKeyFile string, tlsConfig *tls.Config, ovndbclient goovn.Client) error {

if certFile == "" || privKeyFile == "" {
    return nil
}
watcher, err := fsnotify.NewWatcher()
if err != nil {
    return err
}

go func() {
    for {
        select {
        case event, ok := <-watcher.Events:
            if ok && event.Op&fsnotify.Write == fsnotify.Write {
                cert, err := tls.LoadX509KeyPair(certFile, privKeyFile)
                if err != nil {
                    klog.Infof("cannot load new cert with cert %s key %s err %s", certFile, privKeyFile, err)
                    continue
                }
                if reflect.DeepEqual(tlsConfig.Certificates, []tls.Certificate{cert}) {
                    klog.Infof("tls update already finished")
                    continue
                }
                tlsConfig.Certificates = []tls.Certificate{cert}
                err = ovndbclient.ForceReconnect()
                klog.Infof("dbclient %p force reconnect with new tlsconfig %p status %v", ovndbclient, tlsConfig, err)
            }
        case err, ok := <-watcher.Errors:
            if ok {
                klog.Errorf("Error watching for changes to ssl key err: %v", privKeyFile, err)
            }
        }
    }
}()

if err := watcher.Add(certFile); err != nil {
    return err
}
if err := watcher.Add(privKeyFile); err != nil {
    return err
}
return nil

} `

vtolstov commented 3 years ago

what you think about adding such example to examples for this function in godoc format?

winsopc commented 3 years ago

Sure, I will add it.

amorenoz commented 3 years ago

This is a very good feature. Should it be moved to libovsdb?

winsopc commented 3 years ago

@amorenoz I think libovsdb needs more code changes to support this compare to support it in go-ovn layer. The current OvsdbClient struct does not store *tlsconfig info and endpoints address.

winsopc commented 3 years ago

Current solution will trigger connect() to download the DB again. After studying the go-ovn and libovsdb, I think current interface can support the feature without new code changes. we can rely on underlying rpc2.client reconnect mechanism to reconnect with new cert.