Skip to content

Commit

Permalink
Merge pull request #58186 from negz/master
Browse files Browse the repository at this point in the history
Automatic merge from submit-queue (batch tested with PRs 57266, 58187, 58186, 46245, 56509). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Avoid panic in Cloud CIDR Allocator

**What this PR does / why we need it**:
I suspect a race exists where we attempt to look up the CIDR for a terminating node. By the time `updateCIDRAllocation` is called the node has disappeared. We determine it does not have a cloud CIDR (i.e. Alias IP Range) and attempt to record a `CIDRNotAvailable` node status. Unfortunately we reference `node.Name` while `node` is still nil.

By getting the node before looking up the cloud CIDR we avoid the nil pointer dereference, and potentially fail fast in the case the node has disappeared.

**Which issue(s) this PR fixes**:
Fixes #58181

**Release note**:

```release-note
Avoid panic when failing to allocate a Cloud CIDR (aka GCE Alias IP Range). 
```
  • Loading branch information
Kubernetes Submit Queue authored Jan 13, 2018
2 parents 99abe92 + c7988ba commit 15ef3a8
Showing 1 changed file with 6 additions and 8 deletions.
14 changes: 6 additions & 8 deletions pkg/controller/nodeipam/ipam/cloud_cidr_allocator.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,14 @@ func (ca *cloudCIDRAllocator) AllocateOrOccupyCIDR(node *v1.Node) error {

// updateCIDRAllocation assigns CIDR to Node and sends an update to the API server.
func (ca *cloudCIDRAllocator) updateCIDRAllocation(nodeName string) error {
var err error
var node *v1.Node
defer ca.removeNodeFromProcessing(nodeName)

node, err := ca.nodeLister.Get(nodeName)
if err != nil {
glog.Errorf("Failed while getting node %v for updating Node.Spec.PodCIDR: %v", nodeName, err)
return err
}

cidrs, err := ca.cloud.AliasRanges(types.NodeName(nodeName))
if err != nil {
nodeutil.RecordNodeStatusChange(ca.recorder, node, "CIDRNotAvailable")
Expand All @@ -210,12 +214,6 @@ func (ca *cloudCIDRAllocator) updateCIDRAllocation(nodeName string) error {
}
podCIDR := cidr.String()

node, err = ca.nodeLister.Get(nodeName)
if err != nil {
glog.Errorf("Failed while getting node %v for updating Node.Spec.PodCIDR: %v", nodeName, err)
return err
}

if node.Spec.PodCIDR == podCIDR {
glog.V(4).Infof("Node %v already has allocated CIDR %v. It matches the proposed one.", node.Name, podCIDR)
// We don't return here, in order to set the NetworkUnavailable condition later below.
Expand Down

0 comments on commit 15ef3a8

Please sign in to comment.