Merge pull request #610 from aojea/ipam_csr_preempt

gcp-controller: don't make assumptions on PodCIDRs

Author: k8s-ci-robot

cmd/gcp-controller-manager/node_csr_approver.go

@@ -963,27 +963,6 @@ func shouldDeleteNode(ctx *controllerContext, node *v1.Node, getInstance func(*c
 			return true, nil
 		}
 	}
-	// Newly created node might not have pod CIDR allocated yet.
-	if node.Spec.PodCIDR == "" {
-		klog.V(2).Infof("Node %q has empty podCIDR.", node.Name)
-		return false, nil
-	}
-	var unmatchedRanges []string
-	for _, networkInterface := range inst.NetworkInterfaces {
-		for _, r := range networkInterface.AliasIpRanges {
-			if node.Spec.PodCIDR == r.IpCidrRange {
-				klog.V(2).Infof("Instance %q has alias range that matches node's podCIDR.", inst.Name)
-				return false, nil
-			}
-			unmatchedRanges = append(unmatchedRanges, r.IpCidrRange)
-		}
-	}
-	if len(unmatchedRanges) != 0 {
-		klog.Warningf("Instance %q has alias range(s) %v and none of them match node's podCIDR %s, will trigger node deletion.", inst.Name, unmatchedRanges, node.Spec.PodCIDR)
-		return true, nil
-	}
-	// Instance with no alias range is route based, for which node object deletion is unnecessary.
-	klog.V(2).Infof("Instance %q has no alias range.", inst.Name)
 	return false, nil
 }
 

cmd/gcp-controller-manager/node_csr_approver_test.go

@@ -1252,53 +1252,6 @@ func TestShouldDeleteNode(t *testing.T) {
 		getInstanceErr error
 		expectedErr    error
 	}{
-		{
-			desc: "instance with 1 alias range and matches podCIDR",
-			ctx:  &controllerContext{},
-			node: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node-test",
-				},
-				Spec: v1.NodeSpec{
-					PodCIDR: "10.0.0.1/24",
-				},
-			},
-			instance: &compute.Instance{
-				NetworkInterfaces: []*compute.NetworkInterface{
-					{
-						AliasIpRanges: []*compute.AliasIpRange{
-							{
-								IpCidrRange: "10.0.0.1/24",
-							},
-						},
-					},
-				},
-			},
-		},
-		{
-			desc: "instance with 1 alias range doesn't match podCIDR",
-			ctx:  &controllerContext{},
-			node: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node-test",
-				},
-				Spec: v1.NodeSpec{
-					PodCIDR: "10.0.0.1/24",
-				},
-			},
-			instance: &compute.Instance{
-				NetworkInterfaces: []*compute.NetworkInterface{
-					{
-						AliasIpRanges: []*compute.AliasIpRange{
-							{
-								IpCidrRange: "10.0.0.2/24",
-							},
-						},
-					},
-				},
-			},
-			shouldDelete: true,
-		},
 		{
 			desc: "instance with 2 alias range and 1 matches podCIDR",
 			ctx:  &controllerContext{},
@@ -1399,13 +1352,15 @@ func TestShouldDeleteNode(t *testing.T) {
 		},
 	}
 	for _, c := range cases {
-		fakeGetInstance := func(_ *controllerContext, _ string) (*compute.Instance, error) {
-			return c.instance, c.getInstanceErr
-		}
-		shouldDelete, err := shouldDeleteNode(c.ctx, c.node, fakeGetInstance)
-		if err != c.expectedErr || shouldDelete != c.shouldDelete {
-			t.Errorf("%s: shouldDeleteNode=(%v, %v), want (%v, %v)", c.desc, shouldDelete, err, c.shouldDelete, c.expectedErr)
-		}
+		t.Run(c.desc, func(t *testing.T) {
+			fakeGetInstance := func(_ *controllerContext, _ string) (*compute.Instance, error) {
+				return c.instance, c.getInstanceErr
+			}
+			shouldDelete, err := shouldDeleteNode(c.ctx, c.node, fakeGetInstance)
+			if err != c.expectedErr || shouldDelete != c.shouldDelete {
+				t.Errorf("%s: shouldDeleteNode=(%v, %v), want (%v, %v)", c.desc, shouldDelete, err, c.shouldDelete, c.expectedErr)
+			}
+		})
 	}
 }