Skip to main content

[pkg-discuss] Re: code review request: pkg dehydrate/rehydrate

  • From: Yiteng Zhang < >
  • To: Bart Smaalders < >, Erik Trauschke < >
  • Cc:
  • Subject: [pkg-discuss] Re: code review request: pkg dehydrate/rehydrate
  • Date: Mon, 09 Jun 2014 14:03:49 -0700

On 06/ 5/14 01:18 PM, Bart Smaalders wrote:
On 05/29/14 16:23, Yiteng Zhang wrote:
Hi folks,

Can I ask you to review on this?

Some notes:
1) pkg dehydrate/rehydrate go through the api planning code path. After
discussion
in the meeting and some experiments, instead of using manifest
comparison to get
the files and hardlinks that are needed to remove in dehydrate or to add in
rehydrate, I generate these actions to a dict and pass it explicitly to
pkg plans to
propose an repair. Details in imageplan.py.

2) Confirmed with Jesse that pkg dehydrate/rehydrate only needs to work on
alternate root.

3) The last change of pkg man pages was pushed to s11.2 only but not to
s12,
which is weird. So there are a lot of change in src/man/pkg.1.

4) Test suite of pkg dehydrate and pkg rehydrate are combined in one
file, client
test in src/tests/cli/t_pkg_hydrate.py, and api test in
src/tests/api/t_pkg_api_hydrate.py.
Test case might not be sufficient, let me know your idea.

5) Manual test on full, partial and user image.
For example, it takes about 44s to dehydrate an user image only
installed with
package core-os. The size of the image goes from 1.2GB to 308MB. And it
takes
about 3m20s to rehydrate the image.

6) Fix "bug 15521086 pkg fix should be moved into the api".

7) Probably I need to file a bug about this project.

webrev: https://ips.java.net/webrev/yitezhan/pkg_hydrate/

Please let me know your opinion.

Thanks,
Yiteng

This looks like a good start to me.

How does it perform on an image containing solaris-large-server, both in
time taken and space saved?

Some areas of concern:

In __gen_hydrate_actions, you hard-code in exceptions for etc/zones/* files.  
This would be
much better done by adding 'dehydrate=false' to those files in their own 
manifests. That
way if we find other files that need to be retained, we can add the same 
attribute - or
other developers of those packages can.  This would need to be done in ON 
-but it is
file-specific metadata, and belongs in the packages.

In __gen_hydrate_actions, you call get_manifest with ignore_excludes=True.  
Why?

It is possible to add a publisher and install packages from it _after_ 
dehydration.
I think this will break your use of 'all' in the dehydrated_pubs list; you're 
probably better
off not using 'all' but just dealing with the list of publishers.

Pkg operations performed after dehydration may malfunction in interesting 
ways.
Do we need to prevent this or test it?  What happens if I add a package from
the solaris publisher after doing dehydration? What happens if I rehydrate 
after
doing that? It may just work, but we either need to test it or prevent users
from doing silly things.

I am going to implement the idea to prevent silly pkg operations like this:
If the pkg dehydrate command succeeds, a property named dehydrated with the
names of the publishers specified as the value is set on the image. I will change
the behavior of pkg install/uninstall/update/change-facet/change-variant to first check
if the pkg operations are going to modify the package content delivered by the dehydrated
publishers. If so, we will warn the users and do nothing. If not, we do what the pkg
operations were supposed to do.

For example, if users dehydrated on publisher 'solaris', then we will prevent users installing
any package from the 'solaris' publisher, but we won't prevent users installing packages
from other publishers.

At this point, I find it might be a little tricky to implement the idea because we couldn't
simply check the publishers at the very beginning but need to filter the fmri lists by the
publishers. But I believe we can reach there.

Any suggestion or objection?

The pkg fix stuff needs some thought given about how we want to output to 
user.
Should we be using the progress tracker?

As Erik said, I will use the display_plan for license and its progress tracker for other output.

Thanks,
Yiteng

- Bart






[pkg-discuss] Re: code review request: pkg dehydrate/rehydrate

Bart Smaalders 06/05/2014

[pkg-discuss] Re: code review request: pkg dehydrate/rehydrate

Yiteng Zhang 06/05/2014

[pkg-discuss] Re: code review request: pkg dehydrate/rehydrate

Yiteng Zhang 06/09/2014

[pkg-discuss] Re: code review request: pkg dehydrate/rehydrate

Yiteng Zhang 06/13/2014

[pkg-discuss] Re: code review request: pkg dehydrate/rehydrate

Yiteng Zhang 06/19/2014

[pkg-discuss] Re: code review request: pkg dehydrate/rehydrate

Danek Duvall 06/23/2014

[pkg-discuss] Re: code review request: pkg dehydrate/rehydrate

Yiteng Zhang 06/24/2014

[pkg-discuss] Re: code review request: pkg dehydrate/rehydrate

Shawn Walker 06/26/2014

[pkg-discuss] Re: code review request: pkg dehydrate/rehydrate

Yiteng Zhang 06/26/2014

[pkg-discuss] Re: code review request: pkg dehydrate/rehydrate

Shawn Walker 06/26/2014

[pkg-discuss] Re: code review request: pkg dehydrate/rehydrate

Danek Duvall 06/26/2014

[pkg-discuss] Re: code review request: pkg dehydrate/rehydrate

Shawn Walker 06/26/2014

[pkg-discuss] Re: code review request: pkg dehydrate/rehydrate

Danek Duvall 06/26/2014
 
 
Close
loading
Please Confirm
Close