[Ovirt-devel] [PATCH server] Add the installer files as a subpackage of the server package

[Joey Boggs]

Some comments/questions inline

Bryan Kearney wrote:
> I will push the change below. Joey... can you pick up these comments 
> from David?
>
> -- bk
>
> David Lutterkort wrote:
>> On Tue, 2009-01-20 at 16:49 -0500, Bryan Kearney wrote:
>>> This is resend of the three earlier patches. I have combined them 
>>> into a single patch, and addressed all the comments with the 
>>> following excpetions:
>>>
>>> - Passwords are stills stored on the file system. I will add a patch 
>>> ont this.
>>> - Changelog was not updtaed, since that seems to get done at release 
>>> time.
>>
>> ACK .. please address the comments below though, either with a revamped
>> patch or follow up patches.
>>
>>> diff --git a/installer/bin/ovirt-installer 
>>> b/installer/bin/ovirt-installer
>>> new file mode 100755
>>> index 0000000..65dc522
>>> --- /dev/null
>>> +++ b/installer/bin/ovirt-installer
>> ...
>>> +if File.exist?("/usr/sbin/sestatus")
>>> +    sestatus = `/usr/sbin/sestatus`
>>> +    if sestatus !~ /(Current mode:                   
>>> permissive|Current mode:                   disabled|SELinux 
>>> status:                 disabled|SELinux status:                 
>>> permissive)/
>>> +        puts "SELinux enabled, please disable or set in permissive 
>>> mode permanently by editing"
>>> +        puts "/etc/selinux/config and rebooting"
>>> +        exit
>>> +    end
>>> +end
>>
selinuxdisabled wasn't around in F9 :) makes it easier now in F10
>> This is harder than it has to be: why not just run selinuxenabled and
>> getenforce to see if we are in enforcing mode ? We could also offer to
>> put the system into permissive mode if it is enforcing.
>>
>>> +# DNS Configuration
>>> + at cli.say( "\nThe following DNS servers were found:")
>>> +File.open('/etc/resolv.conf').each_line{ |line|
>>> +  line = line.chomp
>>> +    puts line if line =~ /nameserver/ and line !~ /nameserver 
>>> 127.0.0.1/
>>> +}
>>> +dns_servers = prompt_yes_no("Use this systems's dns servers?")
>>
>> Why suppress localhost as a nameserver here ? It should be fine to run
>> your own dnsmasq on localhsot.
Didn't want to add any confusion, but I see your point
>>
>>> +mgmt_ip = `ifconfig #{mgmt_dev}`
>>> +mgmt_ipaddr= mgmt_ip.scan(/\s*inet addr:([\d.]+)/)
>>> +prov_ip = `ifconfig #{prov_dev}`
>>> +prov_ipaddr= prov_ip.scan(/\s*inet addr:([\d.]+)/)
>>> +
>>> +if dns_servers == "y"
>>> +    host_lookup = Socket.getaddrinfo(ipa_host,nil)
>>> +    hostip = host_lookup[1][3]
>>> +    if hostip.to_s != mgmt_ipaddr.to_s
>>> +        @cli.say("Reverse dns lookup for #{ipa_host} failed, exiting")
>>
>> That's a forward DNS lookup you're doing - but you should also check
>> that looking up mgmt_ipaddr gets you ipa_host.
I can switch this around to do reverse, just didnt catch it when I wrote 
it. Or should both foward and reverse be there?
>>
>>> +        exit
>>> +    end
>>> +end
>>> +
>>> +# DHCP Configuration
>>> +dhcp_setup = prompt_yes_no("Does your provisioning network already 
>>> have dhcp?")
>>> +if dhcp_setup == "n"
>>> +    dhcp_interface = prov_dev
>>> +    dhcp_network = prompt_for_answer("Enter the first 3 octets of 
>>> the dhcp network you wish to use (example: 192.168.50):", :regex => 
>>> THREE_OCTETS)
>>> +    dhcp_start = prompt_for_answer("Enter the dhcp pool start 
>>> address (example: 3):", :regex => OCTET)
>>> +    dhcp_stop = prompt_for_answer("Enter the dhcp pool end addess 
>>> (example: 100):", :regex => OCTET)
>>
>> Strictly speaking, this doesn't have to be on a /24 network; maybe just
>> ask for full IP addresses ?
>>
start/stop would be easy to calculate but wouldn't we need to determine 
the subnets as well or are we assuming 1 subnet?

>>> +    dhcp_domain = prompt_for_answer("Enter the dhcp domain you wish 
>>> to use (example: example.com):", :regex => IP_OR_FQDN)
>>
>> Default to dnsdomainname ? (and use that for other places where we ask
>> for a domain)
I don't think we can trust dnsdomainname that's why it's explicity asked 
for and hardcoded during the installation?

>>
>>> +freeipa_password = prompt_for_answer("NOTE: The following pasword 
>>> will also be you ovirtadmin password for the web management login\n\
>>
>> s/you/your/
>>
>>> +# Generate the file and output it.
>>> +FileUtils.mkdir_p("/usr/share/ace/appliances/ovirt")
>>> +config_file = File.new("/usr/share/ace/appliances/ovirt/ovirt.pp", 
>>> "w")
>>> +config_file.write(ERB.new(template, 0, "%>").result)
>>> +config_file.close()
>>
>> Why is the file written to /usr/share ? It should go into /var/lib, and
>> ideally would be configurable (so that I can run the installer as an
>> ordinary user)
>>
>>
The ace installer is looking for it in that spot, would a symlink from 
/usr/share/ace/appliances/ovirt/ovirt.pp to var/lib/ovirt/ovirt.pp 
suffice in this case?

>>> diff --git a/installer/modules/ovirt/manifests/dns.pp 
>>> b/installer/modules/ovirt/manifests/dns.pp
>>> new file mode 100644
>>> index 0000000..5326c7c
>>> --- /dev/null
>>> +++ b/installer/modules/ovirt/manifests/dns.pp
>>
>>> +define dns::remote($mgmt_ipaddr="", 
>>> $prov_ipaddr="",$mgmt_dev="",$prov_dev="") {
>>> +
>>> +#    On the pxe server you will need to ensure that the
>>> +#    next server option points to the ip address of the tftp server
>>> +
>>> +# The following SRV records must be present in the dns server for 
>>> everything
>>> +# to function properly. Replace example.com with the appropriate 
>>> domain
>>> +
>>> +#    _ovirt._tcp.example.com.    SRV 0 5 80 
>>> ovirtwuiserver.example.com.
>>> +#    _ipa._tcp.example.com.      SRV 0 5 80 ipaserver.example.com.
>>> +#    _ldap._tcp.example.com.     SRV 0 5 389 ldapserver.example.com.
>>> +#    _collectd._tcp.example.com. SRV 0 5 25826 
>>> ovirtwuiserver.example.com.
>>> +#    _qpidd._tcp.example.com.    SRV 0 5 5672 
>>> ovirtwuiserver.example.com.
>>> +#    _identify._tcp.example.com. SRV 0 5 12120 
>>> ovirtwuiserver.example.com.
>>> +
>>> +# Also A records must be present for each oVirt node. Without this 
>>> they are unable
>>> +# to determine their hostname and locate the management server.
>>
>> These comments shouldn't be here - they should go into a README that
>> tells people how to use the installer and what infrastructure they need
>> to have.
>>
>> Nobody will find these instructions buried in a puppet manifest.
>>
>> David
>>
>>