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

Add rudimentary windows support for bcfg2 #391

Open
wants to merge 29 commits into
base: master
Choose a base branch
from

Conversation

chschenk
Copy link

@chschenk chschenk commented Jul 14, 2017

This pull request adds rudimentary windows support to bcfg2. The most changes are in the probe and the locking sections:

-Probes
The shebangline of the probes have to be in the format "#!C:\Path\to\interpreter.exe*.fileending" because windows refuses to run files without an fileending. An example shebangline would be "#!C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe*.ps1"

-Locking
Unfortunately windows has no fcntl so that locking under windows is done by writing the pid into the lockfile

-Tools
For managing windows i have written three Tools: WinAction, WinFS, WinService
-WinAction handles the default Action entrys. Example entry <Action name='test' command="echo test > C:\test.txt" timing="post" when="always" status="check"/>
-WinFS handles Path entrys. Example entrys <Path name='/%ProgramFiles%/test/test.txt'/> or <Path name='/C:/test.txt'/>. Currently the options owner, group and mode are ignored completly.
-WinService handles service entrys and starts, stops or restarts the services via powershell. Example Entry: <Service name='icinga2' type='windows' status="on" />


def FindExtra(self):
"""Locate extra Windows services."""
return []
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it possible to get the list of services, that are scheduled to start automatically? Maybe something like this would work:

Get-WMIObject win32_service -Filter "StartMode = 'auto'" | Format-Table Name -HideTableHeaders

Copy link
Member

Choose a reason for hiding this comment

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

Just found the python wmi module. Maybe we just want to rely on this, instead "parsing" the output of powershell scripts.

Copy link
Author

Choose a reason for hiding this comment

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

I have thought about this too, but decided against it because the WMI module is not included in the standard version of python for windows and, at least in my environment, it is extremely difficult to deploy extra pip packages onto windows servers (especially when I consider updating these packages).

os.unlink(Bcfg2.Options.setup.lockfile)
else:
lockfile = open(file, 'w')
fcntl.lockf(lockfile.fileno(), fcntl.LOCK_UN)
Copy link
Member

@AlexanderS AlexanderS Jul 14, 2017

Choose a reason for hiding this comment

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

I think, this does not work (quote from flock(2) man page):

If a process uses open(2) (or similar) to obtain more than one file descriptor for the same file, these file descriptors are treated independently by flock(). An attempt to lock the file using one of these file descriptors may be denied by a lock that the calling process has already placed via another file descriptor.

I do not know if python will cache the fd from above, but I would not rely on this.

Copy link
Author

Choose a reason for hiding this comment

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

I have implemented an completely new locking mechanism which should solve the problem on windows and on Linux. See class Flock in Utils.py

@AlexanderS
Copy link
Member

Oh wow, thanks.

The errors reported by travis-ci, seems only style issues. That should be fixable. Would you mind to fix the obvious stuff (like line too long)?

I don't like the os.name name checks during the imports. Maybe we can do something like the Compat stuff and externalize this into an external module.

@@ -201,7 +201,8 @@ def run_probe(self, probe):
try:
fileending = ''
if os.name == 'nt':
(interpreter, fileending) = probe.attrib.get('interpreter').split('*')
(interpreter, fileending) =
Copy link
Member

Choose a reason for hiding this comment

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

There is a \ missing at the end of the line.

@chschenk
Copy link
Author

@AlexanderS
What do you think of this? I have added the modules which weren't available under Windows to the Compat module and fixed the style issues

@AlexanderS
Copy link
Member

Wow, that looks great.

@solj What do you think?


def RunAction(self, entry):
"""This method handles command execution and status return."""
shell = True
Copy link
Member

@solj solj Aug 4, 2017

Choose a reason for hiding this comment

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

Should this default to False? It seems to me like this block of code will always result in shell = True

ans = safe_input(prompt)
if ans not in ['y', 'Y']:
return False
if False:
Copy link
Member

Choose a reason for hiding this comment

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

if False: seems wrong here.

@chschenk
Copy link
Author

You are right i have fixed the issues

@odenbach
Copy link

Any chance to get this included? I would love to see this upstream.

@solj
Copy link
Member

solj commented Jul 23, 2018

@chschenk It looks like the exception clauses you're using in Utils.py don't work with < python 2.6. Can you modify those so they don't use the "except Exception as" clause?

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.

4 participants