Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for OCS S3 snapstore #261

Merged
merged 1 commit into from
Oct 5, 2020

Conversation

stoyanr
Copy link
Contributor

@stoyanr stoyanr commented Sep 24, 2020

What this PR does / why we need it:
Adds support for OpenShift Container Storage (OCS) S3-compatible snapstore. Also refactors the existing ECS snapstore to avoid code duplication since the 2 snapstores are quite similar.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:
See also gardener/etcd-druid#98.

Release note:

Added support for OpenShift Container Storage (OCS) S3 storage type.

@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 24, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 24, 2020
@stoyanr
Copy link
Contributor Author

stoyanr commented Sep 24, 2020

/cc @shreyas-s-rao

@stoyanr
Copy link
Contributor Author

stoyanr commented Sep 24, 2020

/cc @rootfs

@rootfs
Copy link

rootfs commented Sep 24, 2020

/lgtm

@gardener-robot gardener-robot added the reviewed/lgtm Has approval for merging label Sep 24, 2020
@amshuman-kr
Copy link
Collaborator

@stoyanr Thanks a lot for the PR and the new provider support! We were held up with some landscape issues and review already this week. I apologize.

@stoyanr
Copy link
Contributor Author

stoyanr commented Sep 29, 2020

/cc @shreyas-s-rao @amshuman-kr could you please take a look into this? Thanks!

@shreyas-s-rao
Copy link
Collaborator

Hi @stoyanr. Thanks for your PR. Will review it today, since I was busy the past few days.

Copy link
Collaborator

@amshuman-kr amshuman-kr left a comment

Choose a reason for hiding this comment

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

@stoyanr Thanks a lot for the PR and sorry for the delay in the review! The PR looks good. Would it be possible for you to give access to an OCS environment for us to test before merging?

@stoyanr stoyanr force-pushed the add-ocs-snapstore branch from 17d9939 to bfce75c Compare October 5, 2020 11:40
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/ok-to-test-tm Has approval for running integration tests on TestMachinery needs/ok-to-test-tm Requires integration tests to be run on TestMachinery and removed reviewed/ok-to-test-tm Has approval for running integration tests on TestMachinery labels Oct 5, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 5, 2020
Copy link
Collaborator

@shreyas-s-rao shreyas-s-rao left a comment

Choose a reason for hiding this comment

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

@stoyanr Thanks for the PR and for refactoring the generic S3 snapstore. LGTM.

@shreyas-s-rao shreyas-s-rao merged commit 9906561 into gardener:master Oct 5, 2020
@stoyanr stoyanr deleted the add-ocs-snapstore branch October 5, 2020 14:24
@amshuman-kr amshuman-kr added this to the v0.11.0 milestone Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test-tm Requires integration tests to be run on TestMachinery reviewed/lgtm Has approval for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants