Implement antctl get pod-interface [podName] #334

lzhecheng · 2020-01-20T11:22:50Z

Issue #275

Implemented antctl subcommand: pod-interface

This command is under get group to get pod interface information.
Users can filter the result with "podName" as an arg or "namespace"
as an flag. The default namespace is "default".
Removed duplicated flag in commandDefinition.
Support bool flag

The output should be like this:

{
  "name": "patch-demo-c8796b6dd-5k6qn",
  "InterfaceName": "patch-de-17bf25",
  "IP": "192.168.2.7",
  "MAC": "36:44:77:45:ee:55",
  "PortUUID": "e2bc4318-5cda-4be9-ab61-8ff748db7320",
  "OFPort": 8,
  "ContainerID": "58e4aba280fe1696a647aa0f928071a3bfb25a9da21937dcdc05200fce39ca20",
  "PodNamespace": "default"
}

antrea-bot · 2020-01-20T11:23:05Z

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

/test-e2e: to trigger e2e tests.
/skip-e2e: to skip e2e tests.
/test-conformance: to trigger conformance tests.
/skip-conformance: to skip conformance tests.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-all: to trigger all tests.
/skip-all: to skip all tests.

These commands can only be run by members of the vmware-tanzu organization.

lzhecheng · 2020-01-21T05:13:12Z

/test-all

lzhecheng · 2020-01-21T07:55:37Z

/test-networkpolicy

antoninbas

I may be missing something, but it seems that the command should use the Namespace as well as the Pod name.

IMO, if I have 2 Pods with the same name in 2 different namespaces, I should be able to select a single one. It doesn't seem like it is the case right now. It seems that you stop at the first Pod for which the name matches?

Also, as I commented on the agent-info PR, will we be able to run this command from out-of-cluster (from out of the agent Pod)? That would be very useful.

antoninbas · 2020-01-21T19:49:54Z

pkg/agent/interfacestore/types.go

@@ -72,6 +72,7 @@ type InterfaceStore interface {
 	GetContainerInterface(podName string, podNamespace string) (*InterfaceConfig, bool)
 	GetNodeTunnelInterface(nodeName string) (*InterfaceConfig, bool)
 	GetContainerInterfaceNum() int
+	GetContainerInterfaces() []*InterfaceConfig


maybe others will disagree but I would suggest having a more generic GetInterfacesByType(interfaceType InterfaceType) interface method - like we have GetInterfaceKeysByType below. Maybe it will help keep the number of methods for this interface "reasonable".

Do you mean make the command like antctl get pod podName -n nsName ? Do you use namespace to filter for CRD @mengdie-song ?
Then if user doesn't specify the namespace name, let's show all pods with different namespaces ?

Yes, we can run this command remotely with future implementation.

@lzhecheng For CRD, currently we only have the field LocalPodNum to show the number of Pod. I did not distinguish them by namespaces. I can enhance this part if we want more information for local pods in CRD. But if the number of Pods is too large, I am afraid the CRD is not friendly way to show.

However, CLI can show more information than CRD if we want. Since this patch has already added a separate function GetContainerInterfaces to get details, we can get Pod namespace from InterfaceConfig if we would like to show it via CLI.

antoninbas · 2020-01-21T19:51:04Z

pkg/antctl/antctl.go

@@ -76,6 +78,16 @@ var CommandList = &commandList{
 			CommandGroup:        flat,
 			AddonTransform:      versionTransform,
 		},
+		{
+			Use:                 "pod",
+			Short:               "Print pod's basic information",


I suggest using "Pod" and not "pod" for user-facing messages, for the sake of consistency.

Reasonable. Updated.

antoninbas · 2020-01-21T19:54:00Z

pkg/antctl/handlers/pod.go

@@ -0,0 +1,74 @@
+// Copyright 2020 Antrea Authors


should the file be named "get_pod.go" instead since this is the "pod" subcommand in the "get" command group? Would it be possible to have a "pod" subcommand in another command group?

Yes, it is possible. Updated.

antoninbas · 2020-01-21T19:59:17Z

pkg/antctl/antctl.go

+		{
+			Use:                 "pod",
+			Short:               "Print pod's basic information",
+			Long:                "Print pod's basic information of interface config.",


suggestion: "Print information about the network interface(s) created by the Antrea agent for the specified Pod".

jianjuns

A general question for this cmd - do we plan to add more information about the Pod later? If it is just about Pod interface info, maybe name it get pod-interface?

jianjuns · 2020-01-21T21:59:33Z

pkg/monitor/querier.go

@@ -113,6 +115,11 @@ func (monitor *agentMonitor) GetLocalPodNum() int32 {
 	return int32(monitor.interfaceStore.GetContainerInterfaceNum())
 }

+// GetContainerInterfaces gets the information of the pod with podName.
+func (monitor *agentMonitor) GetContainerInterfaces() []*interfacestore.InterfaceConfig {


I would stop adding more CLI code to Agent/ControllerMonitor which is originally for MonitorCRD.
But we can decide this together with other changes I suggested like extracting API server code out of CLI.

jianjuns · 2020-01-21T22:01:23Z

pkg/antctl/antctl.go

@@ -76,6 +78,16 @@ var CommandList = &commandList{
 			CommandGroup:        flat,
 			AddonTransform:      versionTransform,
 		},
+		{
+			Use:                 "pod",
+			Short:               "Print pod's basic information",


jianjuns

Could you provide an example output of the command? I have three general questions:

Should we add the namespace argument (like --namespace/-N just like kubectl).
Should we support getting all Pods (Pod name is not specified).
If the command is for Pod interface configuration only? If so should we name it get pod-interface; if not should we define a separate struct like PodInfo for future extension, instead of using InterfaceConfig directly.
@tnqn

lzhecheng · 2020-02-05T01:40:56Z

Could you provide an example output of the command? I have three general questions:

Should we add the namespace argument (like --namespace/-N just like kubectl).

Should we support getting all Pods (Pod name is not specified).

If the command is for Pod interface configuration only? If so should we name it get pod-interface; if not should we define a separate struct like PodInfo for future extension, instead of using InterfaceConfig directly.
@tnqn

I just put an example in the description of this PR.

Yes, this is under development.
Yes, this has been implemented.
I agree we can change the command name to "pod-interface".

lzhecheng · 2020-02-21T06:50:57Z

/test-networkpolicy

tnqn · 2020-03-09T13:06:17Z

pkg/agent/apiserver/apiserver.go

@@ -52,6 +53,7 @@ func (s *agentAPIServer) Run(stopCh <-chan struct{}) error {

 func installHandlers(aq monitor.AgentQuerier, npq monitor.AgentNetworkPolicyInfoQuerier, s *genericapiserver.GenericAPIServer) {
 	s.Handler.NonGoRestfulMux.HandleFunc("/agentinfo", agentinfo.HandleFunc(aq))
+	s.Handler.NonGoRestfulMux.HandleFunc("/getpodinterface", getpodinterface.HandleFunc(aq))


Restful api doesn't have verb in url path, which should be expressed by http method.

tnqn · 2020-03-09T13:08:31Z

hack/netpol/pkg/main/main.go

@@ -150,19 +150,19 @@ func main() {
 	failOnError(err)

 	testList := []*TestCase{
-		&TestCase{"DefaultDeny", testDefaultDeny()},


I understand they are redundant but better to not touch unrelated code so that reviewers can focus on your change and there won't be surprise when seeing git history that some lines are changed in an unrelated PR.

Removed since this is fixed in other PR.

tnqn · 2020-03-09T13:10:28Z

pkg/agent/apiserver/handlers/getpodinterface/handler.go

+		m := make(map[string][]*interfacestore.InterfaceConfig)
+		for _, v := range aq.GetInterfacesByType(interfacestore.ContainerInterface) {
+			podName := (*v.ContainerInterfaceConfig).PodName
+			vPodNS := (*v.ContainerInterfaceConfig).PodNamespace


why is there a "v" prefix for namespace only?

There used to be other var named podNS so I added "v" to avoid conflict. Now it is ok to name it "podNS".

tnqn · 2020-03-09T13:11:15Z

pkg/agent/apiserver/handlers/getpodinterface/handler.go

+			if (!doFilter || key[0] == podName) && ns[0] == vPodNS {
+				m[podName] = append(m[podName], v)
+			}
+			fmt.Println("podName:", podName, "vPodNS:", vPodNS)


Why using Println in server code?

It is not final code and only for debugging. Removed.

tnqn · 2020-03-09T13:17:49Z

pkg/agent/apiserver/handlers/getpodinterface/handler.go

+func HandleFunc(aq monitor.AgentQuerier) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		if aq == nil {
+			w.WriteHeader(http.StatusNotImplemented)


This is a closure so aq has been decided before serving requests. If aq could possibly be nil, I would prefer we return error at the earliest place so that we don't need to check aq in every subroutine of the call stack.
Actually I checked agent.go has made sure aq cannot be nil so this check is not necessary.

Thanks for reminding. Code is not updated according to refactored framework.

tnqn · 2020-03-09T13:45:23Z

pkg/agent/apiserver/handlers/getpodinterface/handler.go

+		for _, v := range aq.GetInterfacesByType(interfacestore.ContainerInterface) {
+			podName := (*v.ContainerInterfaceConfig).PodName
+			vPodNS := (*v.ContainerInterfaceConfig).PodNamespace
+			if (!doFilter || key[0] == podName) && ns[0] == vPodNS {


Is namespace a mandatory field? Is it safe to assume len(ns) > 0?

namespace has a default value and if no value after "-n", it will return Error: flag needs an argument: 'n' in -n.

tnqn · 2020-03-09T13:47:44Z

pkg/agent/apiserver/handlers/getpodinterface/handler.go

+			if err != nil {
+				w.WriteHeader(http.StatusInternalServerError)
+			}
+		}


This can be simpler:

if doFilter and len(m) == 0 { w.WriteHeader(http.StatusNotFound) } else { var pods []GetPodInterfaceResponse for k, v := range m { pods = append(pods, GetPodInterfaceResponse{k, v}) } err := json.NewEncoder(w).Encode(pods) if err != nil { w.WriteHeader(http.StatusInternalServerError) } }

Refactored.

tnqn · 2020-03-09T13:53:35Z

pkg/agent/apiserver/handlers/getpodinterface/handler.go

+			return
+		}
+
+		m := make(map[string][]*interfacestore.InterfaceConfig)


Why using map and having the podname as the key? Is there any reason pods having same name but in different namespaces needs to be grouped together?

Refactored.

tnqn · 2020-03-09T13:56:40Z

Could you provide an example output of the command? I have three general questions:

Should we add the namespace argument (like --namespace/-N just like kubectl).

Should we support getting all Pods (Pod name is not specified).

If the command is for Pod interface configuration only? If so should we name it get pod-interface; if not should we define a separate struct like PodInfo for future extension, instead of using InterfaceConfig directly.
@tnqn

@jianjuns, sorry for missing the notification. All sounds good to me.

lzhecheng · 2020-03-10T08:02:18Z

/test-e2e

lzhecheng · 2020-03-17T06:11:47Z

I followed @jianjuns 's suggestion and updated the code.

lzhecheng · 2020-03-17T06:34:36Z

/test-all

tnqn · 2020-03-17T06:33:51Z

pkg/agent/apiserver/handlers/podinterface/handler.go

+// Response describes the internal response struct of pod-interface command.
+type Response struct {
+	PodName       string         `json:"name,omitempty" yaml:"name,omitempty" antctl:"name,The name of the pod"`
+	PodInterfaces []PodInterface `json:",inline" yaml:",inline"`


Again, why multiple interfaces with same Pod name are grouped together? They are logically unrelated, I think the previous flatten struct works.

tnqn · 2020-03-17T06:36:53Z

pkg/agent/apiserver/handlers/podinterface/handler.go

+			podName := (*v.ContainerInterfaceConfig).PodName
+			podNS := (*v.ContainerInterfaceConfig).PodNamespace
+			if (len(name) == 0 || name == podName) && (len(ns) == 0 || ns == podNS) {
+				m[podName] = append(m[podName], generatePodInterface(podName, v))


ditto, is there any problem using a slice to keep result?

tnqn · 2020-03-17T06:58:36Z

pkg/agent/apiserver/handlers/podinterface/handler.go

+				}
+			}
+		} else {
+			var pods []Response


The 3 branches can be simplified as:

if len(name) > 0 && len(m) == 0 { w.WriteHeader(http.StatusNotFound) return } var pods []Response ...

lzhecheng · 2020-03-17T11:25:31Z

Code updated to avoid grouping pods with same name but in different namespaces together. isSingle field in nonResourceEndpoint is turned into outputType, an enum including default, single and multiple.

lzhecheng · 2020-03-18T00:58:30Z

/test-all

lzhecheng · 2020-03-18T06:39:47Z

/test-e2e
/test-conformance

tnqn · 2020-03-18T06:53:31Z

pkg/agent/apiserver/handlers/podinterface/handler.go

+		if len(name) > 0 && len(pods) == 0 {
+			w.WriteHeader(http.StatusNotFound)
+		} else {
+			err := json.NewEncoder(w).Encode(pods)


nit: keep the normal code path at a minimal indentation, and indent the error handling, dealing with it first https://github.com/golang/go/wiki/CodeReviewComments#indent-error-flow

tnqn · 2020-03-18T06:57:29Z

pkg/antctl/command_definition.go

+type OutputType uint
+
+const (
+	dflt OutputType = iota


"default" is shorter than "multiple", why use short for it alone? and this short doesn't seem common.

I think "default" is reserved? Let's change it to "defaultType"

tnqn · 2020-03-18T06:59:31Z

pkg/antctl/command_definition.go

@@ -62,7 +69,7 @@ var groupCommands = map[commandGroup]*cobra.Command{
 }

 type endpointResponder interface {
-	single() bool
+	output() OutputType


output sounds like an action, maybe "outputType" to avoid confusion.

IMO, "output" sounds like a noun which is shorter like the original "single". Also, if changed, we will have outputType() as a function and a outputType as a field in nonResourceEndpoint, which is confusing I think.

Of course it can sound like a noun but it means the output of the responder? I don't know if it's only me who will be confused by the function name. AFAIK, Output is widely used by exec, command, http request to get the output and the noun output is used to take the return value.
Besides, if output is really clear for the mean of outputType, why the type is named OutputType while the method is named output?
Defining a method to get the field with same name is quite common regardless of languages, it's even encouraged to do it in Golang (to save the prefix Get), I don't think it's confusing.

If you have a field called owner (lower case, unexported), the getter method should be called Owner (upper case, exported), not GetOwner.

https://golang.org/doc/effective_go.html#Getters

Thanks for sharing the doc. But outputType() cannot work, it returns type nonResourceEndpoint has both field and method named outputType. I choose OutputType() instead.

tnqn · 2020-03-18T07:02:16Z

pkg/antctl/command_definition_test.go

 		expect       string
 	}{
-		"SingleObject": {
+		"outputObject": {


This case is still testing single object, why change the case name?

Updated. Thanks for reminding me cuz I changed it by mistake.

tnqn · 2020-03-18T07:02:39Z

pkg/antctl/command_definition_test.go

@@ -108,13 +108,13 @@ func TestCommandDefinitionGenerateExample(t *testing.T) {
 	for k, tc := range map[string]struct {
 		use          string
 		cmdChain     string
-		singleObject bool
+		outputObject OutputType


why not use outputType as variable name?

tnqn · 2020-03-18T07:03:34Z

pkg/antctl/command_definition.go

+	// StringFlag represents a string flag
+	StringFlag FlagType = iota
+	// BoolFlag represents a boolean flag
+	BoolFlag


Do we still need this?

Indeed, BoolFlag is not applied by any commands now. However, I suggest keeping it since these newly-added types not just add support for "bool" flag but also it refactors the code to adapt more kinds of flags in the future. Unless there will only be string flag, I think it is worthwhile to keep these changes.

Then we should add this the first time it is used, not assuming it can be used one day unpredictably. Some people are even dedicated to clean up functions, structs that are no longer used no matter how useful they used to be. We shouldn't do the opposite thing.

tnqn · 2020-03-18T11:36:06Z

pkg/antctl/command_definition.go

+				argMap[f.name] = vs
+				continue
+			}
+			// flag is bool


are they still needed?

tnqn · 2020-03-18T11:47:02Z

pkg/antctl/command_definition.go

+const (
+	defaultType OutputType = iota
+	single
+	multiple


I think it's worth to add comments for this values, it took me a while to understand, maybe others will have question too.

Comments added.

weiqiangt

Overall, LGTM, some nits left.

weiqiangt · 2020-03-12T06:13:48Z

pkg/monitor/querier.go

@@ -118,6 +120,11 @@ func (monitor *agentMonitor) GetLocalPodNum() int32 {
 	return int32(monitor.interfaceStore.GetContainerInterfaceNum())
 }

+// GetInterfacesByType gets the interface information by type.
+func (monitor *agentMonitor) GetInterfacesByType(interfaceType interfacestore.InterfaceType) []*interfacestore.InterfaceConfig {


@jianjuns Do you think we should access interfacestore in the handler directly?

weiqiangt · 2020-03-18T14:11:18Z

pkg/antctl/command_definition.go

+	defaultType OutputType = iota
+	// single represents the output type is single item.
+	single
+	// multiple represents the output type is list.


nit, maybe add the modifier always?

weiqiangt · 2020-03-18T14:13:34Z

pkg/antctl/command_definition.go

@@ -473,7 +496,7 @@ func (cd *commandDefinition) newCommandRunE(c *client) func(*cobra.Command, []st
 		if err != nil {
 			return err
 		}
-		return cd.output(resp, os.Stdout, formatterType(outputFormat), cd.getEndpoint().single() || argGet)
+		return cd.output(resp, os.Stdout, formatterType(outputFormat), cd.getEndpoint().OutputType() != multiple && (cd.getEndpoint().OutputType() == single || argGet))


nit: it would be better to have a variable for cd.getEndpoint().OutputType() != multiple && (cd.getEndpoint().OutputType() == single || argGet).

pkg/agent/apiserver/handlers/podinterface/handler.go

jianjuns · 2020-03-18T23:46:47Z

pkg/antctl/command_definition.go

-	isSingle bool
+	path       string
+	params     []flagInfo
+	outputType OutputType


Have a question here: for registered APIGroup exposed via API Server, could we have the "single" case, but is the API path exposes a single object, but not a type of resources?
I am asking because I am adding the controller-info cmd which should be exposed via aggregated API, and wondering it should be declared as a resourceEndpoint or nonResourceEndpoint in antctl.go. The former does not have the "single" type now,
@weiqiangt @tnqn

I think the controller-info is a similar case of version.
If you specified a resourceName, the antctl would always assume the response is single. The antctl does not know what the content is but just dispatch the response to the transform function according to definition.

Sure. My question is could resourceEndpoint also have the "single" type case, or it should be always "multiple"?

For now, if a resourceEndpoint specified resourceName then it is single inexplicitly, otherwise, the antctl will only assume the response is single when running command with arg.
I don't think @lzhecheng implemented multiple for resourceEndpoint, but developers can make the both single transform func and list transform func expect same type to hack a multiple behavior.

* This command is under get group to get pod interface information. Users can filter the result with "podName" as an arg or "namespace" as an flag. The default namespace is "default". * Remove duplicated flag in commandDefinition. * Turn isSingle field in nonResourceEndpoint to outputType which is an enum, including default, single and multiple Co-Authored-By: Antonin Bas <[email protected]>

jianjuns · 2020-03-19T04:44:35Z

LGTM

lzhecheng · 2020-03-19T06:00:21Z

/test-all

tnqn

LGTM

* This command is under get group to get pod interface information. Users can filter the result with "podName" as an arg or "namespace" as an flag. The default namespace is "default". * Remove duplicated flag in commandDefinition. * Turn isSingle field in nonResourceEndpoint to outputType which is an enum, including default, single and multiple Co-Authored-By: Antonin Bas <[email protected]>

This reverts commit eee580b.

lzhecheng requested review from mengdie-song, weiqiangt and vicky-liu January 20, 2020 11:22

vmwclabot added the cla-not-required label Jan 20, 2020

lzhecheng force-pushed the get-pod branch from 3d43f32 to d7610c8 Compare January 21, 2020 01:55

lzhecheng changed the title ~~WIP: Implement antctl get pod [podName]~~ Implement antctl get pod [podName] Jan 21, 2020

lzhecheng force-pushed the get-pod branch from d7610c8 to c3147d1 Compare January 21, 2020 02:38

antoninbas reviewed Jan 21, 2020

View reviewed changes

jianjuns reviewed Jan 21, 2020

View reviewed changes

lzhecheng force-pushed the get-pod branch from c3147d1 to 06aa81b Compare January 22, 2020 08:42

lzhecheng force-pushed the get-pod branch from 06aa81b to 75e5377 Compare February 4, 2020 08:11

jianjuns reviewed Feb 5, 2020

View reviewed changes

lzhecheng changed the title ~~Implement antctl get pod [podName]~~ WIP: Implement antctl get pod [podName] Feb 5, 2020

lzhecheng force-pushed the get-pod branch 2 times, most recently from afc56b7 to 53fc250 Compare February 6, 2020 10:54

lzhecheng force-pushed the get-pod branch 3 times, most recently from e8f65f0 to 251059a Compare March 9, 2020 09:45

tnqn reviewed Mar 9, 2020

View reviewed changes

lzhecheng force-pushed the get-pod branch 4 times, most recently from dfaefd5 to 96a3cb1 Compare March 10, 2020 07:30

lzhecheng force-pushed the get-pod branch from 1008e77 to d8bf1d8 Compare March 17, 2020 06:14

tnqn reviewed Mar 17, 2020

View reviewed changes

lzhecheng force-pushed the get-pod branch 2 times, most recently from ab76612 to 8280564 Compare March 17, 2020 11:22

lzhecheng requested a review from tnqn March 17, 2020 11:22

lzhecheng requested review from jianjuns and antoninbas March 18, 2020 06:08

tnqn reviewed Mar 18, 2020

View reviewed changes

lzhecheng force-pushed the get-pod branch 2 times, most recently from d2aa81a to 92342ae Compare March 18, 2020 10:58

tnqn reviewed Mar 18, 2020

View reviewed changes

lzhecheng force-pushed the get-pod branch from 92342ae to c60515a Compare March 18, 2020 12:38

weiqiangt reviewed Mar 18, 2020

View reviewed changes

jianjuns reviewed Mar 18, 2020

View reviewed changes

lzhecheng force-pushed the get-pod branch from c60515a to 4c0acd6 Compare March 19, 2020 02:06

tnqn approved these changes Mar 19, 2020

View reviewed changes

lzhecheng merged commit 3d5e14c into antrea-io:master Mar 19, 2020

lzhecheng deleted the get-pod branch March 19, 2020 07:21

ZhangYW18 added a commit to ZhangYW18/antrea that referenced this pull request Mar 23, 2020

Revert "Implemented antctl subcommand: pod-interface (antrea-io#334)"

73ff74b

This reverts commit eee580b.

ZhangYW18 added a commit to ZhangYW18/antrea that referenced this pull request Mar 23, 2020

Revert "Implemented antctl subcommand: pod-interface (antrea-io#334)"

e27f2d2

This reverts commit eee580b.

Implement antctl get pod-interface [podName] #334

Implement antctl get pod-interface [podName] #334

Conversation

lzhecheng commented Jan 20, 2020 • edited Loading

antrea-bot commented Jan 20, 2020

lzhecheng commented Jan 21, 2020

lzhecheng commented Jan 21, 2020

antoninbas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lzhecheng Jan 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jianjuns left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jianjuns left a comment

Choose a reason for hiding this comment

lzhecheng commented Feb 5, 2020 • edited Loading

lzhecheng commented Feb 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lzhecheng Mar 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tnqn commented Mar 9, 2020

lzhecheng commented Mar 10, 2020

lzhecheng commented Mar 17, 2020

lzhecheng commented Mar 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lzhecheng commented Mar 17, 2020

lzhecheng commented Mar 18, 2020

lzhecheng commented Mar 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lzhecheng Mar 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weiqiangt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lzhecheng commented Jan 20, 2020 •

edited

Loading

lzhecheng Jan 22, 2020 •

edited

Loading

lzhecheng commented Feb 5, 2020 •

edited

Loading

lzhecheng Mar 10, 2020 •

edited

Loading

lzhecheng Mar 18, 2020 •

edited

Loading