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

[Bryan Kearney]

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
> 
> 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.
> 
>> +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.
> 
>> +        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 ?
> 
>> +    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)
> 
>> +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)
> 
> 
>> 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
> 
>