Skip to content

Commit 1214dc2

Browse files
aojeaaojea
committed
kubelet: Use node addresses from informer
The kubelet certificate manager was using a closure to get the node addresses, but this closure depended on a static field that was only updated during the node status update. This created a twisted dependency between the node.status reconcile loops and the certificate manager. This commit fixes this issue by using the node informer to get the node addresses directly. This ensures that the kubelet always requests a certificate with the latest node addresses.
1 parent 616d6d4 commit 1214dc2

File tree

4 files changed

+191
-15
lines changed

4 files changed

+191
-15
lines changed

‎pkg/kubelet/kubelet.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1096,9 +1096,6 @@ type Kubelet struct {
10961096
rootDirectory string
10971097
podLogsDirectory string
10981098

1099-
lastObservedNodeAddressesMux sync.RWMutex
1100-
lastObservedNodeAddresses []v1.NodeAddress
1101-
11021099
// onRepeatedHeartbeatFailure is called when a heartbeat operation fails more than once. optional.
11031100
onRepeatedHeartbeatFailure func()
11041101

‎pkg/kubelet/kubelet_getters.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,3 +480,13 @@ func (kl *Kubelet) setCachedMachineInfo(info *cadvisorapiv1.MachineInfo) {
480480
defer kl.machineInfoLock.Unlock()
481481
kl.machineInfo = info
482482
}
483+
484+
// getLastStableNodeAddresses returns the last observed node addresses.
485+
func (kl *Kubelet) getLastObservedNodeAddresses() []v1.NodeAddress {
486+
node, err := kl.GetNode()
487+
if err != nil || node == nil {
488+
klog.V(4).InfoS("fail to obtain node from local cache", "node", kl.nodeName, "error", err)
489+
return nil
490+
}
491+
return node.Status.Addresses
492+
}

‎pkg/kubelet/kubelet_getters_test.go‎

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ import (
2121
"testing"
2222

2323
"github.com/stretchr/testify/assert"
24+
v1 "k8s.io/api/core/v1"
25+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26+
"k8s.io/apimachinery/pkg/types"
27+
cloudproviderapi "k8s.io/cloud-provider/api"
2428
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
2529
)
2630

@@ -132,3 +136,180 @@ func TestHandlerSupportsUserNamespaces(t *testing.T) {
132136
assert.False(t, got)
133137
assert.Error(t, err)
134138
}
139+
140+
func Test_getLastObservedNodeAddresses(t *testing.T) {
141+
testCases := []struct {
142+
name string
143+
nodeName types.NodeName
144+
node *v1.Node
145+
externalCloudProvider bool
146+
expectedAddrs []v1.NodeAddress
147+
}{
148+
{
149+
name: "node not found",
150+
nodeName: "test-node",
151+
node: nil,
152+
expectedAddrs: nil,
153+
},
154+
{
155+
name: "empty addresses",
156+
nodeName: "test-node",
157+
node: &v1.Node{
158+
ObjectMeta: metav1.ObjectMeta{Name: "test-node"},
159+
Status: v1.NodeStatus{Addresses: []v1.NodeAddress{}},
160+
},
161+
expectedAddrs: nil,
162+
},
163+
{
164+
name: "no taints",
165+
nodeName: "test-node",
166+
node: &v1.Node{
167+
ObjectMeta: metav1.ObjectMeta{Name: "test-node"},
168+
Status: v1.NodeStatus{
169+
Addresses: []v1.NodeAddress{
170+
{Type: v1.NodeInternalIP, Address: "10.0.0.1"},
171+
{Type: v1.NodeHostName, Address: "test-node"},
172+
},
173+
},
174+
},
175+
expectedAddrs: []v1.NodeAddress{
176+
{Type: v1.NodeInternalIP, Address: "10.0.0.1"},
177+
{Type: v1.NodeHostName, Address: "test-node"},
178+
},
179+
},
180+
{
181+
name: "other taints",
182+
nodeName: "test-node",
183+
node: &v1.Node{
184+
ObjectMeta: metav1.ObjectMeta{Name: "test-node"},
185+
Spec: v1.NodeSpec{
186+
Taints: []v1.Taint{
187+
{Key: "other-taint", Effect: v1.TaintEffectNoSchedule},
188+
},
189+
},
190+
Status: v1.NodeStatus{
191+
Addresses: []v1.NodeAddress{
192+
{Type: v1.NodeInternalIP, Address: "10.0.0.1"},
193+
{Type: v1.NodeHostName, Address: "test-node"},
194+
},
195+
},
196+
},
197+
expectedAddrs: []v1.NodeAddress{
198+
{Type: v1.NodeInternalIP, Address: "10.0.0.1"},
199+
{Type: v1.NodeHostName, Address: "test-node"},
200+
},
201+
},
202+
{
203+
name: "external cloud provider no taints",
204+
nodeName: "test-node",
205+
externalCloudProvider: true,
206+
node: &v1.Node{
207+
ObjectMeta: metav1.ObjectMeta{Name: "test-node"},
208+
Status: v1.NodeStatus{
209+
Addresses: []v1.NodeAddress{
210+
{Type: v1.NodeInternalIP, Address: "10.0.0.1"},
211+
{Type: v1.NodeHostName, Address: "test-node"},
212+
},
213+
},
214+
},
215+
expectedAddrs: []v1.NodeAddress{
216+
{Type: v1.NodeInternalIP, Address: "10.0.0.1"},
217+
{Type: v1.NodeHostName, Address: "test-node"},
218+
},
219+
},
220+
{
221+
name: "no external cloud provider with external cloud provider taint",
222+
nodeName: "test-node",
223+
externalCloudProvider: false,
224+
node: &v1.Node{
225+
ObjectMeta: metav1.ObjectMeta{Name: "test-node"},
226+
Spec: v1.NodeSpec{
227+
Taints: []v1.Taint{
228+
{Key: cloudproviderapi.TaintExternalCloudProvider, Effect: v1.TaintEffectNoSchedule},
229+
},
230+
},
231+
Status: v1.NodeStatus{
232+
Addresses: []v1.NodeAddress{
233+
{Type: v1.NodeInternalIP, Address: "10.0.0.1"},
234+
{Type: v1.NodeHostName, Address: "test-node"},
235+
},
236+
},
237+
},
238+
expectedAddrs: []v1.NodeAddress{
239+
{Type: v1.NodeInternalIP, Address: "10.0.0.1"},
240+
{Type: v1.NodeHostName, Address: "test-node"},
241+
},
242+
},
243+
{
244+
name: "external cloud provider taint",
245+
nodeName: "test-node",
246+
externalCloudProvider: true,
247+
node: &v1.Node{
248+
ObjectMeta: metav1.ObjectMeta{Name: "test-node"},
249+
Spec: v1.NodeSpec{
250+
Taints: []v1.Taint{
251+
{Key: cloudproviderapi.TaintExternalCloudProvider, Effect: v1.TaintEffectNoSchedule},
252+
},
253+
},
254+
Status: v1.NodeStatus{
255+
Addresses: []v1.NodeAddress{
256+
{Type: v1.NodeInternalIP, Address: "10.0.0.1"},
257+
{Type: v1.NodeHostName, Address: "test-node"},
258+
},
259+
},
260+
},
261+
expectedAddrs: []v1.NodeAddress{
262+
{Type: v1.NodeInternalIP, Address: "10.0.0.1"},
263+
{Type: v1.NodeHostName, Address: "test-node"},
264+
}},
265+
{
266+
name: "external cloud provider other taint",
267+
nodeName: "test-node",
268+
externalCloudProvider: true,
269+
node: &v1.Node{
270+
ObjectMeta: metav1.ObjectMeta{Name: "test-node"},
271+
Spec: v1.NodeSpec{
272+
Taints: []v1.Taint{
273+
{Key: "other-taint", Effect: v1.TaintEffectNoSchedule},
274+
},
275+
},
276+
Status: v1.NodeStatus{
277+
Addresses: []v1.NodeAddress{
278+
{Type: v1.NodeInternalIP, Address: "10.0.0.1"},
279+
{Type: v1.NodeHostName, Address: "test-node"},
280+
},
281+
},
282+
},
283+
expectedAddrs: []v1.NodeAddress{
284+
{Type: v1.NodeInternalIP, Address: "10.0.0.1"},
285+
{Type: v1.NodeHostName, Address: "test-node"},
286+
},
287+
},
288+
}
289+
290+
for _, tc := range testCases {
291+
t.Run(tc.name, func(t *testing.T) {
292+
testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */)
293+
defer testKubelet.Cleanup()
294+
kl := testKubelet.kubelet
295+
kl.nodeName = types.NodeName(tc.nodeName)
296+
kl.externalCloudProvider = tc.externalCloudProvider
297+
nodeLister := testNodeLister{}
298+
if tc.node != nil {
299+
nodeLister.nodes = append(nodeLister.nodes, tc.node)
300+
}
301+
kl.nodeLister = nodeLister
302+
addrs := kl.getLastObservedNodeAddresses()
303+
304+
if len(addrs) != len(tc.expectedAddrs) {
305+
t.Errorf("expected %d addresses, got %d", len(tc.expectedAddrs), len(addrs))
306+
} else {
307+
for i := range addrs {
308+
if addrs[i] != tc.expectedAddrs[i] {
309+
t.Errorf("expected address %v, got %v", tc.expectedAddrs[i], addrs[i])
310+
}
311+
}
312+
}
313+
})
314+
}
315+
}

‎pkg/kubelet/kubelet_node_status.go‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,6 @@ func (kl *Kubelet) patchNodeStatus(originalNode, node *v1.Node) (*v1.Node, error
644644
return nil, err
645645
}
646646
kl.lastStatusReportTime = kl.clock.Now()
647-
kl.setLastObservedNodeAddresses(updatedNode.Status.Addresses)
648647

649648
readyIdx, readyCondition := nodeutil.GetNodeCondition(&updatedNode.Status, v1.NodeReady)
650649
if readyIdx >= 0 && readyCondition.Status == v1.ConditionTrue {
@@ -721,17 +720,6 @@ func (kl *Kubelet) setNodeStatus(ctx context.Context, node *v1.Node) {
721720
}
722721
}
723722

724-
func (kl *Kubelet) setLastObservedNodeAddresses(addresses []v1.NodeAddress) {
725-
kl.lastObservedNodeAddressesMux.Lock()
726-
defer kl.lastObservedNodeAddressesMux.Unlock()
727-
kl.lastObservedNodeAddresses = addresses
728-
}
729-
func (kl *Kubelet) getLastObservedNodeAddresses() []v1.NodeAddress {
730-
kl.lastObservedNodeAddressesMux.RLock()
731-
defer kl.lastObservedNodeAddressesMux.RUnlock()
732-
return kl.lastObservedNodeAddresses
733-
}
734-
735723
// defaultNodeStatusFuncs is a factory that generates the default set of
736724
// setNodeStatus funcs
737725
func (kl *Kubelet) defaultNodeStatusFuncs() []func(context.Context, *v1.Node) error {

0 commit comments

Comments
 (0)