github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2026-07-17 20:00:07 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #136561 from DockToFuture/patch/NetworkUnavailable

Browse Source 
Fix route controller condition update
This commit is contained in:
Kubernetes Prow Robot
2026-02-03 22:32:32 +05:30
committed by GitHub
parent 11e3294feb 57315c1974
commit d50446099d
2 changed files with 240 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
staging/src/k8s.io/cloud-provider/controllers/route/route_controller.go
View File

@@ -497,12 +497,12 @@ func (rc *RouteController) reconcile(ctx context.Context, nodes []*v1.Node, rout
func (rc *RouteController) updateNetworkingCondition(node *v1.Node, routesCreated bool) error {
_, condition := nodeutil.GetNodeCondition(&(node.Status), v1.NodeNetworkUnavailable)
if routesCreated && condition != nil && condition.Status == v1.ConditionFalse {
if routesCreated && condition != nil && condition.Status == v1.ConditionFalse && condition.Reason == "RouteCreated" {
klog.V(2).Infof("set node %v with NodeNetworkUnavailable=false was canceled because it is already set", node.Name)
return nil
}
if !routesCreated && condition != nil && condition.Status == v1.ConditionTrue {
if !routesCreated && condition != nil && condition.Status == v1.ConditionTrue && condition.Reason == "NoRouteCreated" {
klog.V(2).Infof("set node %v with NodeNetworkUnavailable=true was canceled because it is already set", node.Name)
return nil
}

238
staging/src/k8s.io/cloud-provider/controllers/route/route_controller_test.go
View File

@@ -636,3 +636,241 @@ func flatten(list []*cloudprovider.Route) []cloudprovider.Route {
}
return structList
}
func TestUpdateNetworkingCondition(t *testing.T) {
cluster := "test-cluster"
_, clusterCIDR, _ := netutils.ParseCIDRSloppy("10.244.0.0/16")
testCases := []struct {
description string
existingCondition *v1.NodeCondition
routesCreated bool
expectedUpdate bool
expectedConditionType v1.ConditionStatus
expectedReason string
expectedMessage string
}{
{
description: "No existing condition, routes created - should update",
existingCondition: nil,
routesCreated: true,
expectedUpdate: true,
expectedConditionType: v1.ConditionFalse,
expectedReason: "RouteCreated",
expectedMessage: "RouteController created a route",
},
{
description: "No existing condition, routes not created - should update",
existingCondition: nil,
routesCreated: false,
expectedUpdate: true,
expectedConditionType: v1.ConditionTrue,
expectedReason: "NoRouteCreated",
expectedMessage: "RouteController failed to create a route",
},
{
description: "Existing condition with same status and reason (RouteCreated) - should not update",
existingCondition: &v1.NodeCondition{
Type: v1.NodeNetworkUnavailable,
Status: v1.ConditionFalse,
Reason: "RouteCreated",
Message: "RouteController created a route",
},
routesCreated: true,
expectedUpdate: false,
},
{
description: "Existing condition with same status but different reason (CalicoIsUp) - should update",
existingCondition: &v1.NodeCondition{
Type: v1.NodeNetworkUnavailable,
Status: v1.ConditionFalse,
Reason: "CalicoIsUp",
Message: "Calico is running on this node",
},
routesCreated: true,
expectedUpdate: true,
expectedConditionType: v1.ConditionFalse,
expectedReason: "RouteCreated",
expectedMessage: "RouteController created a route",
},
{
description: "Existing condition with Status=False but different reason - should update when routes created",
existingCondition: &v1.NodeCondition{
Type: v1.NodeNetworkUnavailable,
Status: v1.ConditionFalse,
Reason: "ExternalCNI",
Message: "External CNI configured",
},
routesCreated: true,
expectedUpdate: true,
expectedConditionType: v1.ConditionFalse,
expectedReason: "RouteCreated",
expectedMessage: "RouteController created a route",
},
{
description: "Existing condition with Status=True and different reason - should update when routes not created",
existingCondition: &v1.NodeCondition{
Type: v1.NodeNetworkUnavailable,
Status: v1.ConditionTrue,
Reason: "SomeOtherReason",
Message: "Some other message",
},
routesCreated: false,
expectedUpdate: true,
expectedConditionType: v1.ConditionTrue,
expectedReason: "NoRouteCreated",
expectedMessage: "RouteController failed to create a route",
},
{
description: "Existing condition with Status=False, transitioning to routes not created - should update",
existingCondition: &v1.NodeCondition{
Type: v1.NodeNetworkUnavailable,
Status: v1.ConditionFalse,
Reason: "RouteCreated",
Message: "RouteController created a route",
},
routesCreated: false,
expectedUpdate: true,
expectedConditionType: v1.ConditionTrue,
expectedReason: "NoRouteCreated",
expectedMessage: "RouteController failed to create a route",
},
{
description: "Existing condition with Status=True and NoRouteCreated reason - should not update",
existingCondition: &v1.NodeCondition{
Type: v1.NodeNetworkUnavailable,
Status: v1.ConditionTrue,
Reason: "NoRouteCreated",
Message: "RouteController failed to create a route",
},
routesCreated: false,
expectedUpdate: false,
},
}
for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) {
node := &v1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "test-node",
UID: "test-uid",
},
Spec: v1.NodeSpec{
PodCIDR: "10.244.0.0/24",
PodCIDRs: []string{"10.244.0.0/24"},
},
}
// Set existing condition if specified
if tc.existingCondition != nil {
node.Status.Conditions = []v1.NodeCondition{*tc.existingCondition}
}
client := fake.NewClientset(node)
informerFactory := informers.NewSharedInformerFactory(client, 0)
rc, err := New(nil, client, informerFactory.Core().V1().Nodes(), cluster, []*net.IPNet{clusterCIDR})
require.NoError(t, err)
rc.nodeListerSynced = alwaysReady
// Call updateNetworkingCondition
err = rc.updateNetworkingCondition(node, tc.routesCreated)
require.NoError(t, err)
if tc.expectedUpdate {
// Verify that a patch was called
actions := client.Actions()
patchFound := false
for _, action := range actions {
if action.GetVerb() == "patch" {
patchFound = true
break
}
}
assert.True(t, patchFound, "Expected a patch action but none found")
// Get the updated node
updatedNode, err := client.CoreV1().Nodes().Get(context.TODO(), node.Name, metav1.GetOptions{})
require.NoError(t, err)
// Verify the condition was updated correctly
_, condition := nodeutil.GetNodeCondition(&updatedNode.Status, v1.NodeNetworkUnavailable)
require.NotNil(t, condition, "Expected NodeNetworkUnavailable condition to exist")
assert.Equal(t, tc.expectedConditionType, condition.Status, "Unexpected condition status")
assert.Equal(t, tc.expectedReason, condition.Reason, "Unexpected condition reason")
assert.Equal(t, tc.expectedMessage, condition.Message, "Unexpected condition message")
} else {
// Verify that no patch was called (or only initial operations)
actions := client.Actions()
patchCount := 0
for _, action := range actions {
if action.GetVerb() == "patch" {
patchCount++
}
}
assert.Equal(t, 0, patchCount, "Expected no patch action but found %d", patchCount)
}
})
}
}
func TestUpdateNetworkingConditionWithCalicoScenario(t *testing.T) {
// This test specifically covers the bug where Calico sets NetworkUnavailable=False
// and RouteController should still be able to update it with its own reason
cluster := "test-cluster"
_, clusterCIDR, _ := netutils.ParseCIDRSloppy("10.244.0.0/16")
node := &v1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "test-node",
UID: "test-uid",
},
Spec: v1.NodeSpec{
PodCIDR: "10.244.0.0/24",
PodCIDRs: []string{"10.244.0.0/24"},
},
Status: v1.NodeStatus{
Conditions: []v1.NodeCondition{
{
Type: v1.NodeNetworkUnavailable,
Status: v1.ConditionFalse,
Reason: "CalicoIsUp",
Message: "Calico is running on this node",
},
},
},
}
client := fake.NewClientset(node)
informerFactory := informers.NewSharedInformerFactory(client, 0)
rc, err := New(nil, client, informerFactory.Core().V1().Nodes(), cluster, []*net.IPNet{clusterCIDR})
require.NoError(t, err)
rc.nodeListerSynced = alwaysReady
// Call updateNetworkingCondition with routes created
err = rc.updateNetworkingCondition(node, true)
require.NoError(t, err)
// Verify that a patch was called
actions := client.Actions()
patchFound := false
for _, action := range actions {
if action.GetVerb() == "patch" {
patchFound = true
break
}
}
assert.True(t, patchFound, "Expected a patch action to update the condition")
// Get the updated node
updatedNode, err := client.CoreV1().Nodes().Get(context.TODO(), node.Name, metav1.GetOptions{})
require.NoError(t, err)
// Verify the condition was updated with RouteCreated reason
_, condition := nodeutil.GetNodeCondition(&updatedNode.Status, v1.NodeNetworkUnavailable)
require.NotNil(t, condition, "Expected NodeNetworkUnavailable condition to exist")
assert.Equal(t, v1.ConditionFalse, condition.Status, "Expected Status to remain False")
assert.Equal(t, "RouteCreated", condition.Reason, "Expected Reason to be updated to RouteCreated")
assert.Equal(t, "RouteController created a route", condition.Message, "Expected Message to be updated")
}