-
Notifications
You must be signed in to change notification settings - Fork 352
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 Ansible role for Windows DCAP CI testing #1417
Add Ansible role for Windows DCAP CI testing #1417
Conversation
This PR is currently blocked by #1375. After that gets merged, I'll rebase the PR and we'll have only the Ansible Windows DCAP changes in the pull request diff. Labeled this as |
@CodeMonkeyLeet If you want to review these changes already, you can only take a look at the commit 21b970e since this is the entire codebase related to the this pull request. |
d2c12d5
to
21b970e
Compare
@CodeMonkeyLeet, regarding the PR changes, I have 2 notes that I'd like to clarify with you:
Regards, |
21b970e
to
feed14b
Compare
Quick update: The PR contains now only the Win DCAP CI Ansible changes. |
ba9fdb1
to
28682cd
Compare
448a832
to
99cb43c
Compare
|
||
--- | ||
psw_archive_url: 'http://registrationcenter-download.intel.com/akdlm/irc_nas/15018/Intel%20SGX%20PSW%20for%20Windows%20v2.2.100.48339.exe' | ||
dcap_archive_url: 'http://registrationcenter-download.intel.com/akdlm/irc_nas/15017/Intel%20SGX%20DCAP%20for%20Windows%20v1.0.100.48134.exe' |
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.
Intel has since pushed out a newer version of the package at: http://registrationcenter-download.intel.com/akdlm/irc_nas/15025/Intel%20SGX%20DCAP%20for%20Windows%20v1.0.101.48697.exe
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.
I'll update this.
@ionutbalutoiu, see replies below:
I don't think you need to explicitly specify it if you're using
Yes, that looks like what I had in mind. If you look at the prototype for how we would like to consume that in the windows/dcap_support branch, the desire is that the hosts/CMakeLists.txt would not have to hardcode a version into the path reference for getting to the nuget header and libraries. |
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.
Generally looks good, but I'm not very familiar with Ansible. @johnkord might have to review that more closely.
@ionutbalutoiu, can you also check that using the updated Intel DCAP package works?
One additional note: the current scripts/ansible/README.md file doesn't mention any support for running playbooks on Windows. I assume it is supported, and if so, can we also update the instructions for how to run the playbook on Windows? |
I noticed that the new DCAP package has only the Do you know if the lack of |
I cannot find any of these drivers listed under Software devices. I'm still debugging to see what's wrong with this.
Awesome! Ansible installs everything to Let me know if this location is good, or if I need to change it. |
Yes. Windows is supported and tested as a targeted Ansible remote machine. I haven't tried to run Ansible from Windows directly though. I only tried running Ansible from a Linux machine targeting both Linux and Windows remote machines. I'll try running from Windows directly and come back with my findings. |
Hey @CodeMonkeyLeet, It turns out that Ansible on Windows doesn't work right away. I tried to install the pip package and hit an issue described here. People recommend using the Windows Linux Subsystem and have Ansible there. I think this is a good workaround. What do you think this ? I could add documentation about this. |
Yep. It looks like the installation method via Installing the drivers via
|
99cb43c
to
b67313b
Compare
@ionutbalutoiu Could you modify the Windows maintainer document to include instructions on how to use this? I think this file would probably be appropriate: https://github.com/Microsoft/openenclave/blob/master/docs/GettingStartedDocs/GettingStarted.Windows.md Perhaps we need to rename our documents again, so it's clear which documents are for maintainers and which are for SDK users (application developers)! It's unfortunate that we either need WSL or to run this ansible playbook remotely to use ansible on Windows, but I don't think that's a complete blocker for maintainers to get a proper environment. I think it would be a major annoyance for SDK users (app devs), though. When we plan to support DCAP on Windows in a release, I imagine we'll want to include some of this logic in these ansible tasks for ACC deployments so that SDK users will be able to function without needing to run an ansible deployment. That's a work item I'll create now and we'll address when the time comes. |
These changes address all the environment requirements needed to do Azure-DCAP-Client CI testing on Windows. This is a new set of Ansible tasks bundled into the file `windows/az-dcap-client/tasks/environment-setup.yml`. All the requirements are greatly documented in the issue openenclave#1320.
b67313b
to
25abc73
Compare
I'd create a new doc under I wouldn't include the Ansible usage steps in What do you think about having a separate doc for Ansible ? |
@ionutbalutoiu That sounds fine to me, and if we want, we can refactor/reorganize the docs later when we plan to support DCAP on Windows! As long as the documentation exists somewhere easy to get to, I'm happy (for now) :) |
This is needed to connect to the target Windows machines
* Update the main README.md from the root Ansible directory * Update the README.md from the inventory Ansible directory
I added to the pull request the documentation on how to use Ansible (the If you want to execute the code from this pull request, do the following:
There's a single remaining item on this pull request. @CodeMonkeyLeet suggested using the new Intel DCAP installer (see conversation from here). I replied on this, but probably got lost in the whole GitHub conversation, please see my above comment. |
|
Please, let me know if Intel reaches back with any response. If you really wish to run Ansible under Windows releases without Linux subsystem support, there might be the following options:
I tried only the first method, and I successfully run the Ansible tasks from this pull request, from a Windows 2016 box against my remote testing Windows 2016 ACC VM. Digging further on this topic, it looks like Ansible won't work at all natively on Windows. So we have to use the Unix code under Windows in some way or another. If you prefer any of the above methods, I could document these as well. |
@CodeMonkeyLeet The system I set up for the VS team didn't have any DCAP support, so it was very trivial to set up! I simply deployed a new Windows ACC VM and then installed the appropriate visual studio tools and clang-7, and everything else was already configured as part of the provision script that an ACC deployment runs. |
@ionutbalutoiu I think that any requirement for Windows devs to have to spin up a custom Linux environment just to run Ansible so they can set up a Windows ACC dev box is going to be a poor experience. Honestly, it's faster for me to just read a set of instructions like the ones I gave you than jump through the hoops of setting up Ansible, and that's a red flag. However, that is a distinct problem from what this PR addresses, so I've spun up #1499 to track one proposed alternative for managing the dev setup experience. For the purposes of this PR, it doesn't sound like we have remaining issues and should be able to proceed? @johnkord, any additional blockers? |
@CodeMonkeyLeet Completely agree with you, Simon. Sounds good, let's proceed, and then address the enclave developer experience in the future. I like the plan to use docker for the development experience, and it hopefully can also help get us to a "reproducible build" scenario |
bors r+ |
👎 Rejected by too few approved reviews |
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.
bors r+
1417: Add Ansible role for Windows DCAP CI testing r=johnkord a=ionutbalutoiu These changes address all the environment requirements needed to do Azure-DCAP-Client CI testing on Windows. This is a new set of Ansible tasks bundled into the file `windows/az-dcap-client/tasks/environment-setup.yml`. All the requirements are greatly documented in the issue #1320. Fixes #1320 Co-authored-by: Ionut Balutoiu <ibalutoiu@cloudbasesolutions.com>
Build succeeded |
These changes address all the environment requirements
needed to do Azure-DCAP-Client CI testing on Windows.
This is a new set of Ansible tasks bundled into the file
windows/az-dcap-client/tasks/environment-setup.yml
.All the requirements are greatly documented in the issue #1320.
Fixes #1320