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
CDRIVER-4454 add integration tests for Azure KMS #1124
Conversation
And regenerate config.yml
.evergreen/config.yml
Outdated
set -o errexit | ||
# TODO: update once https://github.com/mongodb-labs/drivers-evergreen-tools/pull/239 is merged. | ||
git clone https://github.com/kevinAlbs/drivers-evergreen-tools.git --branch DRIVERS-2411 drivers-evergreen-tools | ||
DRIVER_TOOLS=$(pwd)/drivers-evergreen-tools |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DRIVERS_TOOLS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. DRIVERS_TOOLS is consistent with other naming in the config.
.evergreen/config.yml
Outdated
export AZUREKMS_RESOURCEGROUP=${testazurekms_resourcegroup} | ||
export AZUREKMS_VMNAME=${AZUREKMS_VMNAME} | ||
export AZUREKMS_PRIVATEKEYPATH=/tmp/testazurekms_privatekey | ||
DRIVER_TOOLS=$(pwd)/drivers-evergreen-tools |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: If I understand this code correctly, this looks unnecessary if you use DRIVERS_TOOLS env variable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DRIVERS_TOOLS
is not defined before in this task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it
export AZUREKMS_RESOURCEGROUP=${testazurekms_resourcegroup} | ||
export AZUREKMS_VMNAME=${AZUREKMS_VMNAME} | ||
export AZUREKMS_PRIVATEKEYPATH=/tmp/testazurekms_privatekey | ||
DRIVERS_TOOLS=$(pwd)/drivers-evergreen-tools |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NOTE: if you use DRIVERS_TOOLS, this value should already be assigned, at least this is what I saw in the c# driver (same in other places)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed your explanation above. It's a bit strange, but ok :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delayed review. Have some minor change requests.
Summary
Here is an Evergreen patch build with the new tasks: https://spruce.mongodb.com/version/63500fcbd1fe071dfc2682da/
Notes
The additions to the Python evergreen generation scripts are minimal. IMO the Python scripts are not significantly more readable than the Evergreen config YML. For this PR, I found it easier to start with Evergreen YML, then translate to the Python scripts.