[ExternalNode] Add feature gate to run Agent on a non-Kubernetes Node #3542

wenyingd · 2022-03-30T02:27:31Z

Use feature gate "ExternalNode" to support Antrea Agent running
on a non-Kubernetes Node.
Support Agent running APIServer only on localhost if it is not
running on cluster worker Node.
CNIServer is loaded only when Agent is running on cluster worker
Node.

Signed-off-by: wenyingd wenyingd@vmware.com

wenyingd · 2022-03-30T03:04:39Z

/test-all
/test-windows-all
/test-ipv6-all
/test-ipv6-only-all

codecov-commenter · 2022-03-30T03:16:27Z

Codecov Report

Merging #3542 (aacc0f4) into feature/externalnode (3418e20) will decrease coverage by 7.91%.
The diff coverage is 56.19%.

@@                   Coverage Diff                    @@
##           feature/externalnode    #3542      +/-   ##
========================================================
- Coverage                 63.54%   55.62%   -7.92%     
========================================================
  Files                       278      392     +114     
  Lines                     39502    55133   +15631     
========================================================
+ Hits                      25101    30669    +5568     
- Misses                    12466    22089    +9623     
- Partials                   1935     2375     +440

Flag	Coverage Δ
integration-tests	`38.25% <ø> (?)`
kind-e2e-tests	`49.02% <55.23%> (-1.69%)`	⬇️
unit-tests	`43.82% <30.52%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/agent/openflow/pipeline.go	`72.72% <ø> (-1.78%)`	⬇️
pkg/features/antrea_features.go	`8.00% <0.00%> (-3.12%)`	⬇️
pkg/agent/agent.go	`53.60% <35.18%> (-1.33%)`	⬇️
pkg/util/env/env.go	`42.85% <57.14%> (+1.87%)`	⬆️
pkg/agent/config/node_config.go	`96.00% <85.71%> (-4.00%)`	⬇️
pkg/agent/apiserver/apiserver.go	`66.30% <100.00%> (ø)`
pkg/agent/openflow/client.go	`72.31% <100.00%> (+0.13%)`	⬆️
pkg/controller/egress/store/egressgroup.go	`1.72% <0.00%> (-54.32%)`	⬇️
pkg/agent/util/net.go	`29.33% <0.00%> (-22.23%)`	⬇️
pkg/agent/route/route_linux.go	`28.59% <0.00%> (-19.73%)`	⬇️
... and 133 more

wenyingd · 2022-03-30T08:08:07Z

/test-e2e
/test-networkpolicy
/test-windows-networkpolicy

jianjuns

Do we have a way to add a feature gate to isolate VM/BM code?

jianjuns · 2022-03-30T23:02:35Z

build/yamls/antrea-aks.yml

@@ -2975,6 +2975,12 @@ data:
      # Note that setting ProxyLoadBalancerIPs to false usually only makes sense when ProxyAll is set to true and
      # kube-proxy is removed from the cluser, otherwise kube-proxy will still load-balance this traffic.
      #proxyLoadBalancerIPs: true
+
+    # The role in which Antrea Agent is working. Supported values:


Again, can we generate a separate yaml for VM/BM, to avoid these confusing parameters for normal K8s?

I create a sperate build directory for VM/BM configuration, but I didn't modify the create generate-manifest script to generate yaml file because for VM/BM case antrea-agent.conf is used directly on agent side and we don't need a all-in-one yaml file to apply on K8s cluster. Besides, In @mengdie-song 's PR, she will create a separate CRD yaml file for VM agent which could be applied on the cluster, and I think that file should also place in the new configration directory.

I just mean could we find a way to avoid changing the regular yamls and adding VM only contents.

build/yamls/base/conf/antrea-agent.conf

jianjuns · 2022-03-30T23:06:06Z

build/yamls/base/conf/antrea-agent.conf

+
+# The role in which Antrea Agent is working. Supported values:
+# - Node (default)
+# - VM


Any difference between VM and BM and so we need two types? If not or not clear, let us just add one type called "VirtualMachine" for now.

cmd/antrea-agent/agent.go

pkg/agent/agent.go

pkg/features/antrea_features.go

cmd/antrea-agent/agent.go

pkg/agent/querier/querier.go

jianjuns · 2022-03-31T18:00:31Z

Any thoughts on this one?

Do we have a way to add a feature gate to isolate VM/BM code?

jianjuns · 2022-04-02T00:33:10Z

build/yamls/externalnode/conf/antrea-agent.conf

@@ -0,0 +1,39 @@
+# FeatureGates is a map of feature names to bools that enable or disable experimental features.


Probably change "externalnode" in the path to "vmnode".

Again, the problem with "externalnode" is that it can refer to a node/policy receiver without Agent running.

jianjuns · 2022-04-02T00:33:42Z

build/yamls/externalnode/conf/antrea-agent.conf

+# FeatureGates is a map of feature names to bools that enable or disable experimental features.
+featureGates:
+# Enable running agent on a Virtual Machine.
+  ExternalNode: true


Probably call it "VirtualMachineNode".

cmd/antrea-agent/options.go

docs/feature-gates.md

cmd/antrea-agent/options.go

jianjuns · 2022-04-06T18:03:23Z

cmd/antrea-agent/options.go

+	if o.config.TunnelType == "" {
+		o.config.TunnelType = defaultTunnelType
+	}
+	if o.config.ServiceCIDR == "" {


Do we still need this for VM? Why?

No, this is to avoid agent crash on line 140 in cmd/antrea-agent/agent.go. o.config.ServiceCIDR is used to parse the serviceCIDR directly. Maybe I should add feature gate check in that logic.

pkg/agent/agent.go

pkg/agent/agent_windows.go

jianjuns · 2022-04-06T18:13:30Z

pkg/agent/config/node_config.go

@@ -59,6 +59,13 @@ var (
 	VirtualServiceIPv6 = net.ParseIP("fc01::aabb:ccdd:eeff")
 )

+type NodeType string


If we often compare the values, probably use enum.

With feature gate, this is not ofen used in value comparasion. I want to keep string value because I planed to show the Node type in AntreaAgentInfo in future.

feature gate is not used in "inner" pkgs. Always converted to a boolean.

I mean if we compare the values in any frequent call path (e.g. when handling a new change from API or CNI), I think enum is better. If it is just used in bootstrapping, string is fine. Even for enum, you can add a String() func.

jianjuns · 2022-04-07T23:14:24Z

cmd/antrea-agent/options.go

+
+func (o *Options) validateVMOptions() error {
+	var unsupported []string
+	for f, enabled := range o.config.FeatureGates {


There can be features not enabled by feature gate (e.g. encapMode, enableNodePortLocal whose feature gate is enabled by default). Could you check any such features should be validated here too?

I assume you check all the options and no other unsupported options?

Made two changes:

Add unsupported options check with EnableIPSecTunnel, although it is deprecated, the encryptionMode is possibly changed to TrafficEncryptionModeIPSec if it is set as true

Set encryptionMode default value as TrafficEncryptionModeNone in Options.run function, and load value from the configuration only when encapMode requires supporting Encap ( the ToT code actually has a useless configuration parsing with noEncap mode ).

docs/feature-gates.md

pkg/agent/agent.go

pkg/agent/agent_windows.go

jianjuns · 2022-04-07T23:27:03Z

pkg/agent/config/node_config.go

@@ -59,6 +59,13 @@ var (
 	VirtualServiceIPv6 = net.ParseIP("fc01::aabb:ccdd:eeff")
 )

+type NodeType string


feature gate is not used in "inner" pkgs. Always converted to a boolean.

I mean if we compare the values in any frequent call path (e.g. when handling a new change from API or CNI), I think enum is better. If it is just used in bootstrapping, string is fine. Even for enum, you can add a String() func.

pkg/util/env/env.go

jianjuns · 2022-04-07T23:34:24Z

pkg/util/env/env.go

+	NodeNameEnvKey = "NODE_NAME"
+	// ExternalNodeEnvKey is an environment variable used in ExternalNode feature to tell the expected name with which
+	// antrea agent is running. This is also used in the shell where antctl is running on a VM host.
+	ExternalNodeEnvKey    = "EXTERNAL_NODE"


Is it just a name string? If so, why not reuse "NODE_NAME"?

This is mainly for antctl support. I need a different env variable to tell antctl it is running on a VM but not a K8s Node.

Do we assume antctl always runs in a container? If not, the env will not be set anyway right?

I feel better to use another env variable to indicate node type for antctl, and also add a parameter for antctl for the case it runs without an env.

pkg/util/env/env.go

jianjuns · 2022-04-07T23:35:35Z

pkg/util/env/env.go

+	if nodeName != "" {
+		return
+	}
+	klog.Infof("Environment variable %s not found, using hostname instead", ExternalNodeEnvKey)


Better to print NodeNameEnvKey?

NodeNameEnvKey is printed in line 56, maybe I should remove line 56 and change to NodeNameEnvKey in this line.

wenyingd · 2022-04-08T02:47:10Z

/test-all
/test-ipv6-all
/test-ipv6-only-all
/test-windows-all

wenyingd · 2022-04-26T11:27:07Z

/test-windows-e2e
/test-windows-conformance

tnqn · 2022-04-26T13:06:37Z

/test-conformance

tnqn · 2022-04-26T16:05:20Z

build/yamls/externalnode/conf/antrea-agent.conf

+# The path to access the kubeconfig file used in the connection to Antrea Controller. The file contains the
+# antrea-controller APIServer endpoint and the token of ServiceAccount required in the connection.
+antreaClientConnection:
+  kubeconfig: antrea-agent.antrea.kubeconfig


@wenyingd should we use helm to generate this file based on template? Otherwise we would need to update this file manually whenever there is a change to related parameters? I don't mean it needs to be done in this PR.

…3542) 1. Add feature gate for ExternalNode which enables running Agent on a VM or BM. 2. Support Agent running APIServer only on localhost if it is not running on cluster worker Node. 3. CNIServer is loaded only when Agent is running on cluster worker Node. 4. Use a seperate build directory to generate agent configrations for ExternalNode. Signed-off-by: wenyingd <wenyingd@vmware.com>

wenyingd force-pushed the vmbm_agent_role branch from b3d3574 to 24a15dc Compare March 30, 2022 02:54

wenyingd requested review from tnqn, jianjuns and mengdie-song and removed request for tnqn March 30, 2022 03:03

jianjuns reviewed Mar 30, 2022

View reviewed changes

mengdie-song reviewed Mar 31, 2022

View reviewed changes

cmd/antrea-agent/agent.go Outdated Show resolved Hide resolved

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

wenyingd force-pushed the vmbm_agent_role branch 5 times, most recently from 92a71f8 to be15219 Compare March 31, 2022 11:44

wenyingd force-pushed the vmbm_agent_role branch 2 times, most recently from 2a7178d to e826128 Compare April 1, 2022 16:00

jianjuns reviewed Apr 2, 2022

View reviewed changes

wenyingd force-pushed the vmbm_agent_role branch from e826128 to 75c76cf Compare April 6, 2022 11:25

jianjuns reviewed Apr 6, 2022

View reviewed changes

docs/feature-gates.md Outdated Show resolved Hide resolved

docs/feature-gates.md Outdated Show resolved Hide resolved

docs/feature-gates.md Outdated Show resolved Hide resolved

docs/feature-gates.md Outdated Show resolved Hide resolved

jianjuns reviewed Apr 6, 2022

View reviewed changes

wenyingd force-pushed the feature/externalnode branch from e34639d to 03b3f2b Compare April 7, 2022 11:15

wenyingd force-pushed the vmbm_agent_role branch from 75c76cf to 09edb18 Compare April 7, 2022 11:33

jianjuns reviewed Apr 7, 2022

View reviewed changes

wenyingd force-pushed the vmbm_agent_role branch from 09edb18 to f3804b9 Compare April 8, 2022 01:33

wenyingd force-pushed the vmbm_agent_role branch from f3804b9 to 178dfc7 Compare April 8, 2022 09:20

tnqn reviewed Apr 26, 2022

View reviewed changes

tnqn merged commit 3f35235 into antrea-io:feature/externalnode Apr 26, 2022

wenyingd deleted the vmbm_agent_role branch April 28, 2022 06:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ExternalNode] Add feature gate to run Agent on a non-Kubernetes Node #3542

[ExternalNode] Add feature gate to run Agent on a non-Kubernetes Node #3542

wenyingd commented Mar 30, 2022 •

edited

Loading

wenyingd commented Mar 30, 2022

codecov-commenter commented Mar 30, 2022 •

edited

Loading

wenyingd commented Mar 30, 2022

jianjuns left a comment

jianjuns Mar 30, 2022

wenyingd Mar 31, 2022

jianjuns Mar 31, 2022

jianjuns Mar 30, 2022

jianjuns commented Mar 31, 2022

jianjuns Apr 2, 2022

jianjuns Apr 2, 2022

jianjuns Apr 2, 2022

jianjuns Apr 6, 2022

wenyingd Apr 7, 2022

jianjuns Apr 6, 2022

wenyingd Apr 7, 2022

jianjuns Apr 7, 2022

jianjuns Apr 7, 2022

wenyingd Apr 8, 2022

jianjuns Apr 8, 2022

wenyingd Apr 11, 2022

jianjuns Apr 7, 2022

jianjuns Apr 7, 2022

wenyingd Apr 8, 2022

jianjuns Apr 8, 2022

jianjuns Apr 7, 2022

wenyingd Apr 8, 2022

wenyingd commented Apr 8, 2022

wenyingd commented Apr 26, 2022

tnqn commented Apr 26, 2022

tnqn Apr 26, 2022

		@@ -0,0 +1,39 @@
		# FeatureGates is a map of feature names to bools that enable or disable experimental features.

[ExternalNode] Add feature gate to run Agent on a non-Kubernetes Node #3542

[ExternalNode] Add feature gate to run Agent on a non-Kubernetes Node #3542

Conversation

wenyingd commented Mar 30, 2022 • edited Loading

wenyingd commented Mar 30, 2022

codecov-commenter commented Mar 30, 2022 • edited Loading

Codecov Report

wenyingd commented Mar 30, 2022

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jianjuns commented Mar 31, 2022

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

Choose a reason for hiding this comment

wenyingd commented Apr 8, 2022

wenyingd commented Apr 26, 2022

tnqn commented Apr 26, 2022

Choose a reason for hiding this comment

wenyingd commented Mar 30, 2022 •

edited

Loading

codecov-commenter commented Mar 30, 2022 •

edited

Loading