Adds runtime v2 support for Windows shim's#2495
Conversation
|
@crosbymichael @jhowardmsft - PTAL |
jterry75
force-pushed
the
runtime_v2_windows
branch
from
ce6cbb6 to
5a5b3f8
Compare
|
Not sure why ltag was so angry but removed the headers. Applied via the template and pushed updates. Hopefully will pass the build this time. |
|
I'll take a look. It looks like there are other vet and linting errors |
jterry75
force-pushed
the
runtime_v2_windows
branch
from
5a5b3f8 to
fb665c4
Compare
|
@crosbymichael - I think this was due to a |
jterry75
force-pushed
the
runtime_v2_windows
branch
from
fb665c4 to
beef57f
Compare
Implements the various requirements for the runtime v2 code to abstract away the unix/linux code into the appropriate platform level abstractions to use the runtime v2 on Windows as well. Adds support in the Makefile.windows to actually build the runtime v2 code for Windows by setting a shell environment BUILD_WINDOWS_V2=1 before calling make. (Note this disables the compilation of the Windows runtime v1) Signed-off-by: Justin Terry (VM) <[email protected]>
jterry75
force-pushed
the
runtime_v2_windows
branch
from
beef57f to
d3e0c16
Compare
Signed-off-by: Michael Crosby <[email protected]>
Since windows does not require a signal handler, we just block on the channel forever so that it does not exit. Signed-off-by: Michael Crosby <[email protected]>
Add Windows Publisher and Signal Handler
Codecov Report
@@ Coverage Diff @@
## master #2495 +/- ##
=======================================
Coverage 44.76% 44.76%
=======================================
Files 93 93
Lines 9555 9555
=======================================
Hits 4277 4277
Misses 4585 4585
Partials 693 693
Continue to review full report at Codecov.
|
|
@jterry75 it's green! |
|
LGTM |
|
Nice! Thanks for the help |
estesp
left a comment
estesp
left a comment
There was a problem hiding this comment.
Looks good; I think we can use the _unix.go file ending for the two files currently ending in nix and drop the build directive to make it simpler via Go's natural architecture/os support on filenames.
| @@ -0,0 +1,102 @@ | |||
| // +build !windows | |||
There was a problem hiding this comment.
I think renaming this file shim_unix.go without the build directive would work?
| @@ -0,0 +1,57 @@ | |||
| // +build !windows | |||
There was a problem hiding this comment.
similarly, util_unix.go without the build directive would work here
|
And...ignore me; |
| return err | ||
| } | ||
| logrus.WithField("socket", path).Debug("serving api on unix socket") | ||
| logrus.WithField("socket", path).Debug("serving api on abstract socket") |
There was a problem hiding this comment.
Should be in shim_unix.go
| } | ||
|
|
||
| func handleSignals(logger *logrus.Entry, signals chan os.Signal) error { | ||
| <-signals |
There was a problem hiding this comment.
should at least handle os.Interrupt (i.e. ^C) here so the program exits as expected
There was a problem hiding this comment.
Actually shim's don't exit on signals so we really just need this method to block. They exit on the Shutdown rpc
There was a problem hiding this comment.
But I like my ^C!
But again, the shim isn't supposed to be started manually so I won't push on this.
| } | ||
|
|
||
| // AbstractAddress returns an abstract npipe address | ||
| func AbstractAddress(ctx context.Context, id string) (string, error) { |
There was a problem hiding this comment.
that name doesn't make sense on windows.
Could we rename to just SocketAddress or AddressPath ? Or anything without Abstract in it :)
|
@estesp I think I would still prefer for the consistency of using |
This updates some methods for cross platform use as well as unifying _unix files. Signed-off-by: Michael Crosby <[email protected]>
Abstract to SocketAddress
1. Moves the log message for each socket to the appropriate _unix and _windows.go 2. Replaces all reference to Abstract Socket for Windows. 3. Adds support for ctrl+c on Windows to exit a shim. Signed-off-by: Justin Terry (VM) <[email protected]>
|
LGTM |
1 similar comment
|
LGTM |
6 participants
Implements the various requirements for the runtime v2 code to abstract
away the unix/linux code into the appropriate platform level
abstractions to use the runtime v2 on Windows as well.
Adds support in the Makefile.windows to actually build the runtime v2
code for Windows by setting a shell environment BUILD_WINDOWS_V2=1
before calling make. (Note this disables the compilation of the Windows
runtime v1)
Signed-off-by: Justin Terry (VM) [email protected]