Hi Clay,

I have a few comments:

create_client.py:

202, 206, 211 - I think it might be more consistent if perhaps the first line of setup_dhcp was "arch = arch.lower()", and the comparisons checked against the lower case value. Then, the issue of case sensitivity is moot, regardless of whether Sparc, SPARC, or sparc is passed in. (All unlikely, but since /usr/lib/installadm/setup-dhcp appears to require lower-case anyway, this might be smoother).


installadm_common:

Please put the successful run_cmd doctest first (Normal usage scenario before the error cases).

1134, 1154: The fact that these lines are needed implies we have an issue with how we define the "_" function in our modules (which is to say, we don't). This is likely a larger localization issue, but I believe importable modules are not supposed to rely on gettext.install() having been called at some point.

This is certainly a larger issue needing a separate bug.

In any case, would a single "gettext.install()" call be sufficient? (Or even a slightly more cryptic one-liner such as "_ = lambda x: x"?)

1144-1147 - As the doctest shows up in documentation, I think it's best to limit output to the minimum necessary to make the point. With that in mind, simply dumping test['err'] (and possibly test['out']) here would reduce the "noise" level. (Similar thoughts can be applied to the other examples as well - this also could have the side effect of minimizing or eliminating the need for ELLIPSIS and similar statements).

Thanks,
Keith

On 04/13/10 12:21 PM, [email protected] wrote:
Hello,
    This webrev is my proposed fix to the following installadm bugs:
7481    Remove use of site-specific option, GrubMenu, from AI setup
15531    installadm build_136 creates bad macro for X86 service
15593    create-client: needs to quote dhtadm macro strings again

In testing, I found the following and propose to fix it too, supplying doctests to document and test the run_cmd() API: 15588 installadm_common: run_cmd() should raise OSError if non-existent
    command is attempted

Webrev:
http://cr.opensolaris.org/~clayb/15531/

I have manually run the following tests to verify functionality. My baseline machine was running build 132 and my fix, test machine was running build 135 with the fixes applied.

                            Thank you,
                            Clay

SPARC Create Service (fixed):
-----------------------------
r...@jumprope:~# installadm create-service -n install_test_ai_sparc1 -s /tmp/clay_sparc_test_20100326-1512.iso /var/install_test_ai_sparc1 Warning: NWAM is enabled. Please be sure that the IP address for jumprope is static.
Setting up the target image at /var/install_test_ai_sparc1 ...
Registering the service install_test_ai_sparc1._OSInstall._tcp.local

Detected that DHCP is not set up on this server.
If not already configured, please create a DHCP macro
named dhcp_macro_install_test_ai_sparc1 with:
   Boot server IP (BootSrvA) : 172.20.24.78
Boot file (BootFile) : http://172.20.24.78:5555/cgi-bin/wanboot-cgi
If you are running the Solaris DHCP Server, use the following
command to add the DHCP macro, dhcp_macro_install_test_ai_sparc1:
/usr/sbin/dhtadm -g -A -m dhcp_macro_install_test_ai_sparc1 -d :BootSrvA=172.20.24.78:BootFile=\"http://172.20.24.78:5555/cgi-bin/wanboot-cgi\":

Note: Be sure to assign client IP address(es) if needed
(e.g., if running the Solaris DHCP Server, run pntadm(1M)).
Service discovery fallback mechanism set up
Creating SPARC configuration file
Service install_test_ai_sparc is currently being used by SPARC clients which have not explicitly been associated with another service via the 'create-client' subcommand. To select service install_test_ai_sparc1 for those SPARC clients, use the following commands:
/usr/bin/rm -f /etc/netboot/wanboot.conf
/usr/bin/ln -s install_test_ai_sparc1/wanboot.conf /etc/netboot

SPARC Create Service (baseline):
--------------------------------
Setting up the target image at /var/install_test_ai_sparc1 ...
Registering the service install_test_ai_sparc1._OSInstall._tcp.local

Detected that DHCP is not set up on this server.
If not already configured, please create a DHCP macro
named dhcp_macro_install_test_ai_sparc1 with:
   Boot server IP (BootSrvA) : 10.6.68.21
   Boot file      (BootFile) : http://10.6.68.21:5555/cgi-bin/wanboot-cgi
If you are running the Solaris DHCP Server, use the following
command to add the DHCP macro, dhcp_macro_install_test_ai_sparc1:
/usr/sbin/dhtadm -g -A -m dhcp_macro_install_test_ai_sparc1 -d :BootSrvA=10.6.68.21:BootFile=\"http://10.6.68.21:5555/cgi-bin/wanboot-cgi\":

Note: Be sure to assign client IP address(es) if needed
(e.g., if running the Solaris DHCP Server, run pntadm(1M)).
Service discovery fallback mechanism set up
Creating SPARC configuration file
Service osol-dev-128a-ai-sparc is currently being used by SPARC clients which have not explicitly been associated with another service via the 'create-client' subcommand. To select service install_test_ai_sparc1 for those SPARC clients, use the following commands:
/usr/bin/rm -f /etc/netboot/wanboot.conf
/usr/bin/ln -s install_test_ai_sparc1/wanboot.conf /etc/netboot

SPARC Create Service Diffs (Normalized IP addresses):
-----------------------------------------------------
1d0
< Warning: NWAM is enabled. Please be sure that the IP address for jumprope is static.
18c17
< Service install_test_ai_sparc is currently being used by SPARC clients which have not explicitly been associated with another service via the 'create-client' subcommand.
---
Service osol-dev-128a-ai-sparc is currently being used by SPARC clients
which have not explicitly been associated with another service via the 'create-client' subcommand.

SPARC Create Client (fixed):
----------------------------
r...@jumprope:~# pfexec installadm create-client -e c0ffeec0ffee -n
install_test_ai_sparc1
Creating SPARC configuration file


Detected that DHCP is not set up on this server.
If not already configured, please create a DHCP macro
named 01C0FFEEC0FFEE with:
   Boot server IP (BootSrvA) : 172.20.24.78
Boot file (BootFile) : http://172.20.24.78:5555/cgi-bin/wanboot-cgi
If you are running the Solaris DHCP Server, use the following
command to add the DHCP macro, 01C0FFEEC0FFEE:
/usr/sbin/dhtadm -g -A -m 01C0FFEEC0FFEE -d :BootSrvA=172.20.24.78:BootFile=\"http://172.20.24.78:5555/cgi-bin/wanb
oot-cgi\":

Note: Be sure to assign client IP address(es) if needed
(e.g., if running the Solaris DHCP Server, run pntadm(1M)).


SPARC Create Client (baseline):
-------------------------------
Setting up SPARC client...
Creating SPARC configuration file

Detected that DHCP is not set up on this server.
If not already configured, please create a DHCP macro
named 01C0FFEEC0FFEE with:
   Boot server IP (BootSrvA) : 10.6.68.21
   Boot file      (BootFile) : http://10.6.68.21:5555/cgi-bin/wanboot-cgi
If you are running the Solaris DHCP Server, use the following
command to add the DHCP macro, 01C0FFEEC0FFEE:
/usr/sbin/dhtadm -g -A -m 01C0FFEEC0FFEE -d :BootSrvA=10.6.68.21:BootFile=\"http://10.6.68.21:5555/cgi-bin/wanboot-cgi\":

Note: Be sure to assign client IP address(es) if needed
(e.g., if running the Solaris DHCP Server, run pntadm(1M)).

SPARC Create Client Diffs (Normalized IP addresses):
----------------------------------------------------
0a1
Setting up SPARC client...
3d3
<
7c7
<    Boot server IP (BootSrvA) : X.X.X.X
---
   Boot server IP (BootSrvA) : X.X.X.X

X86 Create Service (fixed):
---------------------------
Warning: NWAM is enabled. Please be sure that the IP address for jumprope is static.
Setting up the target image at /var/ai/install_test_ai_x86 ...
Registering the service install_test_ai_x86._OSInstall._tcp.local

Detected that DHCP is not set up on this server.
If not already configured, please create a DHCP macro
named dhcp_macro_install_test_ai_x86 with:
   Boot server IP (BootSrvA) : 172.20.24.78
   Boot file      (BootFile) : install_test_ai_x86
If you are running the Solaris DHCP Server, use the following
command to add the DHCP macro, dhcp_macro_install_test_ai_x86:
/usr/sbin/dhtadm -g -A -m dhcp_macro_install_test_ai_x86 -d :BootSrvA=172.20.24.78:BootFile=install_test_ai_x86:

Note: Be sure to assign client IP address(es) if needed
(e.g., if running the Solaris DHCP Server, run pntadm(1M)).
Service discovery fallback mechanism set up
Service discovery fallback mechanism set up

X86 Create Service (baseline):
------------------------------
Setting up the target image at /var/ai/install_test_ai_x86 ...
Registering the service install_test_ai_x86._OSInstall._tcp.local

Detected that DHCP is not set up on this server.
If not already configured, please create a DHCP macro
named dhcp_macro_install_test_ai_x86 with:
   Boot server IP (BootSrvA) : 10.6.68.21
   Boot file      (BootFile) : install_test_ai_x86
   GRUB Menu      (GrubMenu) : menu.lst.install_test_ai_x86
If you are running the Solaris DHCP Server, use the following
command to add the DHCP macro, dhcp_macro_install_test_ai_x86:
/usr/sbin/dhtadm -g -A -m dhcp_macro_install_test_ai_x86 -d :BootSrvA=10.6.68.21:BootFile=install_test_ai_x86:GrubMenu=menu.lst.install_test_ai_x86:

Additionally, if the site specific symbol GrubMenu
is not present, please add it as follows:
   /usr/sbin/dhtadm -g -A -s GrubMenu -d Site,150,ASCII,1,0

Note: Be sure to assign client IP address(es) if needed
(e.g., if running the Solaris DHCP Server, run pntadm(1M)).
Service discovery fallback mechanism set up
Service discovery fallback mechanism set up

X86 Create Service Diffs (Normalized IP addresses):
---------------------------------------------------
1,2d0
< Warning: NWAM is enabled. Please be sure that the IP address for jumprope
< is static.
10a9
   GRUB Menu      (GrubMenu) : menu.lst.install_test_ai_x86
13,14c12,17
<    /usr/sbin/dhtadm -g -A -m dhcp_macro_install_test_ai_x86 -d
< :BootSrvA=X.X.X.X:BootFile=install_test_ai_x86:
---
   /usr/sbin/dhtadm -g -A -m dhcp_macro_install_test_ai_x86 -d

:BootSrvA=X.X.X.X:BootFile=install_test_ai_x86:GrubMenu=menu.lst.install_test_ai_x86:

Additionally, if the site specific symbol GrubMenu
is not present, please add it as follows:
   /usr/sbin/dhtadm -g -A -s GrubMenu -d Site,150,ASCII,1,0

_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to