Skip to content

Conversation

@lstipakov
Copy link
Member

No description provided.

@lstipakov lstipakov requested a review from flichtenheld March 10, 2025 10:11
@lstipakov lstipakov changed the title Split into multiple files Moving to interactive service Mar 10, 2025
@lstipakov lstipakov requested review from d12fk and selvanair March 11, 2025 08:40
@lstipakov lstipakov self-assigned this Mar 11, 2025
Copy link
Collaborator

@selvanair selvanair left a comment

Choose a reason for hiding this comment

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

Its a bit hard to review this with all the refactoring going on. Would it be possible to split this with the refactoring and cleanup moved to a future PR? I thought the last commit would pretty much covers all that is needed for the iservice-based startup to work.

@lstipakov
Copy link
Member Author

Its a bit hard to review this with all the refactoring going on. Would it be possible to split this with the refactoring and cleanup moved to a future PR? I thought the last commit would pretty much covers all that is needed for the iservice-based startup to work.

But my idea was exactly this - there are few commits for refactoring without any functional changes (like splitting functionality into different files) and this last commit is switching to iservice.

@lstipakov lstipakov force-pushed the iservice branch 3 times, most recently from 8f34d3e to 7fd4ceb Compare March 12, 2025 10:33
@lstipakov lstipakov requested a review from selvanair March 12, 2025 10:39
@lstipakov
Copy link
Member Author

Here you can get an installer with necessary changes to all components. At least here it works fine :)

@d12fk
Copy link
Collaborator

d12fk commented Mar 13, 2025

I think this service should start the interactive service, if it is not running already.

@lstipakov
Copy link
Member Author

lstipakov commented Mar 14, 2025

I think this service should start the interactive service, if it is not running already.

It is now depends on interactive service https://github.com/OpenVPN/openvpn-build/pull/849/files so Windows should start iservice first.

@lstipakov
Copy link
Member Author

Ping @selvanair @d12fk

Copy link
Collaborator

@d12fk d12fk left a comment

Choose a reason for hiding this comment

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

Think this could generally benefit from connecting to the management interface, just like the GUI does. Instead of polling, I mean.

@d12fk
Copy link
Collaborator

d12fk commented Apr 10, 2025

Doxygen would be nice, but a bit more excessive commit messages, describing the things done in the commit, are missing often.

No functional changes.

GitHub: OpenVPN#19

Signed-off-by: Lev Stipakov <[email protected]>
Signed-off-by: Lev Stipakov <[email protected]>
Move OpenVPN service logic into a separate
class which doesn't have any dependencies on
Windows service infrastructure.

Add a log delegate which is used to write
logs either to console or event log.

Add ability to run service logic
as a standalone app.

GitHub: OpenVPN#21

Signed-off-by: Lev Stipakov <[email protected]>
Signed-off-by: Lev Stipakov <[email protected]>
Instead of starting openvpn process directly:

 - connect to interactive service pipe
 - send startup info
 - read openvpn process pid (for polling)

Since we use virtual service account, we don't have
privileges to subscribe for process events, so instead
we use async polling to check if process is alive and
restart it of it is not.

Removed Stop() since due to lack of privileges
the only way now we can stop the process is to
signal the event.

GitHub: OpenVPN#23

Signed-off-by: Lev Stipakov <[email protected]>
Priority class cannot be set by the
virtual service account so remove the code.

Signed-off-by: Lev Stipakov <[email protected]>
The service is installed via MSI Wix script.

GitHub: OpenVPN#24

Signed-off-by: Lev Stipakov <[email protected]>
Instead of having version number in multiple places,
keep it in one place and generate AssemblyInfo.cs on the fly.

While on it, add git commit hash to product version
and bump version to 2.0.0.0.

GitHub: OpenVPN#25

Signed-off-by: Lev Stipakov <[email protected]>
Mono version coming with Ubuntu 22.04 is an outdated
and have bugs like inability to handle escaped
semicolons (https://github.com/mono/mono/pull/1580/files).

Bump to Mono 6.12 and switch to MSBuild (since xbuild is outdated).

Signed-off-by: Lev Stipakov <[email protected]>
@lstipakov
Copy link
Member Author

Doxygen would be nice, but a bit more excessive commit messages, describing the things done in the commit, are missing often.

Made some commit messages more descriptive and added some XML comments - this is a standard way to document code in C# and is natively supported by Visual Studio (unlike Doxygen). Also, Doxygen is able to parse them.

@lstipakov lstipakov merged commit 368caee into OpenVPN:master Apr 15, 2025
4 checks passed
@lstipakov lstipakov deleted the iservice branch April 15, 2025 08:06
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.

3 participants