Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix antrea-agent crashing in networkPolicyOnly mode #795

Merged
merged 1 commit into from
Jun 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions pkg/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}
Expand Down
15 changes: 10 additions & 5 deletions pkg/agent/config/node_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/agent/querier/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
192 changes: 192 additions & 0 deletions pkg/agent/querier/querier_test.go
Original file line number Diff line number Diff line change
@@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tnqn is this test related to this PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, AgentQuerier.GetAgentInfo is the method panic. The test ensures it can get expected result without panic (networkPolicyOnly-mode non-partial case).

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)
})
}
}
2 changes: 1 addition & 1 deletion pkg/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func GetGitSHA() string {
// unreleased versions.
func GetFullVersion() string {
if Version == "" {
return "UKNOWN"
return "UNKNOWN"
}
if ReleaseStatus == "released" {
return Version
Expand Down