Skip to content

Commit 27d1e1c

Browse files
AkarshESYashwantGohokar
authored andcommitted
List instances to check for authorization issues
1 parent 6e761fe commit 27d1e1c

File tree

6 files changed

+77
-1
lines changed

6 files changed

+77
-1
lines changed

pkg/cloudprovider/providers/oci/instances.go

+30-1
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,8 @@ func (cp *CloudProvider) InstanceExistsByProviderID(ctx context.Context, provide
243243
}
244244
instance, err := cp.client.Compute().GetInstance(ctx, instanceID)
245245
if client.IsNotFound(err) {
246-
return false, nil
246+
return cp.checkForAuthorizationError(ctx, providerID)
247+
247248
}
248249
if err != nil {
249250
return false, err
@@ -252,6 +253,34 @@ func (cp *CloudProvider) InstanceExistsByProviderID(ctx context.Context, provide
252253
return !client.IsInstanceInTerminalState(instance), nil
253254
}
254255

256+
func (cp *CloudProvider) checkForAuthorizationError(ctx context.Context, instanceId string) (bool, error) {
257+
cp.logger.With("instanceId", instanceId).Info("Received 404 for an instance, listing instances to check for authorization errors")
258+
compartmentId, err := cp.getCompartmentIDByInstanceID(instanceId)
259+
if err != nil {
260+
return false, err
261+
}
262+
// to eliminate AD specific issues, list all ADs and make AD specific requests
263+
availabilityDomains, err := cp.client.Identity().ListAvailabilityDomains(ctx, compartmentId)
264+
for _, availabilityDomain := range availabilityDomains {
265+
instances, err := cp.client.Compute().ListInstancesByCompartmentAndAD(ctx, compartmentId, *availabilityDomain.Name)
266+
// if we are getting errors for ListInstances the issue can be authorization or other issues
267+
// so to be safe we return the error back as we can't list instances in the compartment
268+
if err != nil {
269+
cp.logger.With("instanceId", instanceId).Info("Received error when listing instances to check for authorization errors")
270+
return false, err
271+
}
272+
273+
for _, instance := range instances {
274+
if *instance.Id == instanceId {
275+
// Can only happen if changes are done in policy in-between requests
276+
return true, nil
277+
}
278+
}
279+
}
280+
281+
return false, nil
282+
}
283+
255284
// InstanceShutdownByProviderID returns true if the instance is shutdown in cloudprovider.
256285
func (cp *CloudProvider) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) {
257286
//Please do not try to optimise it by using InstanceCache because we prefer correctness over efficiency here

pkg/cloudprovider/providers/oci/instances_test.go

+4
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,10 @@ func (MockOCIClient) Identity() client.IdentityInterface {
310310
// MockComputeClient mocks Compute client implementation
311311
type MockComputeClient struct{}
312312

313+
func (c MockComputeClient) ListInstancesByCompartmentAndAD(ctx context.Context, compartmentId, availabilityDomain string) (response []core.Instance, err error) {
314+
return nil, nil
315+
}
316+
313317
func (MockComputeClient) GetInstance(ctx context.Context, id string) (*core.Instance, error) {
314318
if instance, ok := instances[id]; ok {
315319
return instance, nil

pkg/csi/driver/bv_controller_test.go

+4
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,10 @@ type MockComputeClient struct {
542542
compute util.MockOCIComputeClient
543543
}
544544

545+
func (c *MockComputeClient) ListInstancesByCompartmentAndAD(ctx context.Context, compartmentId, availabilityDomain string) (response []core.Instance, err error) {
546+
return nil, nil
547+
}
548+
545549
// GetInstance gets information about the specified instance.
546550
func (c *MockComputeClient) GetInstance(ctx context.Context, id string) (*core.Instance, error) {
547551
if instance, ok := instances[id]; ok {

pkg/oci/client/compute.go

+31
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
type ComputeInterface interface {
2727
// GetInstance gets information about the specified instance.
2828
GetInstance(ctx context.Context, id string) (*core.Instance, error)
29+
ListInstancesByCompartmentAndAD(ctx context.Context, compartmentId, availabilityDomain string) (response []core.Instance, err error)
2930

3031
// GetInstanceByNodeName gets the OCI instance corresponding to the given
3132
// Kubernetes node name.
@@ -53,6 +54,36 @@ func (c *client) GetInstance(ctx context.Context, id string) (*core.Instance, er
5354
return &resp.Instance, nil
5455
}
5556

57+
func (c *client) ListInstancesByCompartmentAndAD(ctx context.Context, compartmentID, availabilityDomain string) ([]core.Instance, error) {
58+
var (
59+
page *string
60+
instances []core.Instance
61+
)
62+
for {
63+
if !c.rateLimiter.Reader.TryAccept() {
64+
return nil, RateLimitError(false, "ListInstances")
65+
}
66+
resp, err := c.compute.ListInstances(ctx, core.ListInstancesRequest{
67+
AvailabilityDomain: &availabilityDomain,
68+
CompartmentId: &compartmentID,
69+
Page: page,
70+
RequestMetadata: c.requestMetadata,
71+
})
72+
incRequestCounter(err, listVerb, instanceResource)
73+
74+
if err != nil {
75+
return nil, errors.WithStack(err)
76+
}
77+
78+
instances = append(instances, resp.Items...)
79+
if page = resp.OpcNextPage; resp.OpcNextPage == nil {
80+
break
81+
}
82+
}
83+
84+
return instances, nil
85+
}
86+
5687
func (c *client) getInstanceByDisplayName(ctx context.Context, compartmentID, displayName string) (*core.Instance, error) {
5788
var (
5889
page *string

pkg/volume/provisioner/block/block_test.go

+4
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,10 @@ func (c *MockFileStorageClient) AwaitMountTargetActive(ctx context.Context, logg
227227

228228
type MockComputeClient struct{}
229229

230+
func (c *MockComputeClient) ListInstancesByCompartmentAndAD(ctx context.Context, compartmentId, availabilityDomain string) (response []core.Instance, err error) {
231+
return nil, nil
232+
}
233+
230234
// GetInstance gets information about the specified instance.
231235
func (c *MockComputeClient) GetInstance(ctx context.Context, id string) (*core.Instance, error) {
232236
return nil, nil

pkg/volume/provisioner/fss/fss_test.go

+4
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,10 @@ func (c *MockFileStorageClient) AwaitMountTargetActive(ctx context.Context, logg
225225

226226
type MockComputeClient struct{}
227227

228+
func (c *MockComputeClient) ListInstancesByCompartmentAndAD(ctx context.Context, compartmentId, availabilityDomain string) (response []core.Instance, err error) {
229+
return nil, nil
230+
}
231+
228232
// GetInstance gets information about the specified instance.
229233
func (c *MockComputeClient) GetInstance(ctx context.Context, id string) (*core.Instance, error) {
230234
return nil, nil

0 commit comments

Comments
 (0)