Hi Drew,
The changes for 7040795 look good.
Looking at the changes, and looking at similar code
in the CleanupCPIOInstall
checkpoint in the new CUD architecture, I wonder whether
we need to update line 311-312 of
usr/src/lib/install_ict/cleanup_cpio_install.py
so that uuid for all post install publishers are reset instead of
just the highest rank publisher. I know this does not have to be resolved
right now, perhaps we should track it with a bug so we don't forget.
Regarding the changes to 7040909, it looks OK in terms
of being able to fix the issues at hand. Please update the comment
for the function in line 320-324 so it better reflects what's happening.
Now that there's no notion of a preferred publisher, I wonder whether
the code for setting of post install publisher can be simplified.
Previously, we are required to have a preferred publisher. So, the
code for setting the post install publisher is written to first replace the
preferred publisher with the preferred publisher for post-install.
Then, all
additional publishers are deleted and finally add all the additional
post install publishers.
Now that we don't have a preferred publisher, do you know whether
we are required to have at least 1 publisher? If we are not required to
have at least 1 publisher, we can make the code for setting post install
publishers much easier to read and understand by first removing all existing
publishers, then, adding in the new list. Again, I understand
that you must get this fix into the gate asap. So, perhaps you can also
track this investigation in a bug, so it can be addressed in the near
future.
Thanks,
--Karen
On 04/30/11 09:12 PM, Drew Fisher wrote:
Good evening.
I need to get a code review for the following bugs:
7040795 <http://monaco.us.oracle.com/detail.jsf?cr=7040795> install
fails with setting preferred publisher errors after fix for 7039499
<http://monaco.us.oracle.com/detail.jsf?cr=7039499> is integrated
7040909 <http://monaco.us.oracle.com/detail.jsf?cr=7040909> install
set the wrong default publisher after fix for 7039499
<http://monaco.us.oracle.com/detail.jsf?cr=7039499> is integrated for
all installs.
http://cr.opensolaris.org/~drewfish/cr_7040795/
Evan already looked at my previous code review for 7040795. I can
also confirm that Mary tested the ISOs I generated and found the
change to the ICT code works. I'd like another pair of eyes on the
fix, if possible.
For 7040909, we ran into a similar issue with the new IPS API. The
concept of a 'preferred' publisher is gone. The code that handled
these settings needed to be streamlined to handle this removal.
For testing, I used DC and the set-ips-attributes checkpoint to change
the publisher(s) after installation. I manually verified the
publishers (via pkg -R /<DC build dataset>/build_data/pkg_image
publisher) to ensure they were updating correctly. There is no need
to construct and test an entire ISO as the pkg5 CLI allows us to test
the publishers in the resulting dataset very easily.
I tested a very large matrix of publishers, mirrors and multiple
origins to ensure everything was set correctly. All tests pass and
the resulting publishers are set correctly.
I also ran all of the transfer unittests and made sure ips.py was pep8
clean.
-----
Test Matrix:
(each entry has a 'publisher / uri' string)
'solaris' publishers:
ipkg = http://ipkg.us.oracle.com/solaris11/dev
ib = http://indiana-build.us.oracle.com:8000
'install-nightly' publishers:
file = file:///export/home/drew/packages/i386/nightly-nd/repo.redist/
'pkg5-nightly' publishers:
pkg5 = http://ipkg.us.oracle.com/internall/solaris11/pkg/nightly
"install" - publishers set for 'transfer-ips-install' checkpoint
"set" - publishers set for 'set-ips-attributes' checkpoint
any asterisk character meant a mirror
----
install: solaris / ipkg
set: solaris / ipkg
install: solaris / ipkg
set: solaris / ib
install: solaris / ipkg, *i-b
set: solaris / ipkg
install: solaris/ ipkg
set solaris / ipkg, *i-b
install: solaris / ipkg, pkg5-nightly / ipkg, install-nightly / file:///
set solaris / ipkg
install: solaris / ipkg, pkg5-nightly / ipkg, install-nightly / file:///
set solaris / i-b
install: solaris / ipkg, pkg5-nightly / ipkg, install-nightly / file:///
set solaris / i-b, *ipkg
install: install-nightly / file:///, solaris / ipkg
set pkg5-nightly
install: install-nightly / file:///, solaris / ipkg
set solaris / ipkg
install: install-nightly / file:///, solaris / ipkg
set solaris / ib
Whew.
-Drew
_______________________________________________
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
