From 5e94ffe90b684d07ce7d1aa370daf0eb2a4eefca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20D=C4=85browski?= Date: Fri, 29 Mar 2024 20:56:08 +0100 Subject: [PATCH 1/2] nodeipam: poll nodes immediately --- pkg/controller/nodeipam/ipam/cidr_allocator.go | 18 +++++++++--------- .../nodeipam/ipam/controller_legacyprovider.go | 3 ++- .../nodeipam/ipam/range_allocator_test.go | 14 -------------- 3 files changed, 11 insertions(+), 24 deletions(-) diff --git a/pkg/controller/nodeipam/ipam/cidr_allocator.go b/pkg/controller/nodeipam/ipam/cidr_allocator.go index 6541e1ca5c5..0e34c094857 100644 --- a/pkg/controller/nodeipam/ipam/cidr_allocator.go +++ b/pkg/controller/nodeipam/ipam/cidr_allocator.go @@ -75,11 +75,10 @@ const ( // updateMaxRetries is the max retries for a failed node updateMaxRetries = 10 -) -// nodePollInterval is used in listing node -// This is a variable instead of a const to enable testing. -var nodePollInterval = 10 * time.Second + // nodePollInterval is used in listing node + nodePollInterval = 10 * time.Second +) // CIDRAllocator is an interface implemented by things that know how // to allocate/occupy/recycle CIDR for nodes. @@ -116,8 +115,7 @@ type nodeReservedCIDRs struct { // New creates a new CIDR range allocator. func New(ctx context.Context, kubeClient clientset.Interface, cloud cloudprovider.Interface, nodeInformer informers.NodeInformer, allocatorType CIDRAllocatorType, allocatorParams CIDRAllocatorParams) (CIDRAllocator, error) { - logger := klog.FromContext(ctx) - nodeList, err := listNodes(logger, kubeClient) + nodeList, err := listNodes(ctx, kubeClient) if err != nil { return nil, err } @@ -132,13 +130,15 @@ func New(ctx context.Context, kubeClient clientset.Interface, cloud cloudprovide } } -func listNodes(logger klog.Logger, kubeClient clientset.Interface) (*v1.NodeList, error) { +func listNodes(ctx context.Context, kubeClient clientset.Interface) (*v1.NodeList, error) { var nodeList *v1.NodeList + logger := klog.FromContext(ctx) + // We must poll because apiserver might not be up. This error causes // controller manager to restart. - if pollErr := wait.Poll(nodePollInterval, apiserverStartupGracePeriod, func() (bool, error) { + if pollErr := wait.PollUntilContextTimeout(ctx, nodePollInterval, apiserverStartupGracePeriod, true, func(ctx context.Context) (bool, error) { var err error - nodeList, err = kubeClient.CoreV1().Nodes().List(context.TODO(), metav1.ListOptions{ + nodeList, err = kubeClient.CoreV1().Nodes().List(ctx, metav1.ListOptions{ FieldSelector: fields.Everything().String(), LabelSelector: labels.Everything().String(), }) diff --git a/pkg/controller/nodeipam/ipam/controller_legacyprovider.go b/pkg/controller/nodeipam/ipam/controller_legacyprovider.go index bb77cac6a48..7cd93385cd5 100644 --- a/pkg/controller/nodeipam/ipam/controller_legacyprovider.go +++ b/pkg/controller/nodeipam/ipam/controller_legacyprovider.go @@ -118,7 +118,8 @@ func NewController( func (c *Controller) Start(logger klog.Logger, nodeInformer informers.NodeInformer) error { logger.Info("Starting IPAM controller", "config", c.config) - nodes, err := listNodes(logger, c.adapter.k8s) + ctx := klog.NewContext(context.TODO(), logger) + nodes, err := listNodes(ctx, c.adapter.k8s) if err != nil { return err } diff --git a/pkg/controller/nodeipam/ipam/range_allocator_test.go b/pkg/controller/nodeipam/ipam/range_allocator_test.go index a5d59483875..c83567033a7 100644 --- a/pkg/controller/nodeipam/ipam/range_allocator_test.go +++ b/pkg/controller/nodeipam/ipam/range_allocator_test.go @@ -292,13 +292,6 @@ func TestOccupyPreExistingCIDR(t *testing.T) { } func TestAllocateOrOccupyCIDRSuccess(t *testing.T) { - // Non-parallel test (overrides global var) - oldNodePollInterval := nodePollInterval - nodePollInterval = test.NodePollInterval - defer func() { - nodePollInterval = oldNodePollInterval - }() - // all tests operate on a single node testCases := []testCase{ { @@ -680,13 +673,6 @@ type releaseTestCase struct { } func TestReleaseCIDRSuccess(t *testing.T) { - // Non-parallel test (overrides global var) - oldNodePollInterval := nodePollInterval - nodePollInterval = test.NodePollInterval - defer func() { - nodePollInterval = oldNodePollInterval - }() - testCases := []releaseTestCase{ { description: "Correctly release preallocated CIDR", From 170d143b1ac459177e9b9e85c235a55e98e405ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20D=C4=85browski?= Date: Sat, 30 Mar 2024 20:46:23 +0100 Subject: [PATCH 2/2] Revert changes in tests --- pkg/controller/nodeipam/ipam/cidr_allocator.go | 6 +++--- .../nodeipam/ipam/range_allocator_test.go | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/pkg/controller/nodeipam/ipam/cidr_allocator.go b/pkg/controller/nodeipam/ipam/cidr_allocator.go index 0e34c094857..6261672bd61 100644 --- a/pkg/controller/nodeipam/ipam/cidr_allocator.go +++ b/pkg/controller/nodeipam/ipam/cidr_allocator.go @@ -75,11 +75,11 @@ const ( // updateMaxRetries is the max retries for a failed node updateMaxRetries = 10 - - // nodePollInterval is used in listing node - nodePollInterval = 10 * time.Second ) +// nodePollInterval is used in listing node +var nodePollInterval = 10 * time.Second + // CIDRAllocator is an interface implemented by things that know how // to allocate/occupy/recycle CIDR for nodes. type CIDRAllocator interface { diff --git a/pkg/controller/nodeipam/ipam/range_allocator_test.go b/pkg/controller/nodeipam/ipam/range_allocator_test.go index c83567033a7..a5d59483875 100644 --- a/pkg/controller/nodeipam/ipam/range_allocator_test.go +++ b/pkg/controller/nodeipam/ipam/range_allocator_test.go @@ -292,6 +292,13 @@ func TestOccupyPreExistingCIDR(t *testing.T) { } func TestAllocateOrOccupyCIDRSuccess(t *testing.T) { + // Non-parallel test (overrides global var) + oldNodePollInterval := nodePollInterval + nodePollInterval = test.NodePollInterval + defer func() { + nodePollInterval = oldNodePollInterval + }() + // all tests operate on a single node testCases := []testCase{ { @@ -673,6 +680,13 @@ type releaseTestCase struct { } func TestReleaseCIDRSuccess(t *testing.T) { + // Non-parallel test (overrides global var) + oldNodePollInterval := nodePollInterval + nodePollInterval = test.NodePollInterval + defer func() { + nodePollInterval = oldNodePollInterval + }() + testCases := []releaseTestCase{ { description: "Correctly release preallocated CIDR",