From bc69f45a9456dd9bd31211dcb93817f7dc59d270 Mon Sep 17 00:00:00 2001 From: Rahul Sharma Date: Tue, 17 Dec 2024 03:17:34 +0000 Subject: [PATCH 1/3] use single copy of instances --- cloud/linode/cloud.go | 6 +++-- cloud/linode/route_controller.go | 4 +-- cloud/linode/route_controller_test.go | 36 ++++++++++++++++++--------- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/cloud/linode/cloud.go b/cloud/linode/cloud.go index e9ac261b..8699dc2a 100644 --- a/cloud/linode/cloud.go +++ b/cloud/linode/cloud.go @@ -97,7 +97,9 @@ func newCloud() (cloudprovider.Interface, error) { Options.VPCNames = Options.VPCName } - routes, err := newRoutes(linodeClient) + instances := newInstances(linodeClient) + + routes, err := newRoutes(linodeClient, instances) if err != nil { return nil, fmt.Errorf("routes client was not created successfully: %w", err) } @@ -123,7 +125,7 @@ func newCloud() (cloudprovider.Interface, error) { // create struct that satisfies cloudprovider.Interface lcloud := &linodeCloud{ client: linodeClient, - instances: newInstances(linodeClient), + instances: instances, loadbalancers: newLoadbalancers(linodeClient, region), routes: routes, } diff --git a/cloud/linode/route_controller.go b/cloud/linode/route_controller.go index 8b3bdf47..ed23f16a 100644 --- a/cloud/linode/route_controller.go +++ b/cloud/linode/route_controller.go @@ -62,7 +62,7 @@ type routes struct { routeCache *routeCache } -func newRoutes(client client.Client) (cloudprovider.Routes, error) { +func newRoutes(client client.Client, instances *instances) (cloudprovider.Routes, error) { timeout := 60 if raw, ok := os.LookupEnv("LINODE_ROUTES_CACHE_TTL_SECONDS"); ok { if t, _ := strconv.Atoi(raw); t > 0 { @@ -77,7 +77,7 @@ func newRoutes(client client.Client) (cloudprovider.Routes, error) { return &routes{ client: client, - instances: newInstances(client), + instances: instances, routeCache: &routeCache{ routes: make(map[int][]linodego.VPCIP, 0), ttl: time.Duration(timeout) * time.Second, diff --git a/cloud/linode/route_controller_test.go b/cloud/linode/route_controller_test.go index 6b2efc64..f8bb513f 100644 --- a/cloud/linode/route_controller_test.go +++ b/cloud/linode/route_controller_test.go @@ -33,7 +33,8 @@ func TestListRoutes(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - routeController, err := newRoutes(client) + instances := newInstances(client) + routeController, err := newRoutes(client, instances) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), gomock.Any()).Times(1).Return([]linodego.Instance{}, nil) @@ -56,7 +57,8 @@ func TestListRoutes(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - routeController, err := newRoutes(client) + instances := newInstances(client) + routeController, err := newRoutes(client, instances) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) @@ -82,7 +84,8 @@ func TestListRoutes(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - routeController, err := newRoutes(client) + instances := newInstances(client) + routeController, err := newRoutes(client, instances) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) @@ -123,7 +126,8 @@ func TestListRoutes(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - routeController, err := newRoutes(client) + instances := newInstances(client) + routeController, err := newRoutes(client, instances) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) @@ -164,7 +168,8 @@ func TestListRoutes(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - routeController, err := newRoutes(client) + instances := newInstances(client) + routeController, err := newRoutes(client, instances) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) @@ -179,7 +184,8 @@ func TestListRoutes(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - routeController, err := newRoutes(client) + instances := newInstances(client) + routeController, err := newRoutes(client, instances) assert.NoError(t, err) vpcIP2 := "10.0.0.3" @@ -283,7 +289,8 @@ func TestCreateRoute(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - routeController, err := newRoutes(client) + instances := newInstances(client) + routeController, err := newRoutes(client, instances) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) @@ -315,7 +322,8 @@ func TestCreateRoute(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - routeController, err := newRoutes(client) + instances := newInstances(client) + routeController, err := newRoutes(client, instances) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) @@ -328,7 +336,8 @@ func TestCreateRoute(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - routeController, err := newRoutes(client) + instances := newInstances(client) + routeController, err := newRoutes(client, instances) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{}, nil) @@ -370,7 +379,8 @@ func TestDeleteRoute(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - routeController, err := newRoutes(client) + instances := newInstances(client) + routeController, err := newRoutes(client, instances) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{}, nil) @@ -400,7 +410,8 @@ func TestDeleteRoute(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - routeController, err := newRoutes(client) + instances := newInstances(client) + routeController, err := newRoutes(client, instances) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) @@ -431,7 +442,8 @@ func TestDeleteRoute(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - routeController, err := newRoutes(client) + instances := newInstances(client) + routeController, err := newRoutes(client, instances) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) From c5368d99535f79fc3133d082da9b6370a8c92476 Mon Sep 17 00:00:00 2001 From: Rahul Sharma Date: Tue, 17 Dec 2024 16:25:28 +0000 Subject: [PATCH 2/3] use single copy of instance cache for node_controller as well --- cloud/linode/cloud.go | 11 +++--- cloud/linode/node_controller.go | 4 +-- cloud/linode/route_controller.go | 4 +-- cloud/linode/route_controller_test.go | 48 +++++++++++++-------------- 4 files changed, 34 insertions(+), 33 deletions(-) diff --git a/cloud/linode/cloud.go b/cloud/linode/cloud.go index 8699dc2a..4d398579 100644 --- a/cloud/linode/cloud.go +++ b/cloud/linode/cloud.go @@ -51,6 +51,8 @@ type linodeCloud struct { routes cloudprovider.Routes } +var instanceCache *instances + func init() { cloudprovider.RegisterCloudProvider( ProviderName, @@ -97,9 +99,8 @@ func newCloud() (cloudprovider.Interface, error) { Options.VPCNames = Options.VPCName } - instances := newInstances(linodeClient) - - routes, err := newRoutes(linodeClient, instances) + instanceCache = newInstances(linodeClient) + routes, err := newRoutes(linodeClient, instanceCache) if err != nil { return nil, fmt.Errorf("routes client was not created successfully: %w", err) } @@ -125,7 +126,7 @@ func newCloud() (cloudprovider.Interface, error) { // create struct that satisfies cloudprovider.Interface lcloud := &linodeCloud{ client: linodeClient, - instances: instances, + instances: instanceCache, loadbalancers: newLoadbalancers(linodeClient, region), routes: routes, } @@ -141,7 +142,7 @@ func (c *linodeCloud) Initialize(clientBuilder cloudprovider.ControllerClientBui serviceController := newServiceController(c.loadbalancers.(*loadbalancers), serviceInformer) go serviceController.Run(stopCh) - nodeController := newNodeController(kubeclient, c.client, nodeInformer) + nodeController := newNodeController(kubeclient, c.client, nodeInformer, instanceCache) go nodeController.Run(stopCh) } diff --git a/cloud/linode/node_controller.go b/cloud/linode/node_controller.go index 52e19422..9b390043 100644 --- a/cloud/linode/node_controller.go +++ b/cloud/linode/node_controller.go @@ -42,7 +42,7 @@ type nodeController struct { queue workqueue.TypedDelayingInterface[any] } -func newNodeController(kubeclient kubernetes.Interface, client client.Client, informer v1informers.NodeInformer) *nodeController { +func newNodeController(kubeclient kubernetes.Interface, client client.Client, informer v1informers.NodeInformer, instanceCache *instances) *nodeController { timeout := defaultMetadataTTL if raw, ok := os.LookupEnv("LINODE_METADATA_TTL"); ok { if t, _ := strconv.Atoi(raw); t > 0 { @@ -52,7 +52,7 @@ func newNodeController(kubeclient kubernetes.Interface, client client.Client, in return &nodeController{ client: client, - instances: newInstances(client), + instances: instanceCache, kubeclient: kubeclient, informer: informer, ttl: timeout, diff --git a/cloud/linode/route_controller.go b/cloud/linode/route_controller.go index ed23f16a..b1aa112b 100644 --- a/cloud/linode/route_controller.go +++ b/cloud/linode/route_controller.go @@ -62,7 +62,7 @@ type routes struct { routeCache *routeCache } -func newRoutes(client client.Client, instances *instances) (cloudprovider.Routes, error) { +func newRoutes(client client.Client, instanceCache *instances) (cloudprovider.Routes, error) { timeout := 60 if raw, ok := os.LookupEnv("LINODE_ROUTES_CACHE_TTL_SECONDS"); ok { if t, _ := strconv.Atoi(raw); t > 0 { @@ -77,7 +77,7 @@ func newRoutes(client client.Client, instances *instances) (cloudprovider.Routes return &routes{ client: client, - instances: instances, + instances: instanceCache, routeCache: &routeCache{ routes: make(map[int][]linodego.VPCIP, 0), ttl: time.Duration(timeout) * time.Second, diff --git a/cloud/linode/route_controller_test.go b/cloud/linode/route_controller_test.go index f8bb513f..e6f2bff0 100644 --- a/cloud/linode/route_controller_test.go +++ b/cloud/linode/route_controller_test.go @@ -33,8 +33,8 @@ func TestListRoutes(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - instances := newInstances(client) - routeController, err := newRoutes(client, instances) + instanceCache := newInstances(client) + routeController, err := newRoutes(client, instanceCache) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), gomock.Any()).Times(1).Return([]linodego.Instance{}, nil) @@ -57,8 +57,8 @@ func TestListRoutes(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - instances := newInstances(client) - routeController, err := newRoutes(client, instances) + instanceCache := newInstances(client) + routeController, err := newRoutes(client, instanceCache) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) @@ -84,8 +84,8 @@ func TestListRoutes(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - instances := newInstances(client) - routeController, err := newRoutes(client, instances) + instanceCache := newInstances(client) + routeController, err := newRoutes(client, instanceCache) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) @@ -126,8 +126,8 @@ func TestListRoutes(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - instances := newInstances(client) - routeController, err := newRoutes(client, instances) + instanceCache := newInstances(client) + routeController, err := newRoutes(client, instanceCache) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) @@ -168,8 +168,8 @@ func TestListRoutes(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - instances := newInstances(client) - routeController, err := newRoutes(client, instances) + instanceCache := newInstances(client) + routeController, err := newRoutes(client, instanceCache) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) @@ -184,8 +184,8 @@ func TestListRoutes(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - instances := newInstances(client) - routeController, err := newRoutes(client, instances) + instanceCache := newInstances(client) + routeController, err := newRoutes(client, instanceCache) assert.NoError(t, err) vpcIP2 := "10.0.0.3" @@ -289,8 +289,8 @@ func TestCreateRoute(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - instances := newInstances(client) - routeController, err := newRoutes(client, instances) + instanceCache := newInstances(client) + routeController, err := newRoutes(client, instanceCache) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) @@ -322,8 +322,8 @@ func TestCreateRoute(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - instances := newInstances(client) - routeController, err := newRoutes(client, instances) + instanceCache := newInstances(client) + routeController, err := newRoutes(client, instanceCache) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) @@ -336,8 +336,8 @@ func TestCreateRoute(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - instances := newInstances(client) - routeController, err := newRoutes(client, instances) + instanceCache := newInstances(client) + routeController, err := newRoutes(client, instanceCache) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{}, nil) @@ -379,8 +379,8 @@ func TestDeleteRoute(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - instances := newInstances(client) - routeController, err := newRoutes(client, instances) + instanceCache := newInstances(client) + routeController, err := newRoutes(client, instanceCache) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{}, nil) @@ -410,8 +410,8 @@ func TestDeleteRoute(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - instances := newInstances(client) - routeController, err := newRoutes(client, instances) + instanceCache := newInstances(client) + routeController, err := newRoutes(client, instanceCache) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) @@ -442,8 +442,8 @@ func TestDeleteRoute(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() client := mocks.NewMockClient(ctrl) - instances := newInstances(client) - routeController, err := newRoutes(client, instances) + instanceCache := newInstances(client) + routeController, err := newRoutes(client, instanceCache) assert.NoError(t, err) client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) From dc0e24c1649c1d001e39c0bfd5ce3c8648f4b192 Mon Sep 17 00:00:00 2001 From: Rahul Sharma Date: Tue, 17 Dec 2024 16:56:03 +0000 Subject: [PATCH 3/3] let http to https test run a bit longer as it sometimes takes a bit longer --- e2e/test/lb-with-http-to-https/chainsaw-test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/test/lb-with-http-to-https/chainsaw-test.yaml b/e2e/test/lb-with-http-to-https/chainsaw-test.yaml index d8bd79b9..cca2b4d9 100644 --- a/e2e/test/lb-with-http-to-https/chainsaw-test.yaml +++ b/e2e/test/lb-with-http-to-https/chainsaw-test.yaml @@ -79,7 +79,7 @@ spec: port_443=$(curl --resolve linode.test:443:$IP --cacert ../certificates/ca.crt -s https://linode.test:443 | grep "test-") if [[ -z $port_80 || -z $port_443 ]]; then - sleep 10 + sleep 20 else echo "all pods responded" break