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 error log when agent fails to connect to K8s API #5353

Merged
merged 1 commit into from
Aug 4, 2023
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
15 changes: 9 additions & 6 deletions pkg/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ var (
// Declared as variables for testing.
defaultFs = afero.NewOsFs()
clock clockutils.WithTicker = &clockutils.RealClock{}

getNodeTimeout = 30 * time.Second
)

// Initializer knows how to setup host networking, OpenVSwitch, and Openflow.
Expand Down Expand Up @@ -905,7 +907,7 @@ func (i *Initializer) setTunnelCsum(tunnelPortName string, enable bool) error {
// host gateway interface.
func (i *Initializer) initK8sNodeLocalConfig(nodeName string) error {
var node *v1.Node
if err := wait.PollImmediate(5*time.Second, 30*time.Second, func() (bool, error) {
if err := wait.PollImmediate(5*time.Second, getNodeTimeout, func() (bool, error) {
var err error
node, err = i.client.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{})
if err != nil {
Expand All @@ -916,18 +918,19 @@ func (i *Initializer) initK8sNodeLocalConfig(nodeName string) error {
if !i.networkConfig.TrafficEncapMode.IsNetworkPolicyOnly() {
// Validate that PodCIDR has been configured.
if node.Spec.PodCIDRs == nil && node.Spec.PodCIDR == "" {
klog.V(2).Info("Waiting for Node PodCIDR configuration to complete")
klog.InfoS("Waiting for Node PodCIDR configuration to complete", "nodeName", nodeName)
return false, nil
}
}
return true, nil
}); err != nil {
if node != nil && node.Spec.PodCIDRs == nil && node.Spec.PodCIDR == "" {
if err == wait.ErrWaitTimeout {
klog.ErrorS(err, "Spec.PodCIDR is empty for Node. Please make sure --allocate-node-cidrs is enabled "+
"for kube-controller-manager and --cluster-cidr specifies a sufficient CIDR range", "nodeName", nodeName)
return fmt.Errorf("CIDR string is empty for Node %s", nodeName)
"for kube-controller-manager and --cluster-cidr specifies a sufficient CIDR range, or nodeIPAM is "+
"enabled for antrea-controller", "nodeName", nodeName)
return fmt.Errorf("Spec.PodCIDR is empty for Node %s", nodeName)
}
return fmt.Errorf("node retrieval failed with the following error: %v", err)
return err
}

// nodeInterface is the interface that has K8s Node IP. transportInterface is the interface that is used for
Expand Down
60 changes: 54 additions & 6 deletions pkg/agent/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@ import (
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8sruntime "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/kubernetes/fake"
k8stesting "k8s.io/client-go/testing"
clockutils "k8s.io/utils/clock"
clocktesting "k8s.io/utils/clock/testing"

Expand Down Expand Up @@ -210,12 +212,15 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
transportAddresses := strings.Join([]string{testTransportIface.ipV4Net.IP.String(), testTransportIface.ipV6Net.IP.String()}, ",")
tests := []struct {
name string
getNodeReaction k8stesting.ReactionFunc
trafficEncapMode config.TrafficEncapModeType
transportIfName string
transportIfCIDRs []string
transportInterface *testTransInterface
tunnelType ovsconfig.TunnelType
mtu int
podCIDR string
expectedErr string
expectedMTU int
expectedNodeLocalIfaceMTU int
expectedNodeAnnotation map[string]string
Expand All @@ -224,6 +229,7 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
name: "noencap mode",
trafficEncapMode: config.TrafficEncapModeNoEncap,
mtu: 0,
podCIDR: podCIDRStr,
expectedNodeLocalIfaceMTU: 1500,
expectedMTU: 1500,
expectedNodeAnnotation: map[string]string{types.NodeMACAddressAnnotationKey: macAddr.String()},
Expand All @@ -232,6 +238,7 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
name: "hybrid mode",
trafficEncapMode: config.TrafficEncapModeHybrid,
mtu: 0,
podCIDR: podCIDRStr,
expectedNodeLocalIfaceMTU: 1500,
expectedMTU: 1500,
expectedNodeAnnotation: map[string]string{types.NodeMACAddressAnnotationKey: macAddr.String()},
Expand All @@ -241,6 +248,7 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
trafficEncapMode: config.TrafficEncapModeEncap,
tunnelType: ovsconfig.GeneveTunnel,
mtu: 0,
podCIDR: podCIDRStr,
expectedNodeLocalIfaceMTU: 1500,
expectedMTU: 1450,
expectedNodeAnnotation: nil,
Expand All @@ -250,6 +258,7 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
trafficEncapMode: config.TrafficEncapModeEncap,
tunnelType: ovsconfig.GeneveTunnel,
mtu: 1400,
podCIDR: podCIDRStr,
expectedNodeLocalIfaceMTU: 1500,
expectedMTU: 1400,
expectedNodeAnnotation: nil,
Expand All @@ -260,6 +269,7 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
transportIfName: testTransportIface.iface.Name,
transportInterface: testTransportIface,
mtu: 0,
podCIDR: podCIDRStr,
expectedNodeLocalIfaceMTU: 1500,
expectedMTU: 1500,
expectedNodeAnnotation: map[string]string{
Expand All @@ -273,6 +283,7 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
transportIfName: testTransportIface.iface.Name,
transportInterface: testTransportIface,
mtu: 0,
podCIDR: podCIDRStr,
expectedNodeLocalIfaceMTU: 1500,
expectedMTU: 1500,
expectedNodeAnnotation: map[string]string{
Expand All @@ -287,6 +298,7 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
transportInterface: testTransportIface,
tunnelType: ovsconfig.GeneveTunnel,
mtu: 0,
podCIDR: podCIDRStr,
expectedNodeLocalIfaceMTU: 1500,
expectedMTU: 1450,
expectedNodeAnnotation: map[string]string{
Expand All @@ -300,6 +312,7 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
transportInterface: testTransportIface,
tunnelType: ovsconfig.GeneveTunnel,
mtu: 1400,
podCIDR: podCIDRStr,
expectedNodeLocalIfaceMTU: 1500,
expectedMTU: 1400,
expectedNodeAnnotation: map[string]string{
Expand All @@ -312,6 +325,7 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
transportIfCIDRs: transportCIDRs,
transportInterface: testTransportIface,
mtu: 0,
podCIDR: podCIDRStr,
expectedNodeLocalIfaceMTU: 1500,
expectedMTU: 1500,
expectedNodeAnnotation: map[string]string{
Expand All @@ -325,6 +339,7 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
transportIfCIDRs: transportCIDRs,
transportInterface: testTransportIface,
mtu: 0,
podCIDR: podCIDRStr,
expectedNodeLocalIfaceMTU: 1500,
expectedMTU: 1500,
expectedNodeAnnotation: map[string]string{
Expand All @@ -339,6 +354,7 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
transportInterface: testTransportIface,
tunnelType: ovsconfig.GeneveTunnel,
mtu: 0,
podCIDR: podCIDRStr,
expectedNodeLocalIfaceMTU: 1500,
expectedMTU: 1450,
expectedNodeAnnotation: map[string]string{
Expand All @@ -352,12 +368,28 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
transportInterface: testTransportIface,
tunnelType: ovsconfig.GeneveTunnel,
mtu: 1400,
podCIDR: podCIDRStr,
expectedNodeLocalIfaceMTU: 1500,
expectedMTU: 1400,
expectedNodeAnnotation: map[string]string{
types.NodeTransportAddressAnnotationKey: transportAddresses,
},
},
{
name: "error getting Node",
getNodeReaction: func(action k8stesting.Action) (handled bool, ret k8sruntime.Object, err error) {
return true, nil, fmt.Errorf("connection error")
},
trafficEncapMode: config.TrafficEncapModeEncap,
tunnelType: ovsconfig.GeneveTunnel,
expectedErr: "failed to get Node with name node1 from K8s: connection error",
},
{
name: "empty node podCIDR",
trafficEncapMode: config.TrafficEncapModeEncap,
tunnelType: ovsconfig.GeneveTunnel,
expectedErr: "Spec.PodCIDR is empty for Node node1",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -366,7 +398,7 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
Name: nodeName,
},
Spec: corev1.NodeSpec{
PodCIDR: podCIDRStr,
PodCIDR: tt.podCIDR,
},
Status: corev1.NodeStatus{
Addresses: []corev1.NodeAddress{
Expand All @@ -378,6 +410,10 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
},
}
client := fake.NewSimpleClientset(node)
if tt.getNodeReaction != nil {
client.PrependReactor("get", "nodes", tt.getNodeReaction)
}

ifaceStore := interfacestore.NewInterfaceStore()
expectedNodeConfig := config.NodeConfig{
Name: nodeName,
Expand Down Expand Up @@ -417,12 +453,18 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
}
defer mockGetIPNetDeviceFromIP(nodeIPNet, ipDevice)()
defer mockNodeNameEnv(nodeName)()
defer mockGetNodeTimeout(100 * time.Millisecond)

require.NoError(t, initializer.initK8sNodeLocalConfig(nodeName))
assert.Equal(t, expectedNodeConfig, *initializer.nodeConfig)
node, err := client.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, tt.expectedNodeAnnotation, node.Annotations)
err := initializer.initK8sNodeLocalConfig(nodeName)
if tt.expectedErr == "" {
require.NoError(t, err)
assert.Equal(t, expectedNodeConfig, *initializer.nodeConfig)
node, err := client.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, tt.expectedNodeAnnotation, node.Annotations)
} else {
assert.ErrorContains(t, err, tt.expectedErr)
}
})
}
}
Expand All @@ -440,6 +482,12 @@ func mockNodeNameEnv(name string) func() {
return func() { os.Unsetenv(env.NodeNameEnvKey) }
}

func mockGetNodeTimeout(timeout time.Duration) func() {
prevTimeout := getNodeTimeout
getNodeTimeout = timeout
return func() { getNodeTimeout = prevTimeout }
}

func mockGetTransportIPNetDeviceByName(ipV4Net, ipV6Net *net.IPNet, ipDevice *net.Interface) func() {
prevGetIPNetDeviceByName := getTransportIPNetDeviceByName
getTransportIPNetDeviceByName = func(ifName, brName string) (*net.IPNet, *net.IPNet, *net.Interface, error) {
Expand Down