On Mar 23, 2009, at 4:26 AM, joel r wrote: > > On Sun, Mar 22, 2009 at 2:33 AM, Luke Kanies <[email protected]> wrote: >> >> On Mar 21, 2009, at 11:59 AM, joel r wrote: >> >>> >>> >>> On Sat, Mar 21, 2009 at 1:40 PM, Paul Nasrat >>> <[email protected]> wrote: >>> But you really should target master for new features like this. >>> >>> Sure. I've just rebased against master. >> >> Is this code online? I don't see it in your git repo. > > It's online now.
Thanks. The branch should probably be renamed - it still has 0.24.7 in the name. There are still a couple of things you can do to make the code less error-prone on non-Windows platforms (that is, for testing, at least). The :useradd_win provider has the confine, but it still has the exception raised for non-windows. The util/windows_system module could use a feature 'windows' to load the win32 libraries. See lib/puppet/features.rb for examples; you could put the feature in lib/puppet/features/windows.rb and Puppet would autoload it when asked for it. It also looks like you're either using full tabs or 8 space tabs; could you switch to four-space tabs? That should be covered, I think, in the developer lifecycle page. > >>> Also rather than a monolithic patch could you post individual >>> patches, see >>> >>> http://reductivelabs.com/trac/puppet/wiki/DevelopmentLifecycle >>> >>> Sorry, I really should have read this document before submitting my >>> patch. >>> >>> >>>> The code is up at http://github.com/finalprefix/puppet/tree/0.24.7-win >>> . >>>> >>>> I'd love any feedback you might have! >>> >>> How are you finding running it on windows, there are a bunch of path >>> issues I know that need to be cleaned up to run puppet. >>> >>> Well Facter tries to run domainname and dnsdomainname, which don't >>> exist on windows. I'll probably patch Facter in a bit. >>> >>> Further, I've been focused on the providers, and I've been running >>> them using the standalone puppet command. At the moment, puppetd >>> doesn't work because fork is not implemented in ruby on Windows. The >>> win32-process gem has an implementation of fork for windows. Perhaps >>> it will solve the problem. >> >> Which of the existing types and providers do you expect to eventually >> work on Windows? > > I'm looking at adding support for windows ACLs to files, and doing > what I can to support packages on Windows. Windows doesn't have > package managers quite like apt / yum. But I might be able to get > something done to support msi. > > Additionally, I'm looking at support for the registry, and basic > directory service support. I think these would be new providers. That all sounds great. I expect with those done, plus your initial user and group support, we'd have the most important bits of Windows covered. > >> I expect that most of those that use forking wouldn't work on Windows >> anyway, but which of them that use fork would you want to use? > > None of these providers would need forking. I guess I was trying to > get puppetd to work with as few changes as possible, which was why I > tried to get fork and pipes to work. More on that below. Ok. > >>> Daemonization on puppetd will also require puppet to be a windows >>> service. Again, there's a gem for this called win32-service. >>> >>> I also need to figure out how to get non inheritable pipes for lines >>> 78 and 79 of lib\puppet\external\event-loop\event-loop.rb: >>> # prevent file descriptor leaks >>> @notify_src.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC) >>> @notify_snk.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC) >>> >>> I attempt to run puppet using "puppet test.pp", and the fcntl calls >>> throws: >>> >>> c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet >>> \external\event-loop\event-loop.rb:78:in `initialize': uninitialized >>> constant Fcntl::F_SETFD (NameError) >>> c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet >>> \external\event-loop\event-loop.rb:31:in `new' >>> c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet >>> \external\event-loop\event-loop.rb:31:in `default' >>> c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet >>> \external\event-loop\event-loop.rb:35:in `current' >>> c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet >>> \external\event-loop\event-loop.rb:285:in `initialize' >>> c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet\util >>> \settings.rb:532:in `new' >>> c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet\util >>> \settings.rb:532:in `set_filetimeout_timer' >>> c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet\util >>> \settings.rb:341:in `parse' >>> c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\lib\puppet >>> \application.rb:215:in `run' >>> c:\ruby-1.8.7\lib\ruby\gems\1.8\gems\puppet-0.24.7\bin\puppet:68 >>> c:\ruby-1.8.7\bin\puppet:19:in `load' >>> c:\ruby-1.8.7\bin\puppet:19 >>> >>> I tried commenting out those two lines, and things worked fine. >> >> Nice - we've got problems going both directions. Unixes inherit too >> many, and Windows can't inherit them at all? > >> It should be straightforward to block these out for Windows boxen. > > Daemonizing puppet on windows would not involve forking. We would make > puppet run as a windows service, probably using win32-service. If we > did, there should be no process invocations to inherit any pipes, and > so we could just skip these lines on Windows configurations. > Daemonizing would probably need some restructuring/additions in the > daemonizing code, since windows use callbacks to signal > start/stop/pause to a service. > > I believe it is possible for a spawned process to inherit pipes on > Windows by passing the right flags. As I've mentioned earlier, IO.pipe > does not by default use the O_NOINHERIT flag, so the resulting pipe > should be inheritable. Whereas windows doesn't fork, there's an > implementation of fork in win32-process that tries to fake it as > closely as possible. However, the child process it spawned was not > able to write back to the parent using the write handle provided by > IO.pipe. I need to explore this a bit more. Ok. I'm pretty open to doing whatever needs to be done, within reason. > >>> From what I read so far, using O_NOINHERIT on windows should be the >>> equivalent of FD_CLOEXEC. However, IO.pipe makes the call to _pipe >>> without this flag. I think it should be possible to make the call >>> with O_NOINHERIT using the Win32API gem. >>> >>> >>> If I get a chance this week I'll try and produce a combined tree >>> cherry-picking your stuff against master. >>> >>> Further comments inline >>> >>>> diff --git a/lib/puppet.rb b/lib/puppet.rb >>>> index 4b0091c..77dbdfe 100644 >>>> --- a/lib/puppet.rb >>>> +++ b/lib/puppet.rb >>>> @@ -190,53 +190,57 @@ module Puppet >>>> # Trap a couple of the main signals. This should probably be >>>> handled >>>> + begin >>>> + [:INT, :TERM].each do |signal| >>>> + trap(signal) do >>>> + Puppet.notice "Caught #{signal}; shutting >>> down" >>>> + Puppet.debug "Signal caught here:" >>>> + caller.each { |l| Puppet.debug l } >>>> + Puppet.shutdown >>>> + end >>>> + end >>> ... >>>> + rescue ArgumentError => e >>>> + raise if e.to_s.index('SIGHUP') == nil >>>> end >>>> end >>> >>> What's the issue you're fixing here, I'm assuming its due to SIGHUP >>> not being defined on windows. There is probably a more explicit >>> way of >>> handling this, as it's such a long method it makes it hard to see >>> the >>> intent. >>> >>> This part isn't necessary anymore, since the settrap call has gone >>> from puppet.rb. >> >> It's still used, in lib/puppet/daemon.rb, but only if you run as a >> daemon. >> >> Again, great work, and thanks. >> >> -- >> Trying to determine what is going on in the world by reading >> newspapers is like trying to tell the time by watching the second >> hand of a clock. --Ben Hecht >> --------------------------------------------------------------------- >> Luke Kanies | http://reductivelabs.com | http://madstop.com >> >> >>> >> > > > -- It has recently been discovered that research causes cancer in labratory rats. --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
