Skip to content

Commit b64ad42

Browse files
AshleyDumaineAshleyDumaine
committed
massive refactor to deal with cyclic imports if we want to use the instance cache in the node IPAM controller
1 parent 36df8ec commit b64ad42

23 files changed

+699
-660
lines changed

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ fmt:
8080
.PHONY: test
8181
# we say code is not worth testing unless it's formatted
8282
test: fmt codegen
83-
go test -v -coverpkg=./sentry,./cloud/linode/client,./cloud/linode/firewall,./cloud/linode,./cloud/nodeipam,./cloud/nodeipam/ipam -coverprofile ./coverage.out -cover ./sentry/... ./cloud/... $(TEST_ARGS)
83+
go test -v -coverpkg=./sentry,./cloud/linode/client,./cloud/linode/firewall,./cloud/linode,./cloud/linode/utils,./cloud/linode/cache,./cloud/nodeipam,./cloud/nodeipam/ipam -coverprofile ./coverage.out -cover ./sentry/... ./cloud/... $(TEST_ARGS)
8484

8585
.PHONY: build-linux
8686
build-linux: codegen

cloud/linode/instances.go renamed to cloud/linode/cache/instances.go

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package linode
1+
package cache
22

33
import (
44
"context"
@@ -18,6 +18,8 @@ import (
1818
"k8s.io/klog/v2"
1919

2020
"github.com/linode/linode-cloud-controller-manager/cloud/linode/client"
21+
"github.com/linode/linode-cloud-controller-manager/cloud/linode/options"
22+
ccmUtils "github.com/linode/linode-cloud-controller-manager/cloud/linode/utils"
2123
"github.com/linode/linode-cloud-controller-manager/sentry"
2224
)
2325

@@ -60,7 +62,7 @@ func (nc *nodeCache) getInstanceAddresses(instance linodego.Instance, vpcips []s
6062

6163
for _, ip := range instance.IPv4 {
6264
ipType := v1.NodeExternalIP
63-
if isPrivate(ip) {
65+
if ccmUtils.IsPrivate(ip, options.Options.LinodeExternalNetwork) {
6466
ipType = v1.NodeInternalIP
6567
}
6668
ips = append(ips, nodeIP{ip: ip.String(), ipType: ipType})
@@ -94,7 +96,7 @@ func (nc *nodeCache) refreshInstances(ctx context.Context, client client.Client)
9496
// If running within VPC, find instances and store their ips
9597
vpcNodes := map[int][]string{}
9698
vpcIPv6AddrTypes := map[string]v1.NodeAddressType{}
97-
for _, name := range Options.VPCNames {
99+
for _, name := range options.Options.VPCNames {
98100
vpcName := strings.TrimSpace(name)
99101
if vpcName == "" {
100102
continue
@@ -134,7 +136,7 @@ func (nc *nodeCache) refreshInstances(ctx context.Context, client client.Client)
134136
newNodes := make(map[int]linodeInstance, len(instances))
135137
for index, instance := range instances {
136138
// if running within VPC, only store instances in cache which are part of VPC
137-
if len(Options.VPCNames) > 0 && len(vpcNodes[instance.ID]) == 0 {
139+
if len(options.Options.VPCNames) > 0 && len(vpcNodes[instance.ID]) == 0 {
138140
continue
139141
}
140142
node := linodeInstance{
@@ -149,13 +151,14 @@ func (nc *nodeCache) refreshInstances(ctx context.Context, client client.Client)
149151
return nil
150152
}
151153

152-
type instances struct {
154+
type Instances struct {
153155
client client.Client
154156

155157
nodeCache *nodeCache
156158
}
157159

158-
func newInstances(client client.Client) *instances {
160+
// NewInstances creates a new Instances cache with a specified TTL for the nodeCache.
161+
func NewInstances(client client.Client) *Instances {
159162
timeout := 15
160163
if raw, ok := os.LookupEnv("LINODE_INSTANCE_CACHE_TTL"); ok {
161164
if t, err := strconv.Atoi(raw); t > 0 && err == nil {
@@ -164,7 +167,7 @@ func newInstances(client client.Client) *instances {
164167
}
165168
klog.V(3).Infof("TTL for nodeCache set to %d", timeout)
166169

167-
return &instances{client, &nodeCache{
170+
return &Instances{client, &nodeCache{
168171
nodes: make(map[int]linodeInstance, 0),
169172
ttl: time.Duration(timeout) * time.Second,
170173
}}
@@ -178,7 +181,7 @@ func (e instanceNoIPAddressesError) Error() string {
178181
return fmt.Sprintf("instance %d has no IP addresses", e.id)
179182
}
180183

181-
func (i *instances) linodeByIP(kNode *v1.Node) (*linodego.Instance, error) {
184+
func (i *Instances) linodeByIP(kNode *v1.Node) (*linodego.Instance, error) {
182185
i.nodeCache.RLock()
183186
defer i.nodeCache.RUnlock()
184187
var kNodeAddresses []string
@@ -192,7 +195,7 @@ func (i *instances) linodeByIP(kNode *v1.Node) (*linodego.Instance, error) {
192195
}
193196
for _, node := range i.nodeCache.nodes {
194197
for _, nodeIP := range node.instance.IPv4 {
195-
if !isPrivate(nodeIP) && slices.Contains(kNodeAddresses, nodeIP.String()) {
198+
if !ccmUtils.IsPrivate(nodeIP, options.Options.LinodeExternalNetwork) && slices.Contains(kNodeAddresses, nodeIP.String()) {
196199
return node.instance, nil
197200
}
198201
}
@@ -201,7 +204,7 @@ func (i *instances) linodeByIP(kNode *v1.Node) (*linodego.Instance, error) {
201204
return nil, cloudprovider.InstanceNotFound
202205
}
203206

204-
func (i *instances) linodeByName(nodeName types.NodeName) *linodego.Instance {
207+
func (i *Instances) linodeByName(nodeName types.NodeName) *linodego.Instance {
205208
i.nodeCache.RLock()
206209
defer i.nodeCache.RUnlock()
207210
for _, node := range i.nodeCache.nodes {
@@ -213,7 +216,7 @@ func (i *instances) linodeByName(nodeName types.NodeName) *linodego.Instance {
213216
return nil
214217
}
215218

216-
func (i *instances) linodeByID(id int) (*linodego.Instance, error) {
219+
func (i *Instances) linodeByID(id int) (*linodego.Instance, error) {
217220
i.nodeCache.RLock()
218221
defer i.nodeCache.RUnlock()
219222
linodeInstance, ok := i.nodeCache.nodes[id]
@@ -223,8 +226,8 @@ func (i *instances) linodeByID(id int) (*linodego.Instance, error) {
223226
return linodeInstance.instance, nil
224227
}
225228

226-
// listAllInstances returns all instances in nodeCache
227-
func (i *instances) listAllInstances(ctx context.Context) ([]linodego.Instance, error) {
229+
// ListAllInstances returns all instances in nodeCache
230+
func (i *Instances) ListAllInstances(ctx context.Context) ([]linodego.Instance, error) {
228231
if err := i.nodeCache.refreshInstances(ctx, i.client); err != nil {
229232
return nil, err
230233
}
@@ -236,7 +239,8 @@ func (i *instances) listAllInstances(ctx context.Context) ([]linodego.Instance,
236239
return instances, nil
237240
}
238241

239-
func (i *instances) lookupLinode(ctx context.Context, node *v1.Node) (*linodego.Instance, error) {
242+
// LookupLinode looks up a Linode instance by its ProviderID or NodeName.
243+
func (i *Instances) LookupLinode(ctx context.Context, node *v1.Node) (*linodego.Instance, error) {
240244
if err := i.nodeCache.refreshInstances(ctx, i.client); err != nil {
241245
return nil, err
242246
}
@@ -247,8 +251,8 @@ func (i *instances) lookupLinode(ctx context.Context, node *v1.Node) (*linodego.
247251
sentry.SetTag(ctx, "provider_id", providerID)
248252
sentry.SetTag(ctx, "node_name", node.Name)
249253

250-
if providerID != "" && isLinodeProviderID(providerID) {
251-
id, err := parseProviderID(providerID)
254+
if providerID != "" && ccmUtils.IsLinodeProviderID(providerID) {
255+
id, err := ccmUtils.ParseProviderID(providerID)
252256
if err != nil {
253257
sentry.CaptureError(ctx, err)
254258
return nil, err
@@ -265,9 +269,9 @@ func (i *instances) lookupLinode(ctx context.Context, node *v1.Node) (*linodego.
265269
return i.linodeByIP(node)
266270
}
267271

268-
func (i *instances) InstanceExists(ctx context.Context, node *v1.Node) (bool, error) {
272+
func (i *Instances) InstanceExists(ctx context.Context, node *v1.Node) (bool, error) {
269273
ctx = sentry.SetHubOnContext(ctx)
270-
if _, err := i.lookupLinode(ctx, node); err != nil {
274+
if _, err := i.LookupLinode(ctx, node); err != nil {
271275
if errors.Is(err, cloudprovider.InstanceNotFound) {
272276
return false, nil
273277
}
@@ -278,9 +282,9 @@ func (i *instances) InstanceExists(ctx context.Context, node *v1.Node) (bool, er
278282
return true, nil
279283
}
280284

281-
func (i *instances) InstanceShutdown(ctx context.Context, node *v1.Node) (bool, error) {
285+
func (i *Instances) InstanceShutdown(ctx context.Context, node *v1.Node) (bool, error) {
282286
ctx = sentry.SetHubOnContext(ctx)
283-
instance, err := i.lookupLinode(ctx, node)
287+
instance, err := i.LookupLinode(ctx, node)
284288
if err != nil {
285289
sentry.CaptureError(ctx, err)
286290
return false, err
@@ -296,9 +300,9 @@ func (i *instances) InstanceShutdown(ctx context.Context, node *v1.Node) (bool,
296300
return false, nil
297301
}
298302

299-
func (i *instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprovider.InstanceMetadata, error) {
303+
func (i *Instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprovider.InstanceMetadata, error) {
300304
ctx = sentry.SetHubOnContext(ctx)
301-
linode, err := i.lookupLinode(ctx, node)
305+
linode, err := i.LookupLinode(ctx, node)
302306
if err != nil {
303307
sentry.CaptureError(ctx, err)
304308
return nil, err
@@ -344,7 +348,7 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloud
344348
klog.Infof("Instance %s, assembled IP addresses: %v", node.Name, addresses)
345349
// note that Zone is omitted as it's not a thing in Linode
346350
meta := &cloudprovider.InstanceMetadata{
347-
ProviderID: fmt.Sprintf("%v%v", providerIDPrefix, linode.ID),
351+
ProviderID: fmt.Sprintf("%v%v", ccmUtils.ProviderIDPrefix, linode.ID),
348352
NodeAddresses: addresses,
349353
InstanceType: linode.Type,
350354
Region: linode.Region,
@@ -353,9 +357,9 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloud
353357
return meta, nil
354358
}
355359

356-
func (i *instances) getLinodeAddresses(ctx context.Context, node *v1.Node) ([]nodeIP, error) {
360+
func (i *Instances) getLinodeAddresses(ctx context.Context, node *v1.Node) ([]nodeIP, error) {
357361
ctx = sentry.SetHubOnContext(ctx)
358-
instance, err := i.lookupLinode(ctx, node)
362+
instance, err := i.LookupLinode(ctx, node)
359363
if err != nil {
360364
sentry.CaptureError(ctx, err)
361365
return nil, err

0 commit comments

Comments
 (0)