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 integration tests for Azure Storage driver #1572

Merged
merged 5 commits into from May 2, 2021

Conversation

c-w
Copy link
Member

@c-w c-w commented Mar 20, 2021

Add integration tests for Azure Storage driver

Description

As discussed in #1553 (comment), this pull request ports the Azure Storage driver integration tests from libcloud-tests to trunk.

The tests are split into a generic storage integration test base (which uses the high level storage driver API to run various scenarios) and specific instantiations which exercise various Azure Storage backends. The intent is that the test base in the future can be re-used to quickly add integration tests for other storage backends.

Notes:

  • Transient errors with the Azure Ubuntu package mirrors required adding a call to apt-get update to fix the CI, similar to what's described in Unable to connect to azure.archive.ubuntu.com flaky failure on ubuntu-latest actions/runner-images#675.
  • The new storage integration have been added as a new tox target to avoid slowing down the unit test suite, re-using some existing patterns laid out by integration tests of the compute API.
  • While libcloud-tests ran against containerized and cloud-based storage accounts, for now this pull request only ports the former as I will soon loose access to my free cloud credits. However, if someone wants to add me to an Azure subscription that is funded for this project, or provide me with the credentials for a service principal, I'm more than happy to add the required code to run the test suite against the live cloud storage backends.

Status

  • done, ready for review

Checklist

@c-w c-w requested a review from Kami March 20, 2021 16:09
@c-w c-w force-pushed the storage-azure-integration-tests branch 5 times, most recently from 5647484 to 5544da1 Compare March 21, 2021 03:10
@c-w c-w removed the request for review from Kami March 21, 2021 03:22
@c-w c-w force-pushed the storage-azure-integration-tests branch from 5544da1 to 13721ca Compare March 21, 2021 03:29
@c-w c-w force-pushed the storage-azure-integration-tests branch from 13721ca to e24f58e Compare March 21, 2021 04:05
@codecov-io
Copy link

codecov-io commented Mar 21, 2021

Codecov Report

Merging #1572 (6803023) into trunk (cc5987b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##            trunk    #1572   +/-   ##
=======================================
  Coverage   83.00%   83.00%           
=======================================
  Files         394      394           
  Lines       84990    84990           
  Branches     9038     9038           
=======================================
  Hits        70550    70550           
  Misses      11373    11373           
  Partials     3067     3067           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc5987b...6803023. Read the comment docs.

@c-w c-w force-pushed the storage-azure-integration-tests branch 7 times, most recently from 2af6945 to 1e569d3 Compare March 21, 2021 04:50
@c-w c-w force-pushed the storage-azure-integration-tests branch from 1e569d3 to c6d108a Compare March 21, 2021 04:59
@c-w c-w requested a review from Kami March 21, 2021 05:01
self.driver = driver_class(**kwargs)

def tearDown(self):
for container in self.driver.list_containers():
Copy link
Member

@Kami Kami Mar 21, 2021

Choose a reason for hiding this comment

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

Should we wrap this with try / except and still proceed if there is an error (but I guess we should at least log an error if there is an exception and instruct user to manually delete those containers or similar).

Copy link
Member Author

@c-w c-w Mar 23, 2021

Choose a reason for hiding this comment

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

Done in d9f577e.

@Kami
Copy link
Member

Kami commented Mar 21, 2021

Thanks for working on this.

While libcloud-tests ran against containerized and cloud-based storage accounts, for now this pull request only ports the former as I will soon loose access to my free cloud credits. However, if someone wants to add me to an Azure subscription that is funded for this project, or provide me with the credentials for a service principal, I'm more than happy to add the required code to run the test suite against the live cloud storage backends.

@tonybaloney Do you happen to know if Azure has some kind of free credits for OSS program, similar to the AWS one which we could use for running end to end tests?

@c-w
Copy link
Member Author

c-w commented Mar 22, 2021

@Kami @tonybaloney Given that ASF is a non-profit, the Azure for non-profits grant could be an avenue to get some credits.

@c-w c-w force-pushed the storage-azure-integration-tests branch from 0c4d093 to d9f577e Compare March 23, 2021 14:18
@tonybaloney
Copy link
Contributor

tonybaloney commented Mar 23, 2021 via email

@c-w
Copy link
Member Author

c-w commented Apr 13, 2021

@tonybaloney @Kami Any thoughts on next steps for this integration test PR?

[testenv:integration-storage]
deps = -r{toxinidir}/integration/storage/requirements.txt

commands = python -m integration.storage
Copy link
Member

Choose a reason for hiding this comment

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

This is fine for now, although in the future I would prefer to use pytest directly - this way common pytest flags (e.g. -s) and plugins can be used.

Copy link
Member

Choose a reason for hiding this comment

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

EDIT: Sorry, I just see you followed an existing pattern since original compute integration tests code is quite old :)

Copy link
Member

@Kami Kami left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for working on this 👍

(I'll push a small change in the near future to utilize pytest instead of standard unit test test loader for consistency with other test jobs and so we utilize various pytest plugins such as test timing info, etc.)

@codecov-commenter
Copy link

codecov-commenter commented May 1, 2021

Codecov Report

Merging #1572 (2024981) into trunk (cc5987b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##            trunk    #1572   +/-   ##
=======================================
  Coverage   83.00%   83.00%           
=======================================
  Files         394      394           
  Lines       84990    84990           
  Branches     9038     9038           
=======================================
  Hits        70550    70550           
  Misses      11373    11373           
  Partials     3067     3067           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc5987b...2024981. Read the comment docs.

@c-w c-w merged commit 38693a4 into trunk May 2, 2021
@c-w c-w deleted the storage-azure-integration-tests branch May 2, 2021 14:31
d-mo pushed a commit to mistio/libcloud that referenced this pull request Nov 15, 2021
* Fix build

* Namespace compute integration tests

* Add integration tests for Azure Storage driver
d-mo pushed a commit to mistio/libcloud that referenced this pull request Nov 19, 2021
* Fix build

* Namespace compute integration tests

* Add integration tests for Azure Storage driver
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants