Repository: antrea-io/antrea
Pull request #3278 (2022-02-01 02:03:15)

Configure crictl correctly in Vagrant testbed
Set endpoints correctly for containerd to avoid error logs.

Signed-off-by: Antonin Bas <abas@vmware.com>

codecov-commenter: # [Codecov](https://codecov.io/gh/antrea-io/antrea/pull/3278?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#3278](https://codecov.io/gh/antrea-io/antrea/pull/3278?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (eb5d4f9) into [main](https://codecov.io/gh/antrea-io/antrea/commit/e23cf3b5714d15ae66a33196c1f870196065f651?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (e23cf3b) will **decrease** coverage by `16.13%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/antrea-io/antrea/pull/3278/graphs/tree.svg?width=650&height=150&src=pr&token=TjWaLL2GoC&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)](https://codecov.io/gh/antrea-io/antrea/pull/3278?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

```diff
@@             Coverage Diff             @@
##             main    #3278       +/-   ##
===========================================
- Coverage   57.99%   41.86%   -16.14%     
===========================================
  Files         331      199      -132     
  Lines       28444    24106     -4338     
===========================================
- Hits        16495    10091     -6404     
- Misses      10158    12984     +2826     
+ Partials     1791     1031      -760     
```

| Flag | Coverage Δ | |
|---|---|---|
| kind-e2e-tests | `?` | |
| unit-tests | `41.86% <ø> (+<0.01%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/antrea-io/antrea/pull/3278?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [pkg/ovs/openflow/default.go](https://codecov.io/gh/antrea-io/antrea/pull/3278/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cGtnL292cy9vcGVuZmxvdy9kZWZhdWx0Lmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [pkg/util/runtime/runtime.go](https://codecov.io/gh/antrea-io/antrea/pull/3278/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cGtnL3V0aWwvcnVudGltZS9ydW50aW1lLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [pkg/agent/nodeportlocal/k8s/annotations.go](https://codecov.io/gh/antrea-io/antrea/pull/3278/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cGtnL2FnZW50L25vZGVwb3J0bG9jYWwvazhzL2Fubm90YXRpb25zLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [pkg/ovs/openflow/logs.go](https://codecov.io/gh/antrea-io/antrea/pull/3278/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cGtnL292cy9vcGVuZmxvdy9sb2dzLmdv) | `9.52% <0.00%> (-90.48%)` | :arrow_down: |
| [pkg/apis/controlplane/register.go](https://codecov.io/gh/antrea-io/antrea/pull/3278/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cGtnL2FwaXMvY29udHJvbHBsYW5lL3JlZ2lzdGVyLmdv) | `0.00% <0.00%> (-90.00%)` | :arrow_down: |
| [pkg/agent/controller/networkpolicy/reject.go](https://codecov.io/gh/antrea-io/antrea/pull/3278/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cGtnL2FnZW50L2NvbnRyb2xsZXIvbmV0d29ya3BvbGljeS9yZWplY3QuZ28=) | `0.00% <0.00%> (-87.91%)` | :arrow_down: |
| [pkg/agent/client.go](https://codecov.io/gh/antrea-io/antrea/pull/3278/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cGtnL2FnZW50L2NsaWVudC5nbw==) | `0.00% <0.00%> (-77.42%)` | :arrow_down: |
| [pkg/ovs/ovsconfig/ovs\_client\_linux.go](https://codecov.io/gh/antrea-io/antrea/pull/3278/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cGtnL292cy9vdnNjb25maWcvb3ZzX2NsaWVudF9saW51eC5nbw==) | `0.00% <0.00%> (-76.93%)` | :arrow_down: |
| [pkg/flowaggregator/certificate.go](https://codecov.io/gh/antrea-io/antrea/pull/3278/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cGtnL2Zsb3dhZ2dyZWdhdG9yL2NlcnRpZmljYXRlLmdv) | `0.00% <0.00%> (-76.58%)` | :arrow_down: |
| [...agent/controller/traceflow/traceflow\_controller.go](https://codecov.io/gh/antrea-io/antrea/pull/3278/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cGtnL2FnZW50L2NvbnRyb2xsZXIvdHJhY2VmbG93L3RyYWNlZmxvd19jb250cm9sbGVyLmdv) | `0.00% <0.00%> (-73.41%)` | :arrow_down: |
| ... and [243 more](https://codecov.io/gh/antrea-io/antrea/pull/3278/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | |


Patch:
diff --git a/CHANGELOG/CHANGELOG-1.5.md b/CHANGELOG/CHANGELOG-1.5.md
index bfda6c555..0c2dc181d 100644
--- a/CHANGELOG/CHANGELOG-1.5.md
+++ b/CHANGELOG/CHANGELOG-1.5.md
@@ -16,7 +16,7 @@
 - Add support for SR-IOV secondary network. Antrea can now create secondary network interfaces for Pods using SR-IOV VFs on bare metal Nodes. ([#2651](https://github.com/antrea-io/antrea/pull/2651), [@arunvelayutham]) [Alpha - Feature Gate: `SecondaryNetwork`]
 - Add support for allocating external IPs for Services of type LoadBalancer from an ExternalIPPool. ([#3147](https://github.com/antrea-io/antrea/pull/3147) [@Shengkai2000]) [Alpha - Feature Gate: `ServiceExternalIP`]
 - Add support for antctl in the flow aggregator Pod. ([#2878](https://github.com/antrea-io/antrea/pull/2878), [@yanjunz97])
-  * Support `antctl get log-level` for changing log verbosity level
+  * Support `antctl log-level` for changing log verbosity level
   * Support `antctl get flowrecords [-o json]` for dumping flow records
   * Support `antctl get recordmetrics` for dumping flow records metrics
 - Add support for the "Pass" action in Antrea-native policies to skip evaluation of further Antrea-native policy rules and delegate evaluation to Kubernetes NetworkPolicy. ([#2964](https://github.com/antrea-io/antrea/pull/2964), [@Dyanngg])
diff --git a/CHANGELOG/README.md b/CHANGELOG/README.md
index 2075b67c4..a97ab1da8 100644
--- a/CHANGELOG/README.md
+++ b/CHANGELOG/README.md
@@ -10,6 +10,7 @@ stages](https://github.com/kubernetes/community/blob/master/contributors/devel/s
 Some experimental features can be enabled / disabled using [Feature
 Gates](../docs/feature-gates.md).
 
+- [CHANGELOG-1.5](CHANGELOG-1.5.md)
 - [CHANGELOG-1.4](CHANGELOG-1.4.md)
 - [CHANGELOG-1.3](CHANGELOG-1.3.md)
 - [CHANGELOG-1.2](CHANGELOG-1.2.md)
diff --git a/cmd/antrea-agent/agent.go b/cmd/antrea-agent/agent.go
index c80035c4d..eecb52fce 100644
--- a/cmd/antrea-agent/agent.go
+++ b/cmd/antrea-agent/agent.go
@@ -451,7 +451,9 @@ func run(o *Options) error {
 
 	// Initialize localPodInformer for NPLAgent and AntreaIPAMController
 	var localPodInformer cache.SharedIndexInformer
-	if features.DefaultFeatureGate.Enabled(features.NodePortLocal) && o.config.NodePortLocal.Enable || features.DefaultFeatureGate.Enabled(features.AntreaIPAM) {
+	if features.DefaultFeatureGate.Enabled(features.NodePortLocal) && o.config.NodePortLocal.Enable ||
+		features.DefaultFeatureGate.Enabled(features.AntreaIPAM) ||
+		features.DefaultFeatureGate.Enabled(features.SecondaryNetwork) {
 		listOptions := func(options *metav1.ListOptions) {
 			options.FieldSelector = fields.OneTermEqualSelector("spec.nodeName", nodeConfig.Name).String()
 		}
@@ -463,6 +465,19 @@ func run(o *Options) error {
 			listOptions,
 		)
 	}
+
+	log.StartLogFileNumberMonitor(stopCh)
+
+	go routeClient.Run(stopCh)
+
+	go cniServer.Run(stopCh)
+
+	go antreaClientProvider.Run(stopCh)
+
+	go nodeRouteController.Run(stopCh)
+
+	go networkPolicyController.Run(stopCh)
+
 	// Initialize the NPL agent.
 	if features.DefaultFeatureGate.Enabled(features.NodePortLocal) && o.config.NodePortLocal.Enable {
 		nplController, err := npl.InitializeNPLAgent(
@@ -477,6 +492,7 @@ func run(o *Options) error {
 		}
 		go nplController.Run(stopCh)
 	}
+
 	// Initialize the Antrea IPAM agent.
 	if features.DefaultFeatureGate.Enabled(features.AntreaIPAM) {
 		ipamController, err := ipam.InitializeAntreaIPAMController(
@@ -490,25 +506,6 @@ func run(o *Options) error {
 		}
 		go ipamController.Run(stopCh)
 	}
-	//  Start the localPodInformer
-	if localPodInformer != nil {
-		go localPodInformer.Run(stopCh)
-	}
-
-	log.StartLogFileNumberMonitor(stopCh)
-
-	go routeClient.Run(stopCh)
-
-	go cniServer.Run(stopCh)
-
-	informerFactory.Start(stopCh)
-	crdInformerFactory.Start(stopCh)
-
-	go antreaClientProvider.Run(stopCh)
-
-	go nodeRouteController.Run(stopCh)
-
-	go networkPolicyController.Run(stopCh)
 
 	if features.DefaultFeatureGate.Enabled(features.SecondaryNetwork) {
 		// Create the NetworkAttachmentDefinition client, which handles access to secondary network object definition from the API Server.
@@ -516,20 +513,25 @@ func run(o *Options) error {
 		if err != nil {
 			return fmt.Errorf("NetworkAttachmentDefinition client creation failed. %v", err)
 		}
-		// Initialize podController to handle secondary network configuration for Pods with k8s.v1.cni.cncf.io/networks Annotation defined.
-		podWatchController, err := podwatch.InitializePodController(
+		// Create podController to handle secondary network configuration for Pods with k8s.v1.cni.cncf.io/networks Annotation defined.
+		podWatchController := podwatch.NewPodController(
 			k8sClient,
 			netAttachDefClient,
-			informerFactory,
+			localPodInformer,
 			nodeConfig.Name,
 			cniPodInfoStore,
 			cniServer)
-		if err != nil {
-			return fmt.Errorf("failed to initialize Pod Controller for secondary network interface management: %v", err)
-		}
 		go podWatchController.Run(stopCh)
 	}
 
+	//  Start the localPodInformer
+	if localPodInformer != nil {
+		go localPodInformer.Run(stopCh)
+	}
+
+	informerFactory.Start(stopCh)
+	crdInformerFactory.Start(stopCh)
+
 	if features.DefaultFeatureGate.Enabled(features.Egress) || features.DefaultFeatureGate.Enabled(features.ServiceExternalIP) {
 		go externalIPPoolController.Run(stopCh)
 		go localIPDetector.Run(stopCh)
diff --git a/docs/antrea-network-policy.md b/docs/antrea-network-policy.md
index e6b6e5a72..35eede834 100644
--- a/docs/antrea-network-policy.md
+++ b/docs/antrea-network-policy.md
@@ -836,7 +836,7 @@ status:
       lastTransitionTime: "2021-01-29T20:21:48Z"
 ```
 
-There are a few __restrictions__ on how ClusterGroups can be configured:
+There are a few **restrictions** on how ClusterGroups can be configured:
 
 - A ClusterGroup is a cluster-scoped resource and therefore can only be set in an Antrea
   ClusterNetworkPolicy's `appliedTo` and `to`/`from` peers.
diff --git a/pkg/agent/controller/serviceexternalip/controller.go b/pkg/agent/controller/serviceexternalip/controller.go
index 0327ce933..7c28e236e 100644
--- a/pkg/agent/controller/serviceexternalip/controller.go
+++ b/pkg/agent/controller/serviceexternalip/controller.go
@@ -201,7 +201,11 @@ func (c *ServiceExternalIPController) enqueueServiceForEndpoints(obj interface{}
 	}
 	service, err := c.serviceLister.Services(endpoints.Namespace).Get(endpoints.Name)
 	if err != nil {
-		klog.ErrorS(err, "failed to get Service for Endpoint", "namespace", endpoints.Namespace, "name", endpoints.Name)
+		// The only possible error Lister.Get can return is NotFound.
+		// It's normal that some Endpoints don't have Service. For example, kube-scheduler and kube-controller-manager
+		// may use Endpoints for leader election. Even if the Endpoints should have a Service but it's not received yet,
+		// it's fine to ignore the error as the Service's add event will enqueue it.
+		klog.V(5).InfoS("Failed to get Service for Endpoints", "Endpoints", klog.KObj(endpoints), "err", err)
 		return
 	}
 	// we only care services with ServiceExternalTrafficPolicy setting to local.
diff --git a/pkg/agent/nodeportlocal/k8s/npl_controller.go b/pkg/agent/nodeportlocal/k8s/npl_controller.go
index d313673af..ba152354a 100644
--- a/pkg/agent/nodeportlocal/k8s/npl_controller.go
+++ b/pkg/agent/nodeportlocal/k8s/npl_controller.go
@@ -42,10 +42,14 @@ import (
 )
 
 const (
-	controllerName = "AntreaAgentNPLController"
+	controllerName = "NPLController"
 	minRetryDelay  = 2 * time.Second
 	maxRetryDelay  = 120 * time.Second
 	numWorkers     = 4
+
+	// Set resyncPeriod to 0 to disable resyncing.
+	// UpdateFunc event handler will be called only when the object is actually updated.
+	resyncPeriod = 0 * time.Minute
 )
 
 type NPLController struct {
@@ -63,7 +67,6 @@ type NPLController struct {
 func NewNPLController(kubeClient clientset.Interface,
 	podInformer cache.SharedIndexInformer,
 	svcInformer cache.SharedIndexInformer,
-	resyncPeriod time.Duration,
 	pt *portcache.PortTable,
 	nodeName string) *NPLController {
 	c := NPLController{
diff --git a/pkg/agent/nodeportlocal/npl_agent_init.go b/pkg/agent/nodeportlocal/npl_agent_init.go
index fb0336913..b2b09ef42 100644
--- a/pkg/agent/nodeportlocal/npl_agent_init.go
+++ b/pkg/agent/nodeportlocal/npl_agent_init.go
@@ -19,7 +19,6 @@ package nodeportlocal
 
 import (
 	"fmt"
-	"time"
 
 	nplk8s "antrea.io/antrea/pkg/agent/nodeportlocal/k8s"
 	"antrea.io/antrea/pkg/agent/nodeportlocal/portcache"
@@ -29,10 +28,6 @@ import (
 	"k8s.io/client-go/tools/cache"
 )
 
-// Set resyncPeriod to 0 to disable resyncing.
-// UpdateFunc event handler will be called only when the object is actually updated.
-const resyncPeriod = 0 * time.Minute
-
 // InitializeNPLAgent initializes the NodePortLocal agent.
 // It sets up event handlers to handle Pod add, update and delete events.
 // When a Pod gets created, a free Node port is obtained from the port table cache and a DNAT rule is added to NAT traffic to the Pod's ip:port.
@@ -49,20 +44,6 @@ func InitializeNPLAgent(
 		return nil, fmt.Errorf("error when initializing NodePortLocal port table: %v", err)
 	}
 
-	return InitController(kubeClient, informerFactory, portTable, nodeName, podInformer)
-}
-
-// InitController initializes the NPLController with appropriate Pod and Service Informers.
-// This function can be used independently while unit testing without using InitializeNPLAgent function.
-func InitController(kubeClient clientset.Interface, informerFactory informers.SharedInformerFactory, portTable *portcache.PortTable, nodeName string, podInformer cache.SharedIndexInformer) (*nplk8s.NPLController, error) {
 	svcInformer := informerFactory.Core().V1().Services().Informer()
-
-	c := nplk8s.NewNPLController(kubeClient,
-		podInformer,
-		svcInformer,
-		resyncPeriod,
-		portTable,
-		nodeName)
-
-	return c, nil
+	return nplk8s.NewNPLController(kubeClient, podInformer, svcInformer, portTable, nodeName), nil
 }
diff --git a/pkg/agent/secondarynetwork/podwatch/controller.go b/pkg/agent/secondarynetwork/podwatch/controller.go
index bbf4746b6..f92441876 100644
--- a/pkg/agent/secondarynetwork/podwatch/controller.go
+++ b/pkg/agent/secondarynetwork/podwatch/controller.go
@@ -26,11 +26,8 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/util/rand"
 	"k8s.io/apimachinery/pkg/util/wait"
-	"k8s.io/client-go/informers"
-	coreinformers "k8s.io/client-go/informers/core/v1"
 	clientset "k8s.io/client-go/kubernetes"
 	corelisters "k8s.io/client-go/listers/core/v1"
 	"k8s.io/client-go/tools/cache"
@@ -81,7 +78,6 @@ type PodController struct {
 func NewPodController(kubeClient clientset.Interface,
 	netAttachDefClient netdefclient.K8sCniCncfIoV1Interface,
 	podInformer cache.SharedIndexInformer,
-	resyncPeriod time.Duration,
 	nodeName string,
 	podCache cnipodcache.CNIPodInfoStore,
 	cniServer *cniserver.CNIServer) *PodController {
@@ -398,26 +394,3 @@ func parsePodSecondaryNetworkAnnotation(netObj string) ([]*SecondaryNetworkObjec
 	}
 	return secNetwork, nil
 }
-
-func InitializePodController(kubeClient clientset.Interface, netAttachDefClient netdefclient.K8sCniCncfIoV1Interface, informerFactory informers.SharedInformerFactory,
-	nodeName string, podInfoStore cnipodcache.CNIPodInfoStore, cniServer *cniserver.CNIServer) (*PodController, error) {
-	// Watch only the Pods which belong to the Node where the agent is running.
-	listOptions := func(options *metav1.ListOptions) {
-		options.FieldSelector = fields.OneTermEqualSelector("spec.nodeName", nodeName).String()
-	}
-	podInformer := coreinformers.NewFilteredPodInformer(
-		kubeClient,
-		metav1.NamespaceAll,
-		resyncPeriod,
-		cache.Indexers{},
-		listOptions,
-	)
-	c := NewPodController(kubeClient,
-		netAttachDefClient,
-		podInformer,
-		resyncPeriod,
-		nodeName,
-		podInfoStore,
-		cniServer)
-	return c, nil
-}


Test patch:
diff --git a/pkg/agent/nodeportlocal/npl_agent_test.go b/pkg/agent/nodeportlocal/npl_agent_test.go
index 8d71e807b..c577e7e99 100644
--- a/pkg/agent/nodeportlocal/npl_agent_test.go
+++ b/pkg/agent/nodeportlocal/npl_agent_test.go
@@ -238,6 +238,7 @@ func setUp(t *testing.T, tc *testConfig, objects ...runtime.Object) *testData {
 		portTable: newPortTable(mockIPTables, mockPortOpener),
 	}
 
+	resyncPeriod := 0 * time.Minute
 	// informerFactory is initialized and started from cmd/antrea-agent/agent.go
 	informerFactory := informers.NewSharedInformerFactory(data.k8sClient, resyncPeriod)
 	listOptions := func(options *metav1.ListOptions) {
@@ -250,9 +251,9 @@ func setUp(t *testing.T, tc *testConfig, objects ...runtime.Object) *testData {
 		cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}, // NamespaceIndex is used in NPLController.
 		listOptions,
 	)
+	svcInformer := informerFactory.Core().V1().Services().Informer()
 
-	c, err := InitController(data.k8sClient, informerFactory, data.portTable, defaultNodeName, localPodInformer)
-	require.NoError(t, err)
+	c := nplk8s.NewNPLController(data.k8sClient, localPodInformer, svcInformer, data.portTable, defaultNodeName)
 
 	data.runWrapper(c)
 	informerFactory.Start(data.stopCh)
diff --git a/test/e2e/infra/vagrant/playbook/roles/common/tasks/containerd.yml b/test/e2e/infra/vagrant/playbook/roles/common/tasks/containerd.yml
index 3c2bd51c7..a186583c5 100644
--- a/test/e2e/infra/vagrant/playbook/roles/common/tasks/containerd.yml
+++ b/test/e2e/infra/vagrant/playbook/roles/common/tasks/containerd.yml
@@ -61,3 +61,8 @@
     daemon_reload: yes
     state: restarted
   when: containerd_config.changed
+
+- name: Configure crictl
+  template:
+    src: templates/crictl.yaml.j2
+    dest: /etc/crictl.yaml
diff --git a/test/e2e/infra/vagrant/playbook/roles/common/templates/crictl.yaml.j2 b/test/e2e/infra/vagrant/playbook/roles/common/templates/crictl.yaml.j2
new file mode 100644
index 000000000..6612ed7e4
--- /dev/null
+++ b/test/e2e/infra/vagrant/playbook/roles/common/templates/crictl.yaml.j2
@@ -0,0 +1,2 @@
+runtime-endpoint: unix:///run/containerd/containerd.sock
+image-endpoint: unix:///run/containerd/containerd.sock