Refactor antctl framework #356

weiqiangt · 2020-02-04T03:40:58Z

Set up an API server on antrea agent
The antctl client is consuming the API server in antrea agent
instead of using the specific antctl server
Added API /networkpolicies, /appliedtogroups, /addressgroups,
/agentinfo and /version APIs to the agent API server
Add get network-policy, get address-group and get applied-to-group
commands for both controller and agent
Make GetVersion in version.go returning a semantic version object.

By applying network policy:

apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
  name: test-network-policy
  namespace: default
spec:
  podSelector:
    matchLabels:
      app: myapp
  policyTypes:
  - Ingress
  - Egress
  ingress:
  - from:
    - ipBlock:
        cidr: 172.17.0.0/16
        except:
        - 172.17.1.0/24
    - podSelector:
        matchLabels:
          app: myapp
    ports:
    - protocol: TCP
      port: 6379
  egress:
  - to:
    - ipBlock:
        cidr: 10.0.0.0/24
    ports:
    - protocol: TCP
      port: 5978

Antrea agent:

antctl get rule -oyaml

- direction: In
  from:
    addressGroups:
    - 5e8df597-d026-5144-bdc7-cd162c389dc3
    ipblocks:
    - cidr: 172.17.0.0/16
      except:
      - 172.17.1.0/24
  services:
  - protocol: TCP
    port: "6379"
- direction: Out
  to:
    ipblocks:
    - cidr: 10.0.0.0/24
  services:
  - protocol: TCP
    port: "5978"

antctl get applied-to-group -oyaml

- name: 5e8df597-d026-5144-bdc7-cd162c389dc3
  pods:
  - pod:
      name: myapp-pod-1
      namespace: default
  - pod:
      name: myapp-pod
      namespace: default

antctl get address-group -oyaml

- name: 5e8df597-d026-5144-bdc7-cd162c389dc3
  pods:
  - ip: 10.10.1.4
  - ip: 10.10.1.3

Antrea controller:

antctl get network-policy -oyaml

- name: test-network-policy
  rules:
  - direction: In
    from:
      addressGroups:
      - 5e8df597-d026-5144-bdc7-cd162c389dc3
      ipblocks:
      - cidr: 172.17.0.0/16
        except:
        - 172.17.1.0/24
    services:
    - protocol: TCP
      port:
        type: 0
        intval: 6379
        strval: ""
  - direction: Out
    to:
      ipblocks:
      - cidr: 10.0.0.0/24
    services:
    - protocol: TCP
      port:
        type: 0
        intval: 5978
        strval: ""
  appliedToGroups:
  - 5e8df597-d026-5144-bdc7-cd162c389dc3

antctl get applied-to-group -oyaml

- name: 5e8df597-d026-5144-bdc7-cd162c389dc3
  pods:
  - pod:
      name: myapp-pod-1
      namespace: default
  - pod:
      name: myapp-pod
      namespace: default

antctl get address-group -oyaml

- name: 5e8df597-d026-5144-bdc7-cd162c389dc3
  pods:
  - ip: 10.10.1.4
  - ip: 10.10.1.3

Resolves #239.

antrea-bot · 2020-02-04T03:41:10Z

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.

antoninbas · 2020-02-05T19:38:49Z

@weiqiangt would you mind moving the openapi stuff to a separate PR (along with the license template change) - and reference #323 in that new PR

Removing the autogenerated code from your anctl refactor PR will make it easier to review

weiqiangt · 2020-02-06T01:05:39Z

@weiqiangt would you mind moving the openapi stuff to a separate PR (along with the license template change) - and reference #323 in that new PR

Removing the autogenerated code from your anctl refactor PR will make it easier to review

Sure, I will separate this CL.

lzhecheng · 2020-02-06T02:55:10Z

Please push your patch again to refresh jenkins jobs' status.

jianjuns

@weiqiangt: thanks for working on this. I guess there can be some misunderstanding in our discussion - what I suggested is that even for Agent, we should move the API server code out of antctl, as even Agent API can be consumed by other clients besides antctl, and one specific example is the Controller, if we want to query Agent info or send ops requests to Agent from a single entity - that is Antrea Controller (it might also mean later we might need to expose Agent API through the Node IP).
Another one - even for Controller, could we also expose some API through UDS, like controller-info, version, etc., so that even when K8s API is down, we can at least do some basic debugging using local CLI. I do not think this is critical though.

pkg/antctl/antctl.go

pkg/antctl/handlers/doc.go

pkg/antctl/server.go

tnqn

Last two questions

tnqn · 2020-02-27T03:49:22Z

pkg/version/version.go

+func GetMajorVersion() string {
+	v, err := semver.Parse(Version[1:])
+	if err != nil {
+		println(err)


why not using log but output directly?

Since the Version is basically a constant, I think we could just ignore the error. Let me remove this error handling.

I removed GetMajorVersion and GetMinorVersion and made the GetVersion to return a semantic version object.

tnqn · 2020-02-27T03:51:47Z

pkg/version/version.go

+		println(err)
+		return "0"
+	}
+	return fmt.Sprintf("%d.%d", v.Minor, v.Patch)


Why merge minor and patch to minor? I think k8sversion.Info's intention is major for major, minor for minor, gitversion for full version (the Version in this file):

version.Info{Major:"1", Minor:"16", GitVersion:"v1.16.3", GitCommit:"b3cbbae08ec52a7fc73d334838e18d17e8512749", GitTreeState:"clean", BuildDate:"2019-11-13T11:13:49Z", GoVersion:"go1.12.12", Compiler:"gc", Platform:"linux/amd64"}

I didn't know the git version is used as full version, I would update it.

weiqiangt · 2020-02-27T04:40:36Z

/test-all

tnqn

LGTM

tnqn · 2020-02-27T08:21:01Z

I think you need to update the PR description now.

@jianjuns the PR looks good to me now, could you give another review?

pkg/agent/server/server.go

weiqiangt · 2020-03-02T05:57:27Z

/test-all

jianjuns · 2020-03-02T06:17:49Z

pkg/agent/apiserver/apiserver.go

+	// Set the PairName but leave certificate directory blank to generate in-memory by default.
+	secureServing.ServerCert.CertDirectory = ""
+	secureServing.ServerCert.PairName = Name
+	ln, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", Port))


I thought I added a comment here before but I could not find it any more. Anyway, have a question here: could we still enable unix domain socket for local CLI? I remember we discussed before security concerns on localhost TCP port without auth. Ideally we should enable the API server on both UDS and later host IP (for remote CLI, and potentially other consumers like Controller, support bundle collection, etc.).
@tnqn

Is 127.0.0.1 same as UDS from security perspective? Do you mean anonymous user can get into the container and access the loopback address?

Yeah, the server which leverages built-in server used uds, but it looks like the APIServer is not able to set up an UDS listener (it looks up port for some reasons).

Yes, one issue is about no way to restrict what local users/processes can access the API endpoint.
If no good way, we can merge the current implementation for now. But let us explore the possibility of enable UDS.

lzhecheng · 2020-03-02T06:28:55Z

/test-all

weiqiangt · 2020-03-02T07:14:23Z

/test-e2e

jianjuns · 2020-03-03T00:47:13Z

I think you need to update the PR description now.

@jianjuns the PR looks good to me now, could you give another review?

LGTM overall. Sorry, I did not get time to do a detailed review. I will try to read the code more, but do not wait for me to merge the PR.

weiqiangt · 2020-03-03T02:57:35Z

/test-e2e

weiqiangt · 2020-03-03T03:36:58Z

/test-all

weiqiangt · 2020-03-03T08:04:39Z

/test-e2e

- Set up an API server on antrea agent - The antctl client is consuming the API server in antrea agent instead of using the specific antctl server - Added API `/rule`, `/appliedtogroup`, `/addressgroup`, `/agentinfo` and `version` APIs to the agent API server - Add `get network-policy`, `get address-group` and `get applied-to-group` commands for controller - Add `get rule`, `get address-group` and `get applied-to-group` commands for agent - Make version file to honor semantic version Signed-off-by: Weiqiang TANG <weiqiangt@vmware.com>

lzhecheng · 2020-03-04T02:09:58Z

/test-e2e

lzhecheng · 2020-03-04T07:59:35Z

/test-e2e

lzhecheng · 2020-03-04T12:50:55Z

/test-e2e

weiqiangt · 2020-03-04T13:07:43Z

/test-all

lzhecheng · 2020-03-04T13:20:24Z

/test-all

vmwclabot added the cla-not-required label Feb 4, 2020

weiqiangt force-pushed the antctl-refactor branch from 4aa3219 to eab31bd Compare February 4, 2020 03:53

weiqiangt requested review from lzhecheng, vicky-liu and jianjuns February 4, 2020 06:48

weiqiangt force-pushed the antctl-refactor branch 2 times, most recently from dfac9a6 to fed76f5 Compare February 4, 2020 13:22

weiqiangt requested a review from antoninbas February 4, 2020 13:28

weiqiangt force-pushed the antctl-refactor branch 2 times, most recently from 8a48f6d to b626119 Compare February 5, 2020 02:49

weiqiangt changed the title ~~[WIP] Refactor antctl framework~~ Refactor antctl framework Feb 6, 2020

weiqiangt requested a review from tnqn February 6, 2020 00:54

weiqiangt force-pushed the antctl-refactor branch 2 times, most recently from e4f7392 to bd0e17b Compare February 6, 2020 01:56

weiqiangt force-pushed the antctl-refactor branch from bd0e17b to 248898e Compare February 6, 2020 08:04

jianjuns reviewed Feb 7, 2020

View reviewed changes

pkg/antctl/antctl.go Outdated Show resolved Hide resolved

pkg/antctl/handlers/doc.go Show resolved Hide resolved

pkg/antctl/server.go Outdated Show resolved Hide resolved

weiqiangt changed the title ~~Refactor antctl framework~~ [WIP]Refactor antctl framework Feb 7, 2020

weiqiangt force-pushed the antctl-refactor branch 7 times, most recently from 7469dc6 to cb86a58 Compare February 19, 2020 04:16

weiqiangt changed the title ~~[WIP]Refactor antctl framework~~ Refactor antctl framework Feb 19, 2020

weiqiangt requested a review from jianjuns February 19, 2020 05:17

tnqn reviewed Feb 27, 2020

View reviewed changes

weiqiangt force-pushed the antctl-refactor branch from 97aa7ad to bd242f7 Compare February 27, 2020 04:38

tnqn previously approved these changes Feb 27, 2020

View reviewed changes

jianjuns reviewed Mar 2, 2020

View reviewed changes

pkg/agent/server/server.go Outdated Show resolved Hide resolved

weiqiangt dismissed tnqn’s stale review via d372db0 March 2, 2020 05:35

weiqiangt force-pushed the antctl-refactor branch from bd242f7 to d372db0 Compare March 2, 2020 05:35

jianjuns reviewed Mar 2, 2020

View reviewed changes

weiqiangt force-pushed the antctl-refactor branch from d372db0 to 5dc1fdc Compare March 3, 2020 03:29

weiqiangt force-pushed the antctl-refactor branch 2 times, most recently from cbf0c79 to cc35c53 Compare March 3, 2020 07:19

weiqiangt force-pushed the antctl-refactor branch from cc35c53 to 13e40c4 Compare March 4, 2020 02:08

tnqn approved these changes Mar 4, 2020

View reviewed changes

weiqiangt merged commit 25ad09a into antrea-io:master Mar 4, 2020

tnqn mentioned this pull request Mar 11, 2020

Namespace cannot be deleted after upgrading antrea from 0.4.0 to 0.5.0-dev #494

Closed

weiqiangt deleted the antctl-refactor branch May 30, 2020 05:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor antctl framework #356

Refactor antctl framework #356

weiqiangt commented Feb 4, 2020 •

edited

Loading

antrea-bot commented Feb 4, 2020

antoninbas commented Feb 5, 2020 •

edited

Loading

weiqiangt commented Feb 6, 2020

lzhecheng commented Feb 6, 2020

jianjuns left a comment

tnqn left a comment

tnqn Feb 27, 2020

weiqiangt Feb 27, 2020

weiqiangt Feb 27, 2020 •

edited

Loading

tnqn Feb 27, 2020

weiqiangt Feb 27, 2020

weiqiangt Feb 27, 2020

weiqiangt commented Feb 27, 2020

tnqn left a comment

tnqn commented Feb 27, 2020

weiqiangt commented Mar 2, 2020

jianjuns Mar 2, 2020

tnqn Mar 2, 2020

weiqiangt Mar 2, 2020

jianjuns Mar 2, 2020

lzhecheng commented Mar 2, 2020

weiqiangt commented Mar 2, 2020

jianjuns commented Mar 3, 2020

weiqiangt commented Mar 3, 2020

weiqiangt commented Mar 3, 2020

weiqiangt commented Mar 3, 2020

lzhecheng commented Mar 4, 2020

lzhecheng commented Mar 4, 2020

lzhecheng commented Mar 4, 2020

weiqiangt commented Mar 4, 2020

lzhecheng commented Mar 4, 2020

Refactor antctl framework #356

Refactor antctl framework #356

Conversation

weiqiangt commented Feb 4, 2020 • edited Loading

antrea-bot commented Feb 4, 2020

antoninbas commented Feb 5, 2020 • edited Loading

weiqiangt commented Feb 6, 2020

lzhecheng commented Feb 6, 2020

jianjuns left a comment

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weiqiangt Feb 27, 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

weiqiangt commented Feb 27, 2020

tnqn left a comment

Choose a reason for hiding this comment

tnqn commented Feb 27, 2020

weiqiangt commented Mar 2, 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

lzhecheng commented Mar 2, 2020

weiqiangt commented Mar 2, 2020

jianjuns commented Mar 3, 2020

weiqiangt commented Mar 3, 2020

weiqiangt commented Mar 3, 2020

weiqiangt commented Mar 3, 2020

lzhecheng commented Mar 4, 2020

lzhecheng commented Mar 4, 2020

lzhecheng commented Mar 4, 2020

weiqiangt commented Mar 4, 2020

lzhecheng commented Mar 4, 2020

weiqiangt commented Feb 4, 2020 •

edited

Loading

antoninbas commented Feb 5, 2020 •

edited

Loading

weiqiangt Feb 27, 2020 •

edited

Loading