Hi Jan,
I took a look at ict.py and install-finish changes from what I think is
the latest webrev (http://cr.opensolaris.org/~dambi/bug-15723-cr), and I
have a few questions/comments:
ict.py:
466-470:
If these are constants, they would be better defined at the class level
or as globals. However, might it be better to pass these in as an
(optional, with the current values as defaults) parameter to __init__?
670-671: Minor nit - the "+" sign is not strictly necessary - the
parentheses will handle the wrapping, and removing the "+" will cause
Python to consider the disparate strings as a single string (as opposed
to concatenating them at run time during each execution, it will
concatenate them during generation of the .pyc file). I'll leave it up
to you as to whether or not this is actually worth bothering changing at
this point, as there's nothing actually "wrong" with it currently.
2461, 2463: Direct use of the dictionary "os.environ" is preferred to
use of "os.putenv"[1]
2468-2469: Again, "+" sign not needed when splitting up a literal string
to wrap a line (Note that it *is* necessary when concatenating a string
literal with a string variable, as on line 2473)
install-finish:
No comments specifically related to your changes; however, Sue asked me
about the change of "pkg_remove_list" to be capitalized. Given the
future of ICTs, it may or may not be worth making this change as part of
your push, but the more appropriate way to handle an executable Python
file like this is:
1) Move the "actual code" (in this case, lines 78 through EOF) into a
"def main()" function
2) Add the following to the end of the file:
if __name__ == '__main__':
main()
This accomplishes two things:
a) If someone, somewhere, attempts to "import install-finish" (a foolish
action, certainly, but accidents happen), these changes mean that none
of the code actually gets executed.
b) Currently, all the variables defined in this file become global
variables (which is why pylint is complaining about the
non-capitalization, which I assume is why pkg_remove_list was updated).
By making the above changes, it's possible to separate out truly global,
static variables from non-global variables (by defining globals outside
of main(), and non-globals inside it).
- Keith
[1] See note under: http://docs.python.org/library/os.html#os.environ
On 06/ 8/10 01:37 AM, Jan Damborsky wrote:
Hi,
could I please ask for reviewing changes enhancing
Automated Installer with support for new SMF based System
Configuration framework [3c] ?
Following bugs cover those changes:
15723 Teach AI to use new SMF based System Configuration framework for
configuring user and root accounts
15410 The installer delivered SMF manifests should be relocated to
/lib/svc/manifest
13737 Automated Installer needs support for setting terminal type from
AI manifest
Webrev:
http://cr.opensolaris.org/~dambi/bug-15723/
Please see below for more details as well as testing accomplished.
Joe offered to review the changes (thanks Joe !) and if possible,
I would like Ethan to take a look at AI portion in order to be
sure AI expectations are met.
As usual, anybody else is of course welcome to take a look as well.
Please provide your comments before COB Monday 6/14.
Thank you very much,
Jan
[1] Summary of changes
* support for new SMF based SC framework in AI (bug 15723)
AI has been modified to handle SC manifest as SMF profile. During ICT
phase,
the profile is syntactically validated and copied to the appropriate
directory
on the target (/a/etc/svc/profile/). It is then processed during
first boot of installed system as part of Early Manifest Import process
(aka EMI) - see [3a] for more details.
The legacy SC parser in AI is still in place and it processes the
existing SC parameters which are not yet transfered to SMF properties
(hostname, timezone). The legacy parser will be removed once those
parameters can be configured via SMF properties.
In order to minimize number of incompatible changes affecting users,
AI can still process SC manifest in legacy format.
The big bang (breaking backward compatibility) is planned to be synced
with the integration of AI/DC manifest rework.
If legacy format is detected, SC manifest is converted to the new format
during the installation. User is informed in log file that legacy SC
manifest
was provided and that the support for this might be removed at any time
without previous notice.
For purposes of the transition, conversion script is delivered into
the AI image. That conversion script can be also used on AI server
side to convert legacy SC manifests to the new format. The script
can handle both standalone SC manifests as well as SC manifest embedded
into AI manifest.
It addresses the common use case when admins have bunch of SC manifests
which are being re-used with every new build coming. Conversion tool
allows to automate the process of transitioning to the new format of
SC manifest.
* new System Configuration SMF service is introduced (bug 15723)
This SMF service takes care of configuring user and root accounts.
See design spec [3b] for more details.
* SMF services residing in install gate now take advantage of EMI (bug
15410)
This changes was needed for correct work of new System Configuration
SMF service. In general, it is required that SMF properties are applied
before 1st SMF service is launched. While there, all SMF services
in slim-source gate were modified.
* NWAM is now configured in SC manifest
As a preparation for static networking configuration in AI, AI was
modified
to enable/disable NWAM via SC manifest - related code was removed from
ICT.
[2] Testing accomplished
* Following types of images were built (based on build 140):
- x86: AI, text installer, GUI
- Sparc: AI, text installer
Installations with built images were tested to guarantee no regressions
were introduced.
* Several scenarios for configuring root&user account were tested -
see following document for list of test cases:
http://cr.opensolaris.org/~dambi/tc_user_root/tc_user_root.txt
* It was verified that old AI image (legacy SC parser) fails
appropriately
when provided with new SC manifests - it fails with following messages
in /tmp/install_log:
...
<AI Jun 8 09:25:19> Parsing system configuration manifest
<AI_E Jun 8 09:25:19> Could not parse property from SC manifest
<AI Jun 8 09:25:19> Automated Installation failed in parser module
<AI Jun 8 09:25:19> Invalid System Configuration manifest provided
* It was verified that new AI image can handle both new as well as
legacy SC manifests.
* It was verified that conversion tool can convert both standalone
as well as embedded legacy SC manifest into new format
* It was verified that installadm(1M) can process new SC manifest -
in particular that 'installadm add' has not been broken by the changes.
[3] References
[3a]
http://hub.opensolaris.org/bin/download/Community+Group+smf/smf_design_docs/emidesignmar09.html
[3b]
http://hub.opensolaris.org/bin/download/Project+caiman/System+Configuration+Project/scsmfdesignv0.1.pdf
[3c]
http://hub.opensolaris.org/bin/view/Project+caiman/System+Configuration+Project
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
