Skip to content

Commit

Permalink
[fix] : use single copy of instances cache (#276)
Browse files Browse the repository at this point in the history
* use single copy of instances

* use single copy of instance cache for node_controller as well

* let http to https test run a bit longer as it sometimes takes a bit longer
  • Loading branch information
rahulait authored Dec 18, 2024
1 parent 6b6064a commit 162a835
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 20 deletions.
9 changes: 6 additions & 3 deletions cloud/linode/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ type linodeCloud struct {
routes cloudprovider.Routes
}

var instanceCache *instances

func init() {
cloudprovider.RegisterCloudProvider(
ProviderName,
Expand Down Expand Up @@ -97,7 +99,8 @@ func newCloud() (cloudprovider.Interface, error) {
Options.VPCNames = Options.VPCName
}

routes, err := newRoutes(linodeClient)
instanceCache = newInstances(linodeClient)
routes, err := newRoutes(linodeClient, instanceCache)
if err != nil {
return nil, fmt.Errorf("routes client was not created successfully: %w", err)
}
Expand All @@ -123,7 +126,7 @@ func newCloud() (cloudprovider.Interface, error) {
// create struct that satisfies cloudprovider.Interface
lcloud := &linodeCloud{
client: linodeClient,
instances: newInstances(linodeClient),
instances: instanceCache,
loadbalancers: newLoadbalancers(linodeClient, region),
routes: routes,
}
Expand All @@ -139,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)
}

Expand Down
4 changes: 2 additions & 2 deletions cloud/linode/node_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions cloud/linode/route_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ type routes struct {
routeCache *routeCache
}

func newRoutes(client client.Client) (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 {
Expand All @@ -77,7 +77,7 @@ func newRoutes(client client.Client) (cloudprovider.Routes, error) {

return &routes{
client: client,
instances: newInstances(client),
instances: instanceCache,
routeCache: &routeCache{
routes: make(map[int][]linodego.VPCIP, 0),
ttl: time.Duration(timeout) * time.Second,
Expand Down
36 changes: 24 additions & 12 deletions cloud/linode/route_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ func TestListRoutes(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
client := mocks.NewMockClient(ctrl)
routeController, err := newRoutes(client)
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)
Expand All @@ -56,7 +57,8 @@ func TestListRoutes(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
client := mocks.NewMockClient(ctrl)
routeController, err := newRoutes(client)
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)
Expand All @@ -82,7 +84,8 @@ func TestListRoutes(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
client := mocks.NewMockClient(ctrl)
routeController, err := newRoutes(client)
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)
Expand Down Expand Up @@ -123,7 +126,8 @@ func TestListRoutes(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
client := mocks.NewMockClient(ctrl)
routeController, err := newRoutes(client)
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)
Expand Down Expand Up @@ -164,7 +168,8 @@ func TestListRoutes(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
client := mocks.NewMockClient(ctrl)
routeController, err := newRoutes(client)
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)
Expand All @@ -179,7 +184,8 @@ func TestListRoutes(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
client := mocks.NewMockClient(ctrl)
routeController, err := newRoutes(client)
instanceCache := newInstances(client)
routeController, err := newRoutes(client, instanceCache)
assert.NoError(t, err)

vpcIP2 := "10.0.0.3"
Expand Down Expand Up @@ -283,7 +289,8 @@ func TestCreateRoute(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
client := mocks.NewMockClient(ctrl)
routeController, err := newRoutes(client)
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)
Expand Down Expand Up @@ -315,7 +322,8 @@ func TestCreateRoute(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
client := mocks.NewMockClient(ctrl)
routeController, err := newRoutes(client)
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)
Expand All @@ -328,7 +336,8 @@ func TestCreateRoute(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
client := mocks.NewMockClient(ctrl)
routeController, err := newRoutes(client)
instanceCache := newInstances(client)
routeController, err := newRoutes(client, instanceCache)
assert.NoError(t, err)

client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{}, nil)
Expand Down Expand Up @@ -370,7 +379,8 @@ func TestDeleteRoute(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
client := mocks.NewMockClient(ctrl)
routeController, err := newRoutes(client)
instanceCache := newInstances(client)
routeController, err := newRoutes(client, instanceCache)
assert.NoError(t, err)

client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{}, nil)
Expand Down Expand Up @@ -400,7 +410,8 @@ func TestDeleteRoute(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
client := mocks.NewMockClient(ctrl)
routeController, err := newRoutes(client)
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)
Expand Down Expand Up @@ -431,7 +442,8 @@ func TestDeleteRoute(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
client := mocks.NewMockClient(ctrl)
routeController, err := newRoutes(client)
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)
Expand Down
2 changes: 1 addition & 1 deletion e2e/test/lb-with-http-to-https/chainsaw-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 162a835

Please sign in to comment.