Skip to content

Add different cloud provider support between BSL and VSL#124

Closed
shubham-pampattiwar wants to merge 5 commits intoopenshift:masterfrom
shubham-pampattiwar:add-diff-bsl-vsl-support
Closed

Add different cloud provider support between BSL and VSL#124
shubham-pampattiwar wants to merge 5 commits intoopenshift:masterfrom
shubham-pampattiwar:add-diff-bsl-vsl-support

Conversation

@shubham-pampattiwar
Copy link
Member

@shubham-pampattiwar shubham-pampattiwar commented Jun 29, 2021

This PR does the following:

Changes tested for:

  • AWS BSL + AWS VSL
  • Azure BSL + AWS VSL

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 29, 2021
@shubham-pampattiwar shubham-pampattiwar marked this pull request as draft June 29, 2021 15:36
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 29, 2021
@shubham-pampattiwar shubham-pampattiwar marked this pull request as ready for review June 30, 2021 23:44
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 30, 2021
@shubham-pampattiwar shubham-pampattiwar force-pushed the add-diff-bsl-vsl-support branch from 8f3c94b to 590970d Compare June 30, 2021 23:53
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 30, 2021
@shubham-pampattiwar shubham-pampattiwar self-assigned this Jul 6, 2021
@shubham-pampattiwar shubham-pampattiwar changed the title Add diff-bsl-vsl-support Add different cloud provider support between BSL and VSL Jul 6, 2021
Copy link
Member

@dymurray dymurray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Just need to test it on my end. I would like to merge this before cutting a new release

Copy link
Member

@dymurray dymurray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall the functionality LGTM and it ran in my env fine.

I have some questions about the ansible logic that I think is just my peeve of doing this in ansible (and we did this before so I'm probably overthinking it).

I'm fine to merge this but if you want to wait until next week I'd be interested in talking through some ideas I have on changing this slightly.

@hhpatel14
Copy link
Contributor

/retest

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 23, 2021
@shubham-pampattiwar shubham-pampattiwar force-pushed the add-diff-bsl-vsl-support branch from 5ab592f to e556917 Compare July 23, 2021 18:29
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 23, 2021
@openshift-ci
Copy link

openshift-ci bot commented Jul 27, 2021

@shubham-pampattiwar: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 27, 2021
@dymurray dymurray closed this Aug 11, 2021
openshift-merge-bot bot pushed a commit that referenced this pull request Sep 9, 2025
…1937)

* Exponential Backoff for CloudStorage reconciler

- Add Conditions field to CloudStorageStatus for better observability
- Implement exponential backoff by returning errors on bucket operations
- Controller-runtime automatically handles retries (5ms to 1000s max)
- Add condition constants for type-safe reason strings
- Create mock bucket client for improved testing
- Add comprehensive tests for backoff behavior and conditions

Key improvements:
- Standard Kubernetes pattern using built-in workqueue backoff
- Self-healing: continues retrying with increasing delays
- Better observability through status conditions
- Per-item backoff: each CloudStorage CR gets independent retry timing

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Add exponential backoff for CloudStorage status update failures (#124)

* Initial plan

* Add exponential backoff for status update failures

- Return error instead of just logging when final status update fails
- Add documentation test explaining the change
- Ensures controller-runtime's exponential backoff is triggered for status update failures

Addresses PR comment #1937 discussion_r2330918689

Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com>
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/oadp-operator that referenced this pull request Sep 9, 2025
…shift#124)

* Initial plan

* Add exponential backoff for status update failures

- Return error instead of just logging when final status update fails
- Add documentation test explaining the change
- Ensures controller-runtime's exponential backoff is triggered for status update failures

Addresses PR comment openshift#1937 discussion_r2330918689

Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com>
openshift-merge-bot bot pushed a commit that referenced this pull request Sep 10, 2025
…queueAfter (#1951)

* Exponential Backoff for CloudStorage reconciler

- Add Conditions field to CloudStorageStatus for better observability
- Implement exponential backoff by returning errors on bucket operations
- Controller-runtime automatically handles retries (5ms to 1000s max)
- Add condition constants for type-safe reason strings
- Create mock bucket client for improved testing
- Add comprehensive tests for backoff behavior and conditions

Key improvements:
- Standard Kubernetes pattern using built-in workqueue backoff
- Self-healing: continues retrying with increasing delays
- Better observability through status conditions
- Per-item backoff: each CloudStorage CR gets independent retry timing

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Add exponential backoff for CloudStorage status update failures (#124)

* Initial plan

* Add exponential backoff for status update failures

- Return error instead of just logging when final status update fails
- Add documentation test explaining the change
- Ensures controller-runtime's exponential backoff is triggered for status update failures

Addresses PR comment #1937 discussion_r2330918689

Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com>

---------

Co-authored-by: Tiger Kaovilai <tkaovila@redhat.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support different cloud providers between BSL + VSL

3 participants