Skip to content

Commit 6991069

Browse files
cheftakocheftako
committed
Push context up to cloud node controller.
This adds context to the cloud node controller. It continues the propogation started in 59287. Fixes 815. Fixed test code calls.
1 parent 019b662 commit 6991069

File tree

3 files changed

+46
-45
lines changed

3 files changed

+46
-45
lines changed

‎pkg/controller/cloud/node_controller.go‎

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ func NewCloudNodeController(
8787
// Use shared informer to listen to add/update of nodes. Note that any nodes
8888
// that exist before node controller starts will show up in the update method
8989
cnc.nodeInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
90-
AddFunc: cnc.AddCloudNode,
91-
UpdateFunc: cnc.UpdateCloudNode,
90+
AddFunc: func(obj interface{}) { cnc.AddCloudNode(context.TODO(), obj) },
91+
UpdateFunc: func(oldObj, newObj interface{}) { cnc.UpdateCloudNode(context.TODO(), oldObj, newObj) },
9292
})
9393

9494
return cnc, nil
@@ -105,11 +105,11 @@ func (cnc *CloudNodeController) Run(stopCh <-chan struct{}) {
105105
// very infrequently. DO NOT MODIFY this to perform frequent operations.
106106

107107
// Start a loop to periodically update the node addresses obtained from the cloud
108-
wait.Until(cnc.UpdateNodeStatus, cnc.nodeStatusUpdateFrequency, stopCh)
108+
wait.Until(func() { cnc.UpdateNodeStatus(context.TODO()) }, cnc.nodeStatusUpdateFrequency, stopCh)
109109
}
110110

111111
// UpdateNodeStatus updates the node status, such as node addresses
112-
func (cnc *CloudNodeController) UpdateNodeStatus() {
112+
func (cnc *CloudNodeController) UpdateNodeStatus(ctx context.Context) {
113113
instances, ok := cnc.cloud.Instances()
114114
if !ok {
115115
utilruntime.HandleError(fmt.Errorf("failed to get instances from cloud provider"))
@@ -123,20 +123,20 @@ func (cnc *CloudNodeController) UpdateNodeStatus() {
123123
}
124124

125125
for i := range nodes.Items {
126-
cnc.updateNodeAddress(&nodes.Items[i], instances)
126+
cnc.updateNodeAddress(ctx, &nodes.Items[i], instances)
127127
}
128128
}
129129

130130
// UpdateNodeAddress updates the nodeAddress of a single node
131-
func (cnc *CloudNodeController) updateNodeAddress(node *v1.Node, instances cloudprovider.Instances) {
131+
func (cnc *CloudNodeController) updateNodeAddress(ctx context.Context, node *v1.Node, instances cloudprovider.Instances) {
132132
// Do not process nodes that are still tainted
133133
cloudTaint := getCloudTaint(node.Spec.Taints)
134134
if cloudTaint != nil {
135135
klog.V(5).Infof("This node %s is still tainted. Will not process.", node.Name)
136136
return
137137
}
138138
// Node that isn't present according to the cloud provider shouldn't have its address updated
139-
exists, err := ensureNodeExistsByProviderID(instances, node)
139+
exists, err := ensureNodeExistsByProviderID(ctx, instances, node)
140140
if err != nil {
141141
// Continue to update node address when not sure the node is not exists
142142
klog.Errorf("%v", err)
@@ -145,7 +145,7 @@ func (cnc *CloudNodeController) updateNodeAddress(node *v1.Node, instances cloud
145145
return
146146
}
147147

148-
nodeAddresses, err := getNodeAddressesByProviderIDOrName(instances, node)
148+
nodeAddresses, err := getNodeAddressesByProviderIDOrName(ctx, instances, node)
149149
if err != nil {
150150
klog.Errorf("%v", err)
151151
return
@@ -192,7 +192,7 @@ func (cnc *CloudNodeController) updateNodeAddress(node *v1.Node, instances cloud
192192
}
193193
}
194194

195-
func (cnc *CloudNodeController) UpdateCloudNode(_, newObj interface{}) {
195+
func (cnc *CloudNodeController) UpdateCloudNode(ctx context.Context, _, newObj interface{}) {
196196
node, ok := newObj.(*v1.Node)
197197
if !ok {
198198
utilruntime.HandleError(fmt.Errorf("unexpected object type: %v", newObj))
@@ -205,11 +205,11 @@ func (cnc *CloudNodeController) UpdateCloudNode(_, newObj interface{}) {
205205
return
206206
}
207207

208-
cnc.initializeNode(node)
208+
cnc.initializeNode(ctx, node)
209209
}
210210

211211
// AddCloudNode handles initializing new nodes registered with the cloud taint.
212-
func (cnc *CloudNodeController) AddCloudNode(obj interface{}) {
212+
func (cnc *CloudNodeController) AddCloudNode(ctx context.Context, obj interface{}) {
213213
node := obj.(*v1.Node)
214214

215215
cloudTaint := getCloudTaint(node.Spec.Taints)
@@ -218,11 +218,11 @@ func (cnc *CloudNodeController) AddCloudNode(obj interface{}) {
218218
return
219219
}
220220

221-
cnc.initializeNode(node)
221+
cnc.initializeNode(ctx, node)
222222
}
223223

224224
// This processes nodes that were added into the cluster, and cloud initialize them if appropriate
225-
func (cnc *CloudNodeController) initializeNode(node *v1.Node) {
225+
func (cnc *CloudNodeController) initializeNode(ctx context.Context, node *v1.Node) {
226226

227227
instances, ok := cnc.cloud.Instances()
228228
if !ok {
@@ -259,7 +259,7 @@ func (cnc *CloudNodeController) initializeNode(node *v1.Node) {
259259
}
260260

261261
if curNode.Spec.ProviderID == "" {
262-
providerID, err := cloudprovider.GetInstanceProviderID(context.TODO(), cnc.cloud, types.NodeName(curNode.Name))
262+
providerID, err := cloudprovider.GetInstanceProviderID(ctx, cnc.cloud, types.NodeName(curNode.Name))
263263
if err == nil {
264264
curNode.Spec.ProviderID = providerID
265265
} else {
@@ -270,7 +270,7 @@ func (cnc *CloudNodeController) initializeNode(node *v1.Node) {
270270
}
271271
}
272272

273-
nodeAddresses, err := getNodeAddressesByProviderIDOrName(instances, curNode)
273+
nodeAddresses, err := getNodeAddressesByProviderIDOrName(ctx, instances, curNode)
274274
if err != nil {
275275
return err
276276
}
@@ -283,15 +283,15 @@ func (cnc *CloudNodeController) initializeNode(node *v1.Node) {
283283
}
284284
}
285285

286-
if instanceType, err := getInstanceTypeByProviderIDOrName(instances, curNode); err != nil {
286+
if instanceType, err := getInstanceTypeByProviderIDOrName(ctx, instances, curNode); err != nil {
287287
return err
288288
} else if instanceType != "" {
289289
klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelInstanceType, instanceType)
290290
curNode.ObjectMeta.Labels[v1.LabelInstanceType] = instanceType
291291
}
292292

293293
if zones, ok := cnc.cloud.Zones(); ok {
294-
zone, err := getZoneByProviderIDOrName(zones, curNode)
294+
zone, err := getZoneByProviderIDOrName(ctx, zones, curNode)
295295
if err != nil {
296296
return fmt.Errorf("failed to get zone from cloud provider: %v", err)
297297
}
@@ -313,7 +313,7 @@ func (cnc *CloudNodeController) initializeNode(node *v1.Node) {
313313
}
314314
// After adding, call UpdateNodeAddress to set the CloudProvider provided IPAddresses
315315
// So that users do not see any significant delay in IP addresses being filled into the node
316-
cnc.updateNodeAddress(curNode, instances)
316+
cnc.updateNodeAddress(ctx, curNode, instances)
317317

318318
klog.Infof("Successfully initialized node %s with cloud provider", node.Name)
319319
return nil
@@ -346,11 +346,11 @@ func excludeCloudTaint(taints []v1.Taint) []v1.Taint {
346346

347347
// ensureNodeExistsByProviderID checks if the instance exists by the provider id,
348348
// If provider id in spec is empty it calls instanceId with node name to get provider id
349-
func ensureNodeExistsByProviderID(instances cloudprovider.Instances, node *v1.Node) (bool, error) {
349+
func ensureNodeExistsByProviderID(ctx context.Context, instances cloudprovider.Instances, node *v1.Node) (bool, error) {
350350
providerID := node.Spec.ProviderID
351351
if providerID == "" {
352352
var err error
353-
providerID, err = instances.InstanceID(context.TODO(), types.NodeName(node.Name))
353+
providerID, err = instances.InstanceID(ctx, types.NodeName(node.Name))
354354
if err != nil {
355355
if err == cloudprovider.InstanceNotFound {
356356
return false, nil
@@ -364,14 +364,14 @@ func ensureNodeExistsByProviderID(instances cloudprovider.Instances, node *v1.No
364364
}
365365
}
366366

367-
return instances.InstanceExistsByProviderID(context.TODO(), providerID)
367+
return instances.InstanceExistsByProviderID(ctx, providerID)
368368
}
369369

370-
func getNodeAddressesByProviderIDOrName(instances cloudprovider.Instances, node *v1.Node) ([]v1.NodeAddress, error) {
371-
nodeAddresses, err := instances.NodeAddressesByProviderID(context.TODO(), node.Spec.ProviderID)
370+
func getNodeAddressesByProviderIDOrName(ctx context.Context, instances cloudprovider.Instances, node *v1.Node) ([]v1.NodeAddress, error) {
371+
nodeAddresses, err := instances.NodeAddressesByProviderID(ctx, node.Spec.ProviderID)
372372
if err != nil {
373373
providerIDErr := err
374-
nodeAddresses, err = instances.NodeAddresses(context.TODO(), types.NodeName(node.Name))
374+
nodeAddresses, err = instances.NodeAddresses(ctx, types.NodeName(node.Name))
375375
if err != nil {
376376
return nil, fmt.Errorf("NodeAddress: Error fetching by providerID: %v Error fetching by NodeName: %v", providerIDErr, err)
377377
}
@@ -412,11 +412,11 @@ func ensureNodeProvidedIPExists(node *v1.Node, nodeAddresses []v1.NodeAddress) (
412412
return nodeIP, nodeIPExists
413413
}
414414

415-
func getInstanceTypeByProviderIDOrName(instances cloudprovider.Instances, node *v1.Node) (string, error) {
416-
instanceType, err := instances.InstanceTypeByProviderID(context.TODO(), node.Spec.ProviderID)
415+
func getInstanceTypeByProviderIDOrName(ctx context.Context, instances cloudprovider.Instances, node *v1.Node) (string, error) {
416+
instanceType, err := instances.InstanceTypeByProviderID(ctx, node.Spec.ProviderID)
417417
if err != nil {
418418
providerIDErr := err
419-
instanceType, err = instances.InstanceType(context.TODO(), types.NodeName(node.Name))
419+
instanceType, err = instances.InstanceType(ctx, types.NodeName(node.Name))
420420
if err != nil {
421421
return "", fmt.Errorf("InstanceType: Error fetching by providerID: %v Error fetching by NodeName: %v", providerIDErr, err)
422422
}
@@ -426,11 +426,11 @@ func getInstanceTypeByProviderIDOrName(instances cloudprovider.Instances, node *
426426

427427
// getZoneByProviderIDorName will attempt to get the zone of node using its providerID
428428
// then it's name. If both attempts fail, an error is returned
429-
func getZoneByProviderIDOrName(zones cloudprovider.Zones, node *v1.Node) (cloudprovider.Zone, error) {
430-
zone, err := zones.GetZoneByProviderID(context.TODO(), node.Spec.ProviderID)
429+
func getZoneByProviderIDOrName(ctx context.Context, zones cloudprovider.Zones, node *v1.Node) (cloudprovider.Zone, error) {
430+
zone, err := zones.GetZoneByProviderID(ctx, node.Spec.ProviderID)
431431
if err != nil {
432432
providerIDErr := err
433-
zone, err = zones.GetZoneByNodeName(context.TODO(), types.NodeName(node.Name))
433+
zone, err = zones.GetZoneByNodeName(ctx, types.NodeName(node.Name))
434434
if err != nil {
435435
return cloudprovider.Zone{}, fmt.Errorf("Zone: Error fetching by providerID: %v Error fetching by NodeName: %v", providerIDErr, err)
436436
}

‎pkg/controller/cloud/node_controller_test.go‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,20 @@ limitations under the License.
1717
package cloud
1818

1919
import (
20+
"context"
2021
"errors"
2122
"testing"
2223
"time"
2324

24-
v1 "k8s.io/api/core/v1"
25+
"k8s.io/api/core/v1"
2526
"k8s.io/client-go/kubernetes/fake"
2627
"k8s.io/client-go/kubernetes/scheme"
2728

2829
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2930
"k8s.io/apimachinery/pkg/types"
3031
"k8s.io/client-go/informers"
3132
"k8s.io/client-go/tools/record"
32-
cloudprovider "k8s.io/cloud-provider"
33+
"k8s.io/cloud-provider"
3334
fakecloud "k8s.io/cloud-provider/fake"
3435
"k8s.io/kubernetes/pkg/controller/testutil"
3536
kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis"
@@ -147,7 +148,7 @@ func TestEnsureNodeExistsByProviderID(t *testing.T) {
147148
}
148149

149150
instances, _ := fc.Instances()
150-
exists, err := ensureNodeExistsByProviderID(instances, tc.node)
151+
exists, err := ensureNodeExistsByProviderID(context.TODO(), instances, tc.node)
151152
assert.Equal(t, err, tc.providerIDErr)
152153

153154
assert.EqualValues(t, tc.expectedCalls, fc.Calls,
@@ -229,7 +230,7 @@ func TestNodeInitialized(t *testing.T) {
229230
}
230231
eventBroadcaster.StartLogging(klog.Infof)
231232

232-
cloudNodeController.AddCloudNode(fnh.Existing[0])
233+
cloudNodeController.AddCloudNode(context.TODO(), fnh.Existing[0])
233234

234235
assert.Equal(t, 1, len(fnh.UpdatedNodes), "Node was not updated")
235236
assert.Equal(t, "node0", fnh.UpdatedNodes[0].Name, "Node was not updated")
@@ -293,7 +294,7 @@ func TestNodeIgnored(t *testing.T) {
293294
}
294295
eventBroadcaster.StartLogging(klog.Infof)
295296

296-
cloudNodeController.AddCloudNode(fnh.Existing[0])
297+
cloudNodeController.AddCloudNode(context.TODO(), fnh.Existing[0])
297298
assert.Equal(t, 0, len(fnh.UpdatedNodes), "Node was wrongly updated")
298299

299300
}
@@ -366,7 +367,7 @@ func TestGCECondition(t *testing.T) {
366367
}
367368
eventBroadcaster.StartLogging(klog.Infof)
368369

369-
cloudNodeController.AddCloudNode(fnh.Existing[0])
370+
cloudNodeController.AddCloudNode(context.TODO(), fnh.Existing[0])
370371

371372
assert.Equal(t, 1, len(fnh.UpdatedNodes), "Node was not updated")
372373
assert.Equal(t, "node0", fnh.UpdatedNodes[0].Name, "Node was not updated")
@@ -455,7 +456,7 @@ func TestZoneInitialized(t *testing.T) {
455456
}
456457
eventBroadcaster.StartLogging(klog.Infof)
457458

458-
cloudNodeController.AddCloudNode(fnh.Existing[0])
459+
cloudNodeController.AddCloudNode(context.TODO(), fnh.Existing[0])
459460

460461
assert.Equal(t, 1, len(fnh.UpdatedNodes), "Node was not updated")
461462
assert.Equal(t, "node0", fnh.UpdatedNodes[0].Name, "Node was not updated")
@@ -545,7 +546,7 @@ func TestNodeAddresses(t *testing.T) {
545546
}
546547
eventBroadcaster.StartLogging(klog.Infof)
547548

548-
cloudNodeController.AddCloudNode(fnh.Existing[0])
549+
cloudNodeController.AddCloudNode(context.TODO(), fnh.Existing[0])
549550

550551
assert.Equal(t, 1, len(fnh.UpdatedNodes), "Node was not updated")
551552
assert.Equal(t, "node0", fnh.UpdatedNodes[0].Name, "Node was not updated")
@@ -562,7 +563,7 @@ func TestNodeAddresses(t *testing.T) {
562563
},
563564
}
564565

565-
cloudNodeController.UpdateNodeStatus()
566+
cloudNodeController.UpdateNodeStatus(context.TODO())
566567

567568
updatedNodes := fnh.GetUpdatedNodesCopy()
568569

@@ -657,13 +658,13 @@ func TestNodeProvidedIPAddresses(t *testing.T) {
657658
}
658659
eventBroadcaster.StartLogging(klog.Infof)
659660

660-
cloudNodeController.AddCloudNode(fnh.Existing[0])
661+
cloudNodeController.AddCloudNode(context.TODO(), fnh.Existing[0])
661662

662663
assert.Equal(t, 1, len(fnh.UpdatedNodes), "Node was not updated")
663664
assert.Equal(t, "node0", fnh.UpdatedNodes[0].Name, "Node was not updated")
664665
assert.Equal(t, 3, len(fnh.UpdatedNodes[0].Status.Addresses), "Node status unexpectedly updated")
665666

666-
cloudNodeController.UpdateNodeStatus()
667+
cloudNodeController.UpdateNodeStatus(context.TODO())
667668

668669
updatedNodes := fnh.GetUpdatedNodesCopy()
669670

@@ -864,7 +865,7 @@ func TestNodeAddressesNotUpdate(t *testing.T) {
864865
cloud: fakeCloud,
865866
}
866867

867-
cloudNodeController.updateNodeAddress(fnh.Existing[0], fakeCloud)
868+
cloudNodeController.updateNodeAddress(context.TODO(), fnh.Existing[0], fakeCloud)
868869

869870
if len(fnh.UpdatedNodes) != 0 {
870871
t.Errorf("Node was not correctly updated, the updated len(nodes) got: %v, wanted=0", len(fnh.UpdatedNodes))
@@ -946,7 +947,7 @@ func TestNodeProviderID(t *testing.T) {
946947
}
947948
eventBroadcaster.StartLogging(klog.Infof)
948949

949-
cloudNodeController.AddCloudNode(fnh.Existing[0])
950+
cloudNodeController.AddCloudNode(context.TODO(), fnh.Existing[0])
950951

951952
assert.Equal(t, 1, len(fnh.UpdatedNodes), "Node was not updated")
952953
assert.Equal(t, "node0", fnh.UpdatedNodes[0].Name, "Node was not updated")
@@ -1029,7 +1030,7 @@ func TestNodeProviderIDAlreadySet(t *testing.T) {
10291030
}
10301031
eventBroadcaster.StartLogging(klog.Infof)
10311032

1032-
cloudNodeController.AddCloudNode(fnh.Existing[0])
1033+
cloudNodeController.AddCloudNode(context.TODO(), fnh.Existing[0])
10331034

10341035
assert.Equal(t, 1, len(fnh.UpdatedNodes), "Node was not updated")
10351036
assert.Equal(t, "node0", fnh.UpdatedNodes[0].Name, "Node was not updated")

‎pkg/controller/cloud/node_lifecycle_controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ func (c *CloudNodeLifecycleController) MonitorNodes() {
166166

167167
// At this point the node has NotReady status, we need to check if the node has been removed
168168
// from the cloud provider. If node cannot be found in cloudprovider, then delete the node
169-
exists, err := ensureNodeExistsByProviderID(instances, node)
169+
exists, err := ensureNodeExistsByProviderID(context.TODO(), instances, node)
170170
if err != nil {
171171
klog.Errorf("error checking if node %s exists: %v", node.Name, err)
172172
continue

0 commit comments

Comments
 (0)