Skip to content
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

Win supervisor #2278

Merged
merged 11 commits into from
May 25, 2020
Merged

Win supervisor #2278

merged 11 commits into from
May 25, 2020

Conversation

anjmao
Copy link
Contributor

@anjmao anjmao commented May 21, 2020

  1. Add Windows supervisor service.
  2. Add Windows WireGuard tunnel. (Will need some changes in separate PR)
  3. Move common network utils to separate package to reuse for all platforms.
  4. Refactor myst process kill to be cross platform.
  5. No need to pass myst user for myst process startup as it now will be started from the app.

supervisor/daemon/myst.go Show resolved Hide resolved
supervisor/daemon/myst.go Outdated Show resolved Hide resolved
supervisor/daemon/myst.go Outdated Show resolved Hide resolved
supervisor/daemon/myst.go Show resolved Hide resolved
if err != nil {
return fmt.Errorf("could not kill process %d: %w", proc.Pid, err)
}
state, err := proc.Wait()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we probably should not wait forever here..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no way to provide context or timeout to proc.Wait, kill is running in separate goroutine so it will not block the whole supervisor communaction.

supervisor/install/install_windows.go Show resolved Hide resolved
supervisor/install/install_windows.go Show resolved Hide resolved
supervisor/install/install_windows.go Show resolved Hide resolved
supervisor/install/install_windows.go Outdated Show resolved Hide resolved
supervisor/install/install_windows.go Show resolved Hide resolved
@anjmao anjmao changed the title Win supervisor WIP: Win supervisor May 22, 2020
@anjmao anjmao force-pushed the win-supervisor branch 2 times, most recently from 6d7542a to b655d93 Compare May 22, 2020 14:10
@anjmao anjmao requested a review from zolia May 22, 2020 14:14
@anjmao anjmao changed the title WIP: Win supervisor Win supervisor May 22, 2020
@codecov-commenter
Copy link

codecov-commenter commented May 22, 2020

Codecov Report

Merging #2278 into master will decrease coverage by 0.04%.
The diff coverage is 14.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2278      +/-   ##
==========================================
- Coverage   47.93%   47.88%   -0.05%     
==========================================
  Files         273      275       +2     
  Lines       12770    12790      +20     
==========================================
+ Hits         6121     6125       +4     
- Misses       6062     6077      +15     
- Partials      587      588       +1     
Impacted Files Coverage Δ
utils/netutil/network.go 0.00% <0.00%> (ø)
utils/netutil/network_linux.go 0.00% <ø> (ø)
core/connection/manager.go 81.94% <100.00%> (+0.04%) ⬆️
core/discovery/discovery.go 64.75% <0.00%> (-0.82%) ⬇️
nat/traversal/pinger.go 74.27% <0.00%> (ø)
tequilapi/endpoints/sse_handler.go 72.17% <0.00%> (ø)
p2p/channel.go 76.62% <0.00%> (+1.53%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f1819f...b655d93. Read the comment docs.

}

func (c Config) valid() bool {
return c.MystPath != "" && c.OpenVPNPath != ""
return true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like not needed method?

@anjmao anjmao merged commit 45b6d5b into master May 25, 2020
@anjmao anjmao deleted the win-supervisor branch May 25, 2020 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants