From 36d2855abf33edfd23f6e153a70a7db66189175a Mon Sep 17 00:00:00 2001 From: Quan Tian Date: Thu, 4 Jun 2020 20:58:36 +0800 Subject: [PATCH] Fix antrea-agent crashing in networkPolicyOnly mode The PodCIDR of NodeConfig could be nil for the networkPolicyOnly trafficEncapMode. The AgentQuerier should check nil before converting it to string. Besides, this patch removes the redundant field BridgeName and fixes a typo. --- pkg/agent/agent.go | 7 +- pkg/agent/config/node_config.go | 15 ++- pkg/agent/querier/querier.go | 4 +- pkg/agent/querier/querier_test.go | 192 ++++++++++++++++++++++++++++++ pkg/version/version.go | 2 +- 5 files changed, 211 insertions(+), 9 deletions(-) create mode 100644 pkg/agent/querier/querier_test.go diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index f9465474ea7..8c010df52d4 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -503,7 +503,11 @@ func (i *Initializer) initNodeLocalConfig() error { } if i.networkConfig.TrafficEncapMode.IsNetworkPolicyOnly() { - i.nodeConfig = &config.NodeConfig{Name: nodeName, NodeIPAddr: localAddr, BridgeName: i.ovsBridgeClient.GetBridgeName(), UplinkNetConfig: new(config.AdapterNetConfig)} + i.nodeConfig = &config.NodeConfig{ + Name: nodeName, + OVSBridge: i.ovsBridge, + NodeIPAddr: localAddr, + UplinkNetConfig: new(config.AdapterNetConfig)} return nil } @@ -524,7 +528,6 @@ func (i *Initializer) initNodeLocalConfig() error { OVSBridge: i.ovsBridge, PodCIDR: localSubnet, NodeIPAddr: localAddr, - BridgeName: i.ovsBridgeClient.GetBridgeName(), UplinkNetConfig: new(config.AdapterNetConfig)} return nil } diff --git a/pkg/agent/config/node_config.go b/pkg/agent/config/node_config.go index 8cea312912e..59a49c5f79b 100644 --- a/pkg/agent/config/node_config.go +++ b/pkg/agent/config/node_config.go @@ -58,12 +58,17 @@ type AdapterNetConfig struct { // Local Node configurations retrieved from K8s API or host networking state. type NodeConfig struct { - Name string - OVSBridge string - PodCIDR *net.IPNet - NodeIPAddr *net.IPNet + // The Node's name used in Kubernetes. + Name string + // The name of the OpenVSwitch bridge antrea-agent uses. + OVSBridge string + // The CIDR block to allocate Pod IPs out of. + // It's nil for the networkPolicyOnly trafficEncapMode which doesn't do IPAM. + PodCIDR *net.IPNet + // The Node's IP used in Kubernetes. It has the network mask information. + NodeIPAddr *net.IPNet + // The config of the gateway network device, i.e. gw0 by default. GatewayConfig *GatewayConfig - BridgeName string UplinkNetConfig *AdapterNetConfig } diff --git a/pkg/agent/querier/querier.go b/pkg/agent/querier/querier.go index 685a60013fa..34a24795f5a 100644 --- a/pkg/agent/querier/querier.go +++ b/pkg/agent/querier/querier.go @@ -193,7 +193,9 @@ func (aq agentQuerier) GetAgentInfo(agentInfo *v1beta1.AntreaAgentInfo, partial agentInfo.Version = querier.GetVersion() agentInfo.PodRef = querier.GetSelfPod() agentInfo.NodeRef = querier.GetSelfNode(true, aq.nodeConfig.Name) - agentInfo.NodeSubnet = []string{aq.nodeConfig.PodCIDR.String()} + if aq.nodeConfig.PodCIDR != nil { + agentInfo.NodeSubnet = []string{aq.nodeConfig.PodCIDR.String()} + } agentInfo.OVSInfo.BridgeName = aq.nodeConfig.OVSBridge agentInfo.APIPort = aq.apiPort } diff --git a/pkg/agent/querier/querier_test.go b/pkg/agent/querier/querier_test.go new file mode 100644 index 00000000000..6f6d57f9610 --- /dev/null +++ b/pkg/agent/querier/querier_test.go @@ -0,0 +1,192 @@ +// Copyright 2020 Antrea Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package querier + +import ( + "net" + "testing" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/vmware-tanzu/antrea/pkg/agent/config" + interfacestoretest "github.com/vmware-tanzu/antrea/pkg/agent/interfacestore/testing" + openflowtest "github.com/vmware-tanzu/antrea/pkg/agent/openflow/testing" + "github.com/vmware-tanzu/antrea/pkg/apis/clusterinformation/v1beta1" + binding "github.com/vmware-tanzu/antrea/pkg/ovs/openflow" + ovsconfigtest "github.com/vmware-tanzu/antrea/pkg/ovs/ovsconfig/testing" + queriertest "github.com/vmware-tanzu/antrea/pkg/querier/testing" +) + +const ovsVersion = "2.10.0" + +func getIPNet(ip string) *net.IPNet { + _, ipNet, _ := net.ParseCIDR(ip) + return ipNet +} + +func TestAgentQuerierGetAgentInfo(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + interfaceStore := interfacestoretest.NewMockInterfaceStore(ctrl) + interfaceStore.EXPECT().GetContainerInterfaceNum().Return(2).AnyTimes() + + ofClient := openflowtest.NewMockClient(ctrl) + ofClient.EXPECT().GetFlowTableStatus().Return([]binding.TableStatus{ + { + ID: 1, + FlowCount: 2, + }, + }).AnyTimes() + ofClient.EXPECT().IsConnected().Return(true).AnyTimes() + + ovsBridgeClient := ovsconfigtest.NewMockOVSBridgeClient(ctrl) + ovsBridgeClient.EXPECT().GetOVSVersion().Return(ovsVersion, nil).AnyTimes() + + networkPolicyInfoQuerier := queriertest.NewMockAgentNetworkPolicyInfoQuerier(ctrl) + networkPolicyInfoQuerier.EXPECT().GetNetworkPolicyNum().Return(10).AnyTimes() + networkPolicyInfoQuerier.EXPECT().GetAppliedToGroupNum().Return(20).AnyTimes() + networkPolicyInfoQuerier.EXPECT().GetAddressGroupNum().Return(30).AnyTimes() + networkPolicyInfoQuerier.EXPECT().GetControllerConnectionStatus().Return(true).AnyTimes() + + tests := []struct { + name string + nodeConfig *config.NodeConfig + apiPort int + partial bool + expectedAgentInfo *v1beta1.AntreaAgentInfo + }{ + { + name: "networkPolicyOnly-mode non-partial", + nodeConfig: &config.NodeConfig{ + Name: "foo", + OVSBridge: "br-int", + NodeIPAddr: getIPNet("10.10.0.10"), + }, + apiPort: 10350, + partial: false, + expectedAgentInfo: &v1beta1.AntreaAgentInfo{ + ObjectMeta: v1.ObjectMeta{Name: "foo"}, + NodeRef: corev1.ObjectReference{Kind: "Node", Name: "foo"}, + NodeSubnet: nil, + OVSInfo: v1beta1.OVSInfo{ + Version: ovsVersion, + BridgeName: "br-int", + FlowTable: map[string]int32{"1": 2}, + }, + NetworkPolicyControllerInfo: v1beta1.NetworkPolicyControllerInfo{ + NetworkPolicyNum: 10, + AppliedToGroupNum: 20, + AddressGroupNum: 30, + }, + LocalPodNum: 2, + AgentConditions: []v1beta1.AgentCondition{ + { + Type: v1beta1.AgentHealthy, + Status: corev1.ConditionTrue, + }, + { + Type: v1beta1.ControllerConnectionUp, + Status: corev1.ConditionTrue, + }, + { + Type: v1beta1.OVSDBConnectionUp, + Status: corev1.ConditionTrue, + }, + { + Type: v1beta1.OpenflowConnectionUp, + Status: corev1.ConditionTrue, + }, + }, + APIPort: 10350, + Version: "UNKNOWN", + }, + }, + { + name: "encap-mode non-partial", + nodeConfig: &config.NodeConfig{ + Name: "foo", + OVSBridge: "br-int", + NodeIPAddr: getIPNet("10.10.0.10"), + PodCIDR: getIPNet("20.20.20.0/24"), + }, + apiPort: 10350, + partial: false, + expectedAgentInfo: &v1beta1.AntreaAgentInfo{ + ObjectMeta: v1.ObjectMeta{Name: "foo"}, + NodeRef: corev1.ObjectReference{Kind: "Node", Name: "foo"}, + NodeSubnet: []string{"20.20.20.0/24"}, + OVSInfo: v1beta1.OVSInfo{ + Version: ovsVersion, + BridgeName: "br-int", + FlowTable: map[string]int32{"1": 2}, + }, + NetworkPolicyControllerInfo: v1beta1.NetworkPolicyControllerInfo{ + NetworkPolicyNum: 10, + AppliedToGroupNum: 20, + AddressGroupNum: 30, + }, + LocalPodNum: 2, + AgentConditions: []v1beta1.AgentCondition{ + { + Type: v1beta1.AgentHealthy, + Status: corev1.ConditionTrue, + }, + { + Type: v1beta1.ControllerConnectionUp, + Status: corev1.ConditionTrue, + }, + { + Type: v1beta1.OVSDBConnectionUp, + Status: corev1.ConditionTrue, + }, + { + Type: v1beta1.OpenflowConnectionUp, + Status: corev1.ConditionTrue, + }, + }, + APIPort: 10350, + Version: "UNKNOWN", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + aq := agentQuerier{ + nodeConfig: tt.nodeConfig, + interfaceStore: interfaceStore, + ofClient: ofClient, + ovsBridgeClient: ovsBridgeClient, + networkPolicyInfoQuerier: networkPolicyInfoQuerier, + apiPort: tt.apiPort, + } + agentInfo := &v1beta1.AntreaAgentInfo{} + aq.GetAgentInfo(agentInfo, tt.partial) + // Check AgentConditions separately as it contains timestamp we cannot predict. + assert.Equal(t, len(tt.expectedAgentInfo.AgentConditions), len(agentInfo.AgentConditions)) + for i := range agentInfo.AgentConditions { + assert.Equal(t, tt.expectedAgentInfo.AgentConditions[i].Status, agentInfo.AgentConditions[i].Status) + assert.Equal(t, tt.expectedAgentInfo.AgentConditions[i].Type, agentInfo.AgentConditions[i].Type) + } + // Exclude AgentConditions before comparing the whole objects. + tt.expectedAgentInfo.AgentConditions = nil + agentInfo.AgentConditions = nil + assert.Equal(t, tt.expectedAgentInfo, agentInfo) + }) + } +} diff --git a/pkg/version/version.go b/pkg/version/version.go index db4efcadea8..c6dbe1200ea 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -49,7 +49,7 @@ func GetGitSHA() string { // unreleased versions. func GetFullVersion() string { if Version == "" { - return "UKNOWN" + return "UNKNOWN" } if ReleaseStatus == "released" { return Version