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

feat: azure support #613

Merged
merged 2 commits into from
Dec 19, 2024
Merged

Conversation

noaccOS
Copy link
Collaborator

@noaccOS noaccOS commented Oct 9, 2024

add azure support by adding a waffle compatible storage module based on azurex.

the configuration mostly maps to existing s3 configuration, so for now I did not deem it necessary to separate the configuration. I am however open to implementing a separate configuration if this was preferred though.

Two new environment variables were added for configuration:

  • STORAGE_TYPE, which can be either s3 or azure
  • AZURE_API_URL, because it needs more flexibility than just scheme, host and port.

also add integration tests and allow runtime configuration of s3 storage

closes #233

@coveralls
Copy link

coveralls commented Oct 9, 2024

Pull Request Test Coverage Report for Build ed8f2f75dcbdc53f01633d70396ac929ff4d4b0e-PR-613

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+79.0%) to 79.008%

Totals Coverage Status
Change from base Build 12281651344: 79.0%
Covered Lines: 1227
Relevant Lines: 1553

💛 - Coveralls

@coveralls
Copy link

Pull Request Test Coverage Report for Build a7116681ef8baefe2232c24de76888c5218d59a8-PR-613

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-1.6%) to 79.008%

Files with Coverage Reduction New Missed Lines %
lib/edgehog/base_images/bucket_storage.ex 3 0.0%
Totals Coverage Status
Change from base Build 1cd7df0671e9ad40c0bf17c4fb5316ad96e497b0: -1.6%
Covered Lines: 1227
Relevant Lines: 1553

💛 - Coveralls

Copy link
Collaborator

@rbino rbino left a comment

Choose a reason for hiding this comment

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

Not urgent, but before merging this needs:

  • Changelog entry
  • Documentation about how to use Azure Blob Storage in the "Deploying with Kubernetes" page

@noaccOS noaccOS marked this pull request as draft December 3, 2024 18:04
@noaccOS noaccOS force-pushed the feat/azure branch 11 times, most recently from 81e9f1c to 6242cec Compare December 5, 2024 10:57
@noaccOS noaccOS marked this pull request as ready for review December 5, 2024 11:06
Comment on lines 45 to 58
def delete(%BaseImage{} = base_image) do
%BaseImage{
url: url,
tenant_id: tenant_id,
version: base_image_version,
base_image_collection_id: base_image_collection_id
} = base_image

scope = %{
tenant_id: tenant_id,
base_image_collection_id: base_image_collection_id,
base_image_version: base_image_version
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

This was broken from previous releases.
It would be nice to have this fix in v0.9 and forward-port it to main.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in #764

add runtime s3 configuration for the test environment as well as prod.
this is needed during CI but should not affect normal use 

Signed-off-by: Francesco Noacco <[email protected]>
Copy link
Collaborator

@davidebriani davidebriani left a comment

Choose a reason for hiding this comment

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

Brilliant PR!
If we manage to use a more informative changelog line, it's a merge

CHANGELOG.md Outdated
@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [0.10] - Unreleased
### Added
- Managed OTA operations expose the update target that created them in graphql ([#356](https://github.com/edgehog-device-manager/edgehog/issues/356).
- Azure support
Copy link
Collaborator

Choose a reason for hiding this comment

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

Azure support for what?
Written like this it could mean anything: e.g. we added support for deploying Edgehog services on AKS clusters.

A reference to the relevant issue would be a bonus point.

It could be something like:

Support for using Azure Storage as the persistence layer for asset uploads ([#233](https://github.com/edgehog-device-manager/edgehog/issues/233)).

add azure support by adding a waffle compatible storage module based
on azurex.

new environment variables were added for configuration:
- STORAGE_TYPE, which can be either s3 or azure
- AZURE_BLOB_ENDPOINT: equivalent to s3's asset_host. maps to azurex's api_url.
- AZURE_REGION: the region for the blob storage. only used if AZURE_BLOB_ENDPOINT is unset.
- AZURE_CONTAINER: equivalent to s3's bucket
- AZURE_STORAGE_ACCOUNT_NAME: azure's AccountName
- AZURE_STORAGE_ACCOUNT_KEY: azure's AccountKey
- AZURE_CONNECTION_STRING: an azure connection string. maps to previous values.

Signed-off-by: Francesco Noacco <[email protected]>
@davidebriani davidebriani merged commit 6d3e198 into edgehog-device-manager:main Dec 19, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Azure Blob Storage
5 participants