Refactor driver to include Windows support (includes new transport for all supported platforms) #340
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
|
@jeffreycoe This looks like a great effort. I have also been looking into this recently and as you have probablly seen, InSpec's Train docker transport doesn't understand how to detect Docker on Windows. As you probably found out, the docker-api gem used needs a bit of modernization in order to use the right Docker API version too when on Windows. I have also been playing with this in a separate standalone driver that offers a 'dockercli' transport (name required by kitchen-inspec I believe). Feel free to rescue any of the bits out of there you need. The driver with all my bits and monkeypatched DockerAPI and Train transport is at: https://github.com/stuartpreston/kitchen-dockerwin/ |
This was referenced Jun 14, 2019
|
@stuartpreston Appreciate the feedback. I’ll take a look at merging the code you referenced to see if we can get these updates working with InSpec. Thanks! |
|
I have tested this PR in the following environment: OS: Windows 2019 Core Server I had to install the Ruby Development Kit when installing the ruby gem from source code in order to compile the dependencies that require low level libraries.
kitchen.yml output It created the container and destroys it but seems that is not executing my test yet. Will add a few tests and get back. |
|
I did another test using the https://gist.github.com/lmayorga1980/8c3922f6133d16dfc9025938bd2b3049 |
|
@lmayorga1980 Thanks for testing and glad to see the |
|
I see, I guess change beed to through kitchen-inspec first since its crashing on the Transport piece |
|
Using the last commit of the PR. ---
driver:
name: docker
provisioner:
name: shell
verifier:
name: inspec
platforms:
- name: windows
driver_config:
image: mcr.microsoft.com/windows/servercore:ltsc2019
platform: windows
transport:
name: docker
env_variables:
TEST_KEY: TEST_VALUE
suites:
- name: default
verifier:
inspec_tests:
- test/integration/default/default_spec.rb
attributes:stacktrace |
jeffreycoe
changed the title
|
@stuartpreston @lmayorga1980 These commits include a bunch of monkey patches to get InSpec working with Windows. (Thanks @stuartpreston for the code). So far, it seems to be working. I haven't encountered any issues. Looking for some feedback and some additional testing with these latest commits. Thanks. |
|
Also, I was testing with an older version of the ChefDK (v3.10.1), but I found there have been updates to the Train gem (starting in version 2.1.0) which help with detecting the Windows platform. I upgraded to the latest Chef Workstation 0.4.1 and it has been working with no issues. |
|
Platform: Windows 2019 Core Ruby: ruby 2.6.3p62 (2019-04-16 revision 67580) [x64-mingw32] Results: https://gist.github.com/lmayorga1980/436d1a9011d6681267ee6ccc3410404a I am not using ChefDK and using just the gems coming from |
|
@lmayorga1980 Forgot to mention it doesn't support named pipes yet, so by default the transport has to be configured to use a TCP socket so the InSpec verifier works. Can you configure the Docker engine to listen on a TCP port and try again? I updated the readme to include this info now. You'll need to update the Example configuration is shown below: Then update your .kitchen.yml file to include the socket option in the transport section: |
|
It worked 👍 🎉 🎈 I added a repo to reproduce the behavior when future changes are introduced. https://github.com/LM3CORP/test-kitchen-docker-windows/tree/master/win2019core |
4 tasks
|
Please Merge this. Looks good. |
Signed-off-by: Jeffrey Coe <[email protected]>
Signed-off-by: Jeffrey Coe <[email protected]>
|
@lmayorga1980 I made some minor updates to organize things a little better for the overridden classes to support the InSpec verifier on Windows. CI tests have been updated for Windows, too. As long as the tests complete successfully, and no one has any issues with the code, we should be able to merge now. |
jeffreycoe
changed the title
Signed-off-by: Jeffrey Coe <[email protected]>
|
@jeffreycoe 👍 I will be running some specific tests this week but everything in the PR looks good. Thanks for including this desired feature... |
|
@lmayorga1980 Thanks for all of your help with testing. Looks like my testing completed successfully, and the CI tests passed with the last commit. This PR is ready to be merged. |
|
@jeffreycoe no problem. Let me know if there is something I could do. |
stuartpreston
approved these changes
Jul 2, 2019
|
@stuartpreston Thanks for approving. Would we be able to merge the code at this point? Let us know if you need anything else from us. Thanks. |
|
@stuartpreston Can you merge this PR? |
|
Happy to merge the PR but we need someone with the relevant org access to push the gem, etc. I'll see if I can find someone with the correct permissions. |
|
Much appreciated 👍 |
|
@stuartpreston Thank you! |
|
@stuartpreston Would you mind creating a new release with this code included? |
|
Hi @yeoldegrove - @stuartpreston isn't able to release the gem, but only merge code into this repo. I've reached out to someone who is working on getting the right individuals involved who can release this gem. It should be available soon, hopefully. |
yeoldegrove
added a commit
to yeoldegrove/kitchen-docker
that referenced
this pull request
Oct 7, 2019
|
Hi @jeffreycoe ! Thank you for the rework. Unfortunately when destroying the instances, however, the docker images are automatically removed, but then it produces an exception: This image ID does not exist at all in my image list! anyone is facing the same issue? Kr, Rshad |
11 tasks
|
Hi @rshad - Could you open a new issue with the steps to reproduce? Are you using the code from the master branch? I have been running this code on macOS, Linux, and Windows machines and have not encountered this error yet. |
|
Hi @jeffreycoe ! Yes, I first took the master branch, to solve the issue commented in #338, but I got an error when creating the container for each Kitchen instance, which was reported here #355 and solved here #356. So I then took the Fork branch of #356 and got such error. Kr, Rshad |
|
Hi @jeffreycoe ! I created an issue #360 and the corresponding PR with the fix #361. Kr, Rshad |
This was referenced Jun 22, 2020
Closed
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.
This pull request adds Windows support to the Kitchen driver. It also includes a new transport which supports all supported Linux platforms and Windows. We decided to add a new transport to leverage a lot of the same code the driver uses in the same code base so we can continue to build support for both Windows and Linux in one location within this gem.
I’m requesting feedback (and a code review) from the community on this approach and some help with additional testing.
This update fixes issues #259, #318, #338