Skip to content

Commit 3a9b0b4

Browse files
kaovilaiclaude
kaovilai
and
claude
committed
Add retry logic and status conditions to CloudStorage controller
- Limit bucket creation retries to 3 attempts with exponential backoff (30s, 1m, 2m) - Add Conditions and RetryCount fields to CloudStorageStatus for tracking state - Set BucketReady=True when bucket is available, BucketCreationFailed=True after retry limit - Provide clear user guidance to recreate CR once permissions are fixed - Reset retry count on successful bucket creation or when bucket already exists - Update CRDs and OLM bundle with new status fields Resolves infinite retry loops on permission denied errors like GCP 403 responses. 🤖 Generated with [Claude Code](https://claude.ai/code) Fix unparam linting issue in createTestCloudCredentialsSecret Remove unused namespace parameter since it always receives the same value (test-namespace) 🤖 Generated with [Claude Code](https://claude.ai/code) Refactor CloudStorage controller tests to use mock bucket client - Add mock bucket client implementation for testing - Refactor all tests to use dependency injection via BucketClientFactory - Extract mock AWS credentials to a constant - Create helper function for test cloud credentials secret creation - Add helper function to find conditions by type - Improve test reliability by using mocks instead of actual bucket operations - Add retry logic and status conditions to CloudStorage controller 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 9725289 commit 3a9b0b4

9 files changed

+768
-39
lines changed

‎api/v1alpha1/cloudstorage_types.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,14 @@ const (
2929
GCPBucketProvider CloudStorageProvider = CloudStorageProvider(DefaultPluginGCP)
3030
)
3131

32+
// CloudStorage condition constants
33+
const (
34+
// ConditionBucketReady indicates that the bucket has been successfully created and is available
35+
ConditionBucketReady = "BucketReady"
36+
// ConditionBucketCreationFailed indicates that bucket creation has failed permanently after retries
37+
ConditionBucketCreationFailed = "BucketCreationFailed"
38+
)
39+
3240
type CloudStorageSpec struct {
3341
// name is the name requested for the bucket (aws, gcp) or container (azure)
3442
Name string `json:"name"`
@@ -63,6 +71,12 @@ type CloudStorageStatus struct {
6371
// LastSyncTimestamp is the last time the contents of the CloudStorage was synced
6472
// +operator-sdk:csv:customresourcedefinitions:type=status,displayName="LastSyncTimestamp"
6573
LastSynced *metav1.Time `json:"lastSyncTimestamp,omitempty"`
74+
// Conditions represent the latest available observations of the CloudStorage's current state
75+
// +operator-sdk:csv:customresourcedefinitions:type=status
76+
Conditions []metav1.Condition `json:"conditions,omitempty"`
77+
// RetryCount tracks the number of failed bucket creation attempts
78+
// +operator-sdk:csv:customresourcedefinitions:type=status
79+
RetryCount int `json:"retryCount,omitempty"`
6680
}
6781

6882
// +kubebuilder:object:root=true

‎api/v1alpha1/zz_generated.deepcopy.go

Lines changed: 22 additions & 15 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎bundle/manifests/oadp-operator.clusterserviceversion.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,10 @@ spec:
394394
kind: CloudStorage
395395
name: cloudstorages.oadp.openshift.io
396396
statusDescriptors:
397+
- description: Conditions represent the latest available observations of the
398+
CloudStorage's current state
399+
displayName: Conditions
400+
path: conditions
397401
- description: LastSyncTimestamp is the last time the contents of the CloudStorage
398402
was synced
399403
displayName: LastSyncTimestamp
@@ -402,6 +406,9 @@ spec:
402406
(azure)
403407
displayName: Name
404408
path: name
409+
- description: RetryCount tracks the number of failed bucket creation attempts
410+
displayName: Retry Count
411+
path: retryCount
405412
version: v1alpha1
406413
- description: DataDownload represents a data download of a volume snapshot. There
407414
is one DataDownload created per volume to be restored.

‎bundle/manifests/oadp.openshift.io_cloudstorages.yaml

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,64 @@ spec:
9999
type: object
100100
status:
101101
properties:
102+
conditions:
103+
description: Conditions represent the latest available observations
104+
of the CloudStorage's current state
105+
items:
106+
description: Condition contains details for one aspect of the current
107+
state of this API Resource.
108+
properties:
109+
lastTransitionTime:
110+
description: |-
111+
lastTransitionTime is the last time the condition transitioned from one status to another.
112+
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
113+
format: date-time
114+
type: string
115+
message:
116+
description: |-
117+
message is a human readable message indicating details about the transition.
118+
This may be an empty string.
119+
maxLength: 32768
120+
type: string
121+
observedGeneration:
122+
description: |-
123+
observedGeneration represents the .metadata.generation that the condition was set based upon.
124+
For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date
125+
with respect to the current state of the instance.
126+
format: int64
127+
minimum: 0
128+
type: integer
129+
reason:
130+
description: |-
131+
reason contains a programmatic identifier indicating the reason for the condition's last transition.
132+
Producers of specific condition types may define expected values and meanings for this field,
133+
and whether the values are considered a guaranteed API.
134+
The value should be a CamelCase string.
135+
This field may not be empty.
136+
maxLength: 1024
137+
minLength: 1
138+
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
139+
type: string
140+
status:
141+
description: status of the condition, one of True, False, Unknown.
142+
enum:
143+
- "True"
144+
- "False"
145+
- Unknown
146+
type: string
147+
type:
148+
description: type of condition in CamelCase or in foo.example.com/CamelCase.
149+
maxLength: 316
150+
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
151+
type: string
152+
required:
153+
- lastTransitionTime
154+
- message
155+
- reason
156+
- status
157+
- type
158+
type: object
159+
type: array
102160
lastSyncTimestamp:
103161
description: LastSyncTimestamp is the last time the contents of the
104162
CloudStorage was synced
@@ -108,6 +166,10 @@ spec:
108166
description: Name is the name requested for the bucket (aws, gcp)
109167
or container (azure)
110168
type: string
169+
retryCount:
170+
description: RetryCount tracks the number of failed bucket creation
171+
attempts
172+
type: integer
111173
required:
112174
- name
113175
type: object

‎config/crd/bases/oadp.openshift.io_cloudstorages.yaml

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,64 @@ spec:
9999
type: object
100100
status:
101101
properties:
102+
conditions:
103+
description: Conditions represent the latest available observations
104+
of the CloudStorage's current state
105+
items:
106+
description: Condition contains details for one aspect of the current
107+
state of this API Resource.
108+
properties:
109+
lastTransitionTime:
110+
description: |-
111+
lastTransitionTime is the last time the condition transitioned from one status to another.
112+
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
113+
format: date-time
114+
type: string
115+
message:
116+
description: |-
117+
message is a human readable message indicating details about the transition.
118+
This may be an empty string.
119+
maxLength: 32768
120+
type: string
121+
observedGeneration:
122+
description: |-
123+
observedGeneration represents the .metadata.generation that the condition was set based upon.
124+
For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date
125+
with respect to the current state of the instance.
126+
format: int64
127+
minimum: 0
128+
type: integer
129+
reason:
130+
description: |-
131+
reason contains a programmatic identifier indicating the reason for the condition's last transition.
132+
Producers of specific condition types may define expected values and meanings for this field,
133+
and whether the values are considered a guaranteed API.
134+
The value should be a CamelCase string.
135+
This field may not be empty.
136+
maxLength: 1024
137+
minLength: 1
138+
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
139+
type: string
140+
status:
141+
description: status of the condition, one of True, False, Unknown.
142+
enum:
143+
- "True"
144+
- "False"
145+
- Unknown
146+
type: string
147+
type:
148+
description: type of condition in CamelCase or in foo.example.com/CamelCase.
149+
maxLength: 316
150+
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
151+
type: string
152+
required:
153+
- lastTransitionTime
154+
- message
155+
- reason
156+
- status
157+
- type
158+
type: object
159+
type: array
102160
lastSyncTimestamp:
103161
description: LastSyncTimestamp is the last time the contents of the
104162
CloudStorage was synced
@@ -108,6 +166,10 @@ spec:
108166
description: Name is the name requested for the bucket (aws, gcp)
109167
or container (azure)
110168
type: string
169+
retryCount:
170+
description: RetryCount tracks the number of failed bucket creation
171+
attempts
172+
type: integer
111173
required:
112174
- name
113175
type: object

‎config/manifests/bases/oadp-operator.clusterserviceversion.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,10 @@ spec:
403403
kind: CloudStorage
404404
name: cloudstorages.oadp.openshift.io
405405
statusDescriptors:
406+
- description: Conditions represent the latest available observations of the
407+
CloudStorage's current state
408+
displayName: Conditions
409+
path: conditions
406410
- description: LastSyncTimestamp is the last time the contents of the CloudStorage
407411
was synced
408412
displayName: LastSyncTimestamp
@@ -411,6 +415,9 @@ spec:
411415
(azure)
412416
displayName: Name
413417
path: name
418+
- description: RetryCount tracks the number of failed bucket creation attempts
419+
displayName: Retry Count
420+
path: retryCount
414421
version: v1alpha1
415422
- description: DataProtectionApplication represents configuration to install a
416423
data protection application to safely backup and restore, perform disaster

0 commit comments

Comments
 (0)