Skip to content

Commit e992008

Browse files
author
Ravi Sankar Penta
committed
Fix clearInitialNodeNetworkUnavailableCondition() in sdn master
This change fixes these 2 issues: - Currently, clearing NodeNetworkUnavailable node condition only works if we are successful in updating the node status during the first iteration. Subsequent retries will not work because: 1. knode != node 2. node.Status is updated in memory 3. knode.UpdateNodeStatus() (3) will have no effect as in step (2) node.Status is updated but not knode.Status - Node object passed to this method is pointer to an item in the informer cache and it should not be modified directly.
1 parent 1e29a56 commit e992008

File tree

1 file changed

+17
-28
lines changed

1 file changed

+17
-28
lines changed

pkg/network/master/subnets.go

+17-28
Original file line numberDiff line numberDiff line change
@@ -191,52 +191,41 @@ func (master *OsdnMaster) deleteNode(nodeName string) error {
191191
// condition doesn't get added for network plugins that don't want it, and then
192192
// we can remove this function.
193193
func (master *OsdnMaster) clearInitialNodeNetworkUnavailableCondition(node *kapi.Node) {
194-
knode := node
194+
// Informer cache should not be mutated, so get a copy of the object
195+
knode := node.DeepCopy()
195196
cleared := false
196197
resultErr := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
197198
var err error
198199

199200
if knode != node {
200-
knode, err = master.kClient.Core().Nodes().Get(node.ObjectMeta.Name, metav1.GetOptions{})
201+
knode, err = master.kClient.Core().Nodes().Get(node.Name, metav1.GetOptions{})
201202
if err != nil {
202203
return err
203204
}
204205
}
205206

206-
// Let caller modify knode's status, then push to api server.
207-
_, condition := GetNodeCondition(&node.Status, kapi.NodeNetworkUnavailable)
208-
if condition != nil && condition.Status != kapi.ConditionFalse && condition.Reason == "NoRouteCreated" {
209-
condition.Status = kapi.ConditionFalse
210-
condition.Reason = "RouteCreated"
211-
condition.Message = "openshift-sdn cleared kubelet-set NoRouteCreated"
212-
condition.LastTransitionTime = metav1.Now()
213-
knode, err = master.kClient.Core().Nodes().UpdateStatus(knode)
214-
if err == nil {
215-
cleared = true
207+
for i := range knode.Status.Conditions {
208+
if knode.Status.Conditions[i].Type == kapi.NodeNetworkUnavailable {
209+
condition := &knode.Status.Conditions[i]
210+
if condition.Status != kapi.ConditionFalse && condition.Reason == "NoRouteCreated" {
211+
condition.Status = kapi.ConditionFalse
212+
condition.Reason = "RouteCreated"
213+
condition.Message = "openshift-sdn cleared kubelet-set NoRouteCreated"
214+
condition.LastTransitionTime = metav1.Now()
215+
216+
if knode, err = master.kClient.Core().Nodes().UpdateStatus(knode); err == nil {
217+
cleared = true
218+
}
219+
}
216220
}
217221
}
218222
return err
219223
})
220224
if resultErr != nil {
221225
utilruntime.HandleError(fmt.Errorf("status update failed for local node: %v", resultErr))
222226
} else if cleared {
223-
glog.Infof("Cleared node NetworkUnavailable/NoRouteCreated condition for %s", node.ObjectMeta.Name)
224-
}
225-
}
226-
227-
// TODO remove this and switch to external
228-
// GetNodeCondition extracts the provided condition from the given status and returns that.
229-
// Returns nil and -1 if the condition is not present, and the index of the located condition.
230-
func GetNodeCondition(status *kapi.NodeStatus, conditionType kapi.NodeConditionType) (int, *kapi.NodeCondition) {
231-
if status == nil {
232-
return -1, nil
233-
}
234-
for i := range status.Conditions {
235-
if status.Conditions[i].Type == conditionType {
236-
return i, &status.Conditions[i]
237-
}
227+
glog.Infof("Cleared node NetworkUnavailable/NoRouteCreated condition for %s", node.Name)
238228
}
239-
return -1, nil
240229
}
241230

242231
func (master *OsdnMaster) watchNodes() {

0 commit comments

Comments
 (0)