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.

Sign up for GitHub

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

Jump to bottom

Enhancing etcd-backup-restore to support Azurite - the Azure Blob Storage emulator #699

Merged
merged 4 commits into from
Feb 13, 2024
Merged

Enhancing etcd-backup-restore to support Azurite - the Azure Blob Storage emulator #699

merged 4 commits into from
Feb 13, 2024

Conversation

renormalize
Copy link
Member

@renormalize renormalize commented Jan 2, 2024 •
edited by shreyas-s-rao
Loading

What this PR does / why we need it:
Changes to make etcd-backup-restore work with Azurite - the Azure Blob Storage emulator to enable easier local development and testing.

Two new environment variables are introduced to make use of Azurite with etcd-backup-restore:

  • EMULATOR_ENABLED - indicates whether Azurite is being used instead of ABS infrastructure. Takes boolean values, and hence can be set to true or false.
  • AZURE_STORAGE_API_ENDPOINT - stores the endpoint at which Azurite is hosted. For example, if Azurite is hosted at http://localhost:10000, AZURE_STORAGE_API_ENDPOINT is set to this address. etcd-backup-restore reacts to the presence of this environment variable and sets the endpoint for Azurite accordingly if and only if the EMULATOR_ENABLED is set to true. This environment variable currently can not be used to set a custom endpoint for ABS, and can only be made use of to use Azurite.

Which issue(s) this PR fixes:
Fixes #698

Special notes for your reviewer:

Release note:

Added support to use Azurite, which emulates Azure Blob Storage for local development and testing - which can be enabled by setting the `AZURE_EMULATOR_ENABLED` and  `AZURITE_STORAGE_API_ENDPOINT` environment variables.

@renormalize renormalize added this to the v0.28.0 milestone Jan 2, 2024
@renormalize renormalize requested a review from a team as a code owner January 2, 2024 11:43
@gardener-robot gardener-robot added needs/review Needs review size/s Size of pull request is small (see gardener-robot robot/bots/size.py) labels Jan 2, 2024
@renormalize renormalize self-assigned this Jan 2, 2024
@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 Jan 2, 2024
@renormalize renormalize added kind/enhancement Enhancement, improvement, extension area/dev-productivity Developer productivity related (how to improve development) area/cost Cost related needs/lgtm Needs approval for merging platform/azure Microsoft Azure platform/infrastructure area/testing Testing related 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 Jan 2, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 added the needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 2, 2024
@gardener-robot gardener-robot removed the needs/review Needs review label Jan 2, 2024
@renormalize renormalize removed their assignment Jan 2, 2024
@renormalize
Copy link
Member Author

/retest

@ishan16696 ishan16696 modified the milestones: v0.28.0, v0.29.0 Jan 9, 2024
@renormalize renormalize 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 Jan 10, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 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 Jan 10, 2024
@renormalize renormalize 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 Jan 10, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 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 Jan 10, 2024
@renormalize renormalize mentioned this pull request Jan 10, 2024
Merged
seshachalam-yv
seshachalam-yv requested changes Feb 7, 2024
View reviewed changes
chart/etcd-backup-restore/templates/etcd-backup-secret.yaml Outdated Show resolved Hide resolved
chart/etcd-backup-restore/templates/etcd-statefulset.yaml Outdated Show resolved Hide resolved
pkg/snapstore/abs_snapstore.go Outdated Show resolved Hide resolved
@gardener-robot gardener-robot added needs/changes Needs (more) changes and removed needs/lgtm Needs approval for merging labels Feb 7, 2024
@renormalize
Copy link
Member Author

@seshachalam-yv @anveshreddy18
Since a common environment variable will be used to signify that an emulator is being used, would it not be preferable if this environment variable const envEmulatorEnabled = "EMULATOR_ENABLED" be moved to pkg/snapstore/snapstore.go or pkg/types/snapstore.go - instead of defining the same variable both in abs_snapstore.go and gcs_snapstore.go?
Ideally, this environment variable will be used with localstack as well, so moving it would be preferable in my opinion.

@seshachalam-yv
Copy link
Contributor

@seshachalam-yv @anveshreddy18 Since a common environment variable will be used to signify that an emulator is being used, would it not be preferable if this environment variable const envEmulatorEnabled = "EMULATOR_ENABLED" be moved to pkg/snapstore/snapstore.go or pkg/types/snapstore.go - instead of defining the same variable both in abs_snapstore.go and gcs_snapstore.go? Ideally, this environment variable will be used with localstack as well, so moving it would be preferable in my opinion.

Yeah we can move it to pkg/snapstore/snapstore.go

@renormalize
Added support to etcd-backup-restore to make use of Azurite - the Azu…
7a49bff 
…re Blob Storage Emulator while conforming to common design for Azurite and GCS emulator

* Added the `AZURE_ENABLE_STORAGE_EMULATOR` environment variable to indicate
  that the Azurite emulator is being used. It takes a boolean value.

* The `AZURE_STORAGE_API_ENDPOINT` environment variable stores the endpoint
  exposed by the Azurite emulator.

* Added the optional fields `enableAzurite` and `storageAPIEndpoint` to the
  example file example/storage-provider-secrets/00-azure-blob-storage-secret.yaml

* Added the optional fields `enableAzurite` and `storageAPIEndpoint` to the
  template file chart/etcd-backup-restore/templates/etcd-statefulset.yaml

* Added the optional fields `enableAzurite` and `storageAPIEndpoint` to the
  file chart/etcd-backup-restore/values.yaml

* Added the optional fields `enableAzurite` and `storageAPIEndpoint` to the
  template file chart/etcd-backup-restore/templates/etcd-backup-secret.yaml
@renormalize
Address review comments 1, make changes for uniformity with gardener#697
97fac6f 


* Changed the environment variable that is used to signify that the Azurite
  emulator is being used from `AZURE_ENABLE_STORAGE_EMULATOR` to `EMULATOR_ENABLED`.
  The corresponding changes are also to be made in gardener#697.

* The field which signifies the emulator is being used for Azure in the secret is
  changed from `enableAzurite` to `emulatorEnabled`, mirroring the enivornment
  variable name change.

* A new function `constuctBlobServiceURL()` is introduced, which makes the
  handling of the detection of the Azurite emulator logic cleaner.
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has 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 Feb 7, 2024
@renormalize
Unit tests added for ConstructBlobServiceURL
3b0b9a1 
* `constructBlobServiceURL` visibility changed to `ConstructBlobServiceURL`.

* Unit tests added for `ConstructBlobServiceURL`.

* Redundant arguments in `ConstructBlobServiceURL` removed.
@gardener-robot gardener-robot added size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) and removed size/s Size of pull request is small (see gardener-robot robot/bots/size.py) labels Feb 8, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 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 Feb 8, 2024
@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 Feb 8, 2024
seshachalam-yv
seshachalam-yv requested changes Feb 8, 2024
View reviewed changes
pkg/snapstore/abs_snapstore.go Outdated Show resolved Hide resolved
pkg/snapstore/snapstore_test.go Outdated Show resolved Hide resolved
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has 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 Feb 8, 2024
@renormalize
Addressed review comments 2
279cc2d 
* The function `ConstructBlobServiceURL()` is restructured to not be nested.

* Previously defined constants in `pkg/snapstore/snapstore.go` used in
  `snapstore_test.go` instead of repeating the constants.
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has 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 Feb 8, 2024
seshachalam-yv
seshachalam-yv approved these changes Feb 8, 2024
View reviewed changes
Copy link
Contributor

@seshachalam-yv seshachalam-yv left a comment

Choose a reason for hiding this comment

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

Thank you, @renormalize, for incorporating the tests and addressing the feedback provided in my review comments.
/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/changes Needs (more) changes labels Feb 8, 2024
@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 Feb 8, 2024
@renormalize renormalize changed the title Changes to enable etcd-backup-restore to make use of Azurite - the Azure Blob Storage emulator Enhancingetcd-backup-restore to support Azurite - the Azure Blob Storage emulator Feb 9, 2024
@renormalize renormalize changed the title Enhancingetcd-backup-restore to support Azurite - the Azure Blob Storage emulator Enhancing etcd-backup-restore to support Azurite - the Azure Blob Storage emulator Feb 9, 2024
@ishan16696
Copy link
Member

/assign

@renormalize renormalize merged commit edff58c into gardener:master Feb 13, 2024
9 checks passed
@renormalize renormalize deleted the azurite branch April 4, 2024 14:21
@anveshreddy18 anveshreddy18 mentioned this pull request Jul 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ishan16696 ishan16696 ishan16696 approved these changes

@seshachalam-yv seshachalam-yv seshachalam-yv approved these changes

Assignees

@ishan16696 ishan16696

@renormalize renormalize

Labels
area/cost Cost related area/dev-productivity Developer productivity related (how to improve development) area/testing Testing related kind/enhancement Enhancement, improvement, extension needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) platform/azure Microsoft Azure platform/infrastructure reviewed/lgtm Has approval for merging reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) size/m Size of pull request is medium (see gardener-robot robot/bots/size.py)
Projects
None yet
Milestone
v0.29.0
7 participants
@renormalize @seshachalam-yv @ishan16696 @gardener-robot-ci-1 @gardener-robot-ci-2 @gardener-robot-ci-3 @gardener-robot