Skip to content

Commit

Permalink
Fix kvstore kv-compare cmd
Browse files Browse the repository at this point in the history
Summary:
Looks like we have a dummy comparison cmd here :P

Issues found:
kvstore kv-compare is the cmd to do the comparison between current nodes kvstore dict and its peer's kvstore dict.

Let's say we have node1, nod2 and node3 and they are peers of each other. We try do compare kvstore copy of node1 with node2 and node3 to see if they are consistent. However, by utilizing the current `KvStoreClient` functionality, we are verifying node1's kvstore copy with its OWN.

Reason is root caused due to the usage of `cli_opts` passed in to KvStoreClient. Although `zmq_endpoint` is passed in separately, it is NOT honored and used.

KvStoreClient:
```
  21 class KvStoreClient(OpenrClientDeprecated):
  22     def __init__(self, cli_opts, host=None):
  23         host = host if host else cli_opts.host
  24         zmq_endpoint = "tcp://[{}]:{}".format(host, cli_opts.kv_rep_port)
  25         super(KvStoreClient, self).__init__(
  26             OpenrModuleType.KVSTORE, zmq_endpoint, cli_opts
  27         )
```

OpenrClientDeprecated:
```
 153     def get_thrift_client(self, use_ssl):
 154         socket = (
 155             TSSLSocket.TSSLSocket(

 156                 host=self.cli_opts.host,
 157                 port=self.cli_opts.openr_ctrl_port,

 158                 # verify server
 159                 cert_reqs=ssl.CERT_REQUIRED,
 160                 ca_certs=self.cli_opts.ca_file,
 161                 certfile=self.cli_opts.cert_file,
 162                 keyfile=self.cli_opts.key_file,
 163                 verify_name=self.cli_opts.acceptable_peer_name,
 164             )
 165             if use_ssl
 166             else TSocket.TSocket(
 167                 host=self.cli_opts.host, port=self.cli_opts.openr_ctrl_port
 168             )
 169
```

FIx:
As part of the zmq->thrift migration, this OpenrClientDeprecated will no longer be used. By utilizing OpenrCtrlClient, host info is honored, which leads to the expected behavior of dumping kvstore dic of peer nodes.

Reviewed By: saifhhasan, tonyqiu1019

Differential Revision: D15929012

fbshipit-source-id: 4afc7261ce11d0ae4281206604f3dd4256e8a1fd
  • Loading branch information
xiangxu1121 authored and facebook-github-bot committed Jun 21, 2019
1 parent 9d7f9db commit 43ea2c6
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 20 deletions.
4 changes: 2 additions & 2 deletions openr/py/openr/cli/commands/kvstore.py
Expand Up @@ -14,7 +14,7 @@
import string
import sys
import time
from builtins import object, str
from builtins import str
from itertools import combinations
from typing import Any, Callable, Dict, List

Expand All @@ -24,7 +24,7 @@
from openr.AllocPrefix import ttypes as alloc_types
from openr.cli.utils import utils
from openr.cli.utils.commands import OpenrCtrlCmd
from openr.clients import kvstore_client, kvstore_subscriber
from openr.clients import kvstore_subscriber
from openr.KvStore import ttypes as kv_store_types
from openr.Lsdb import ttypes as lsdb_types
from openr.Network import ttypes as network_types
Expand Down
32 changes: 14 additions & 18 deletions openr/py/openr/cli/utils/utils.py
Expand Up @@ -20,10 +20,8 @@

import bunch
import click
import zmq
from openr.AllocPrefix import ttypes as alloc_types
from openr.clients.kvstore_client import KvStoreClient
from openr.clients.lm_client import LMClient
from openr.clients.openr_client import get_openr_ctrl_client
from openr.Fib import ttypes as fib_types
from openr.KvStore import ttypes as kv_store_types
from openr.Lsdb import ttypes as lsdb_types
Expand Down Expand Up @@ -139,15 +137,13 @@ def get_fib_agent_client(
return client


def get_connected_node_name(cli_opts):
def get_connected_node_name(cli_opts: bunch.Bunch) -> str:
""" get the identity of the connected node by querying link monitor"""

client = LMClient(cli_opts)

try:
return client.get_identity()
except zmq.error.Again:
return cli_opts.host
identity = None
with get_openr_ctrl_client(cli_opts.host, cli_opts) as client:
identity = client.getInterfaces().thisNodeName
return identity


def get_route_nexthops(
Expand Down Expand Up @@ -1053,14 +1049,14 @@ def sprint_prefixes_db_delta(global_prefixes_db, prefix_db):
return strs


def dump_node_kvs(cli_opts, host):
client = KvStoreClient(cli_opts, host=host)
try:
kv = client.dump_all_with_filter()
except zmq.error.Again:
print("cannot connect to {}'s kvstore".format(host))
return None
return kv
def dump_node_kvs(cli_opts: bunch.Bunch, host: str) -> kv_store_types.Publication:
pub = None

with get_openr_ctrl_client(host, cli_opts) as client:
keyDumpParams = kv_store_types.KeyDumpParams(Consts.ALL_DB_MARKER)
pub = client.getKvStoreKeyValsFiltered(keyDumpParams)

return pub


def print_allocations_table(alloc_str):
Expand Down

0 comments on commit 43ea2c6

Please sign in to comment.