Switch to Github actions #124
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
realloc may fail, which is why we need to avoid directly assigning the reallocated pointer with the "realloc" return value. Instead, we must free the original buffer if realloc fails. Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>
We're reusing parts of the original nbd header, which includes enums. Visual Studio will warn us, suggesting scoped enum classes to be used instead. We'll stick with enums for now and disable this warning. We might actually expose this as a C library at some point. Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>
As requested by the driver verifier, we'll add a few CDB pointer checks. Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>
We're currently using AppVeyor for a minimal WNBD test job that is simply building the driver. Switching to Github Actions allows us to install the driver and actually invoke the functional tests. We can also use longer timeouts, which means we can enable the static analysis test. We'll run NBD tests as well, using qemu-nbd to spin up a NBD server on the fly. One thing to note is that we'll no longer publish debug builds. Those were published by the Appveyor job, however Github Action artifacts are removed after 30 days. It doesn't actually make sense to release unsigned debug builds, especially considering that the Ceph MSI installers from cloudbase.it bundle the latest WNBD driver, signed by Cloudbase (and soon by Microsoft). Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>
One of the WNBD tests performs IO operations and then checks the IO counters. Those counters are incremented after the IO operations return, which is why those checks are performed in a loop using the "EVENTUALLY" helper. The issue is that while the check is performed in a loop, the stats aren't refreshed. This is hacky but can work when directly accessing the WNBD_DISK counters, however this specific test will fetch a copy of those counters using the libwnbd functions. We'll refresh the counters inside the EVENTUALLY loop. Since the first argument of the EVENTUALLY macro is checked using an "if" statement, we'll just pass the stats refresh call as an init statement separated by ";": EVENTUALLY(init-statement; condition, retry_attempts, retry_intv) The test was also attempting to cause an invalid request in order to check that the according counter got incremented. However, it was modifying a flag (flush support) that didn't affect libwnbd's copy of the connection settings. We'll go ahead and remove this check for now. Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>
We're using gtest, which isn't properly understood by the MS static code analysis tools. For example, it complains that we may be accessing a null pointer after an explicit gtest assertion. Besides, static analysis for functional tests isn't as important as validating other compontents such as libwnbd or the driver. For now, we'll disable the static analysis for the functional tests. Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>
SDV fails with a cryptic error while running as part of the GH
action:
smvcl.log: c1 : fatal error C1250: Unable to load plug-in '\x22dcA»'.
[D:\a\wnbd\wnbd\vstudio\driver.vcxproj]
We'll make it optional for now. Also, we're dropping redundant
actions from the post-build analsis step.
Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>
We've moved to Github Actions so we'll proceed with dropping the Appveyor job and updating the docs accordingly. We'll no longer publish debug artifacts. Users may fetch the Ceph bundle instead, which includes a WNBD build signed by Cloudbase: https://cloudbase.it/ceph-for-windows/ Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We're switching from Appveyor to Github Actions for the following reasons:
While at it, we're fixing flaky IO stats tests and we're improving the static analysis job. We'll also address a few issues brought up by the static check.