zpool mirror and raidz virtual devices

94 views
Skip to first unread message

Josh Cooper

unread,
Nov 11, 2014, 2:06:51 AM11/11/14
to puppe...@googlegroups.com
There looks to be a bug in the current zpool provider, where if you have a manifest like:

    mirror => ["disk1 disk2", "disk3 disk4"],

Puppet will execute:

    zpool create data mirror "disk1 disk2" "disk3 disk4"

Instead of:

    zpool create data mirror disk1 disk2 mirror disk3 disk4

Adam put together a PR at https://github.com/Incognito1992/puppet/compare/ticket/master/PUP-3388-zpool-mirrors, but while researching it, I think this is really a regression due to http://projects.puppetlabs.com/issues/16157, commit https://github.com/puppetlabs/puppet/commit/4a6853e1de083842362dc30d0842bbbb9b029f64, first introduced in Puppet 3.0.

The Oracle docs[1] seem to support that Puppet is now doing the wrong thing:

Virtual devices are specified one at a time on the command line, separated by whitespace. The keywords “mirror” and “raidz” are used to distinguish where a group ends and another begins. For example, the following creates two root vdevs, each a mirror of two disks:

   # zpool create mypool mirror c0t0d0 c0t1d0 mirror c1t0d0 c1t1d0

This seems pretty straightforward, but I wanted to raise the issue here, because I am not a Solaris expert, and it's been broken for awhile without anyone noticing... I've submitted a PR at https://github.com/puppetlabs/puppet/pull/3299, comments welcome.

Josh

Spencer Krum

unread,
Nov 11, 2014, 3:27:29 AM11/11/14
to puppe...@googlegroups.com
I'm as big a ZFS fanboy as anyone, but I've never used Puppet to manage a zpool, and I never will. If this has been broken for a long time, then my situation is not atypical.

Maybe this is a good time to split the zpool stuff out of Puppet core and into a module?

I can see instances where I would use puppet to manage zfs filesystems, fwiw.





--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/f7237160-0528-47ee-8ba3-335fbd64ed9a%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.



--
Spencer Krum
(619)-980-7820

Joshua Hoblitt

unread,
Nov 11, 2014, 8:09:04 AM11/11/14
to puppe...@googlegroups.com
On 11/11/2014 12:06 AM, Josh Cooper wrote:
Adam put together a PR at https://github.com/Incognito1992/puppet/compare/ticket/master/PUP-3388-zpool-mirrors, but while researching it, I think this is really a regression due to http://projects.puppetlabs.com/issues/16157, commit https://github.com/puppetlabs/puppet/commit/4a6853e1de083842362dc30d0842bbbb9b029f64, first introduced in Puppet 3.0.

The type has been broken since 3.0?  Can we consider that a "deprecation" cycle and remove it from core? :)

-Josh

--

Josh Cooper

unread,
Nov 11, 2014, 6:13:55 PM11/11/14
to puppe...@googlegroups.com
On Tue, Nov 11, 2014 at 5:09 AM, Joshua Hoblitt <jhob...@cpan.org> wrote:
On 11/11/2014 12:06 AM, Josh Cooper wrote:
Adam put together a PR at https://github.com/Incognito1992/puppet/compare/ticket/master/PUP-3388-zpool-mirrors, but while researching it, I think this is really a regression due to http://projects.puppetlabs.com/issues/16157, commit https://github.com/puppetlabs/puppet/commit/4a6853e1de083842362dc30d0842bbbb9b029f64, first introduced in Puppet 3.0.

 The type has been broken since 3.0?

Managing zpool raidz and mirror virtual devices has been broken since 3.0. I don't know how critical those are to managing zpools in general. So it could be that it works for the 80% case?

  Can we consider that a "deprecation" cycle and remove it from core? :)

I'm +1 for removing it, filed as PUP-3661.


-Josh

--

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--
Josh Cooper
Developer, Puppet Labs

Geoffrey Gardella

unread,
Nov 13, 2014, 1:35:21 PM11/13/14
to puppe...@googlegroups.com
Hi Josh!
I am with Oracle and have just been assigned to work on Puppet in Solaris. I am just getting oriented, but this looks like something we should be working on. Is this functionality being covered elsewhere? If not, I can open an internal ticket to get this covered.

So far, we aren't even pushing our changes back upstream, so fixing that that is my first order of business.

Cheers,
Geoffrey

Spencer Krum

unread,
Nov 13, 2014, 4:30:20 PM11/13/14
to puppe...@googlegroups.com
Josh, What version of solaris were you testing this on? Its possible that the zpool command has changed behavior more recently than Puppet 3.0, thus making it look like this has been broken that long.

Furthermore I'm not sure if the different zfs implementations all use the exact same command structure at this point.

With Geoffery stepping up to take ownership of this stuff, and some consensus around spitting out the zfs stuff, I think we should split out the zfs/zpool types and providers into puppet-community/puppet-module-zfs and give Geoffery commit access.

We should do this with at least a ping to the FreeBSD folks in case they want to get involved as well.

Thanks,
Spencer

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/7dd557f6-cac1-4d99-a9c5-89dfc6158b82%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.



--
Spencer Krum
(619)-980-7820

Josh Cooper

unread,
Nov 14, 2014, 1:52:51 AM11/14/14
to puppe...@googlegroups.com
On Thu, Nov 13, 2014 at 10:35 AM, Geoffrey Gardella <gard...@gmail.com> wrote:
Hi Josh!
I am with Oracle and have just been assigned to work on Puppet in Solaris. I am just getting oriented, but this looks like something we should be working on.

Welcome Geoffrey, that's great news!

Is this functionality being covered elsewhere? If not, I can open an internal ticket to get this covered.

We merged a fix for https://tickets.puppetlabs.com/browse/PUP-3388 recently, and I verified it worked as expected on Solaris 11.1. In the immediate term, could you take a look and confirm?

zpool { tstpool:
  ensure => present,
  mirror => ['/ztstpool/dsk1 /ztstpool/dsk2', '/ztstpool/dsk3 /ztstpool/dsk4']
}

root@yscuew2s4qea3eu:~/puppet# bundle exec puppet apply zpool.pp --debug
...
Debug: Executing '/usr/sbin/zpool create tstpool mirror /ztstpool/dsk1 /ztstpool/dsk2 mirror /ztstpool/dsk3 /ztstpool/dsk4'
Notice: /Stage[main]/Main/Zpool[tstpool]/ensure: created
...
root@yscuew2s4qea3eu:~/puppet# zpool status -v tstpool
  pool: tstpool
 state: ONLINE
  scan: none requested
config:

        NAME                STATE     READ WRITE CKSUM
        tstpool             ONLINE       0     0     0
          mirror-0          ONLINE       0     0     0
            /ztstpool/dsk1  ONLINE       0     0     0
            /ztstpool/dsk2  ONLINE       0     0     0
          mirror-1          ONLINE       0     0     0
            /ztstpool/dsk3  ONLINE       0     0     0
            /ztstpool/dsk4  ONLINE       0     0     0


So far, we aren't even pushing our changes back upstream, so fixing that that is my first order of business.

It would be great to get those fixes merged upstream. You can find us on #puppet-dev if you need assistance with the PR process. Also, every Weds from 10-12 PST, we hold a PR triage on google hangouts. It would be great to catch up at next week's PR triage and talk about next steps.

Also the following query roughly captures the current set of open Solaris issues:


It would be great if you could chime in on those tickets, and see if your changes address some of them.

We are also looking at moving non-core types and providers out of puppet, and giving community subject matter experts commit access. We are actively working on removing nagios, with the goal of establishing a pattern so that other non-core types and providers can be moved in a similar way. I imagine zpool could follow soon after.

 


Cheers,
Geoffrey


On Monday, November 10, 2014 11:06:51 PM UTC-8, Josh Cooper wrote:
There looks to be a bug in the current zpool provider, where if you have a manifest like:

    mirror => ["disk1 disk2", "disk3 disk4"],

Puppet will execute:

    zpool create data mirror "disk1 disk2" "disk3 disk4"

Instead of:

    zpool create data mirror disk1 disk2 mirror disk3 disk4

Adam put together a PR at https://github.com/Incognito1992/puppet/compare/ticket/master/PUP-3388-zpool-mirrors, but while researching it, I think this is really a regression due to http://projects.puppetlabs.com/issues/16157, commit https://github.com/puppetlabs/puppet/commit/4a6853e1de083842362dc30d0842bbbb9b029f64, first introduced in Puppet 3.0.

The Oracle docs[1] seem to support that Puppet is now doing the wrong thing:

Virtual devices are specified one at a time on the command line, separated by whitespace. The keywords “mirror” and “raidz” are used to distinguish where a group ends and another begins. For example, the following creates two root vdevs, each a mirror of two disks:

   # zpool create mypool mirror c0t0d0 c0t1d0 mirror c1t0d0 c1t1d0

This seems pretty straightforward, but I wanted to raise the issue here, because I am not a Solaris expert, and it's been broken for awhile without anyone noticing... I've submitted a PR at https://github.com/puppetlabs/puppet/pull/3299, comments welcome.

Josh

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+...@googlegroups.com.



--
Josh Cooper
Developer, Puppet Labs

Join us at PuppetConf 2015, October 5-9 in Portland, OR - http://2015.puppetconf.com.  
Register early to save 40%!

Josh Cooper

unread,
Nov 14, 2014, 1:53:34 AM11/14/14
to puppe...@googlegroups.com
On Thu, Nov 13, 2014 at 1:30 PM, Spencer Krum <krum.s...@gmail.com> wrote:
Josh, What version of solaris were you testing this on?

Solaris 11.1
 
Its possible that the zpool command has changed behavior more recently than Puppet 3.0, thus making it look like this has been broken that long.

Yep, that's a good point, though I am fairly certain the old code assumed all devices were part of the same mirror. For example:

zpool { tstpool:
  ensure => present,
  mirror => ['/ztstpool/dsk1', '/ztstpool/dsk2', '/ztstpool/dsk3', '/ztstpool/dsk4']
}

Would result in one mirror with 4 virtual devices:

Debug: Executing '/usr/sbin/zpool create tstpool mirror /ztstpool/dsk1 /ztstpool/dsk2 /ztstpool/dsk3 /ztstpool/dsk4'
Notice: /Stage[main]/Main/Zpool[tstpool]/ensure: created
...
root@yscuew2s4qea3eu:~/puppet# zpool status -v tstpool
  pool: tstpool
 state: ONLINE
  scan: none requested
config:

        NAME                STATE     READ WRITE CKSUM
        tstpool             ONLINE       0     0     0
          mirror-0          ONLINE       0     0     0
            /ztstpool/dsk1  ONLINE       0     0     0
            /ztstpool/dsk2  ONLINE       0     0     0
            /ztstpool/dsk3  ONLINE       0     0     0
            /ztstpool/dsk4  ONLINE       0     0     0

And trying to specify multiple mirrors, each containing a set of virtual devices:

zpool { tstpool:
  ensure => present,
  mirror => ['/ztstpool/dsk1 /ztstpool/dsk2', '/ztstpool/dsk3 /ztstpool/dsk4']
}

Would outright fail:

Debug: Executing '/usr/sbin/zpool create tstpool mirror /ztstpool/dsk1 /ztstpool/dsk2 /ztstpool/dsk3 /ztstpool/dsk4'
Error: Execution of '/usr/sbin/zpool create tstpool mirror /ztstpool/dsk1 /ztstpool/dsk2 /ztstpool/dsk3 /ztstpool/dsk4' returned 1: Unable to build pool from specified devices: cannot open '/ztstpool/dsk1 /ztstpool/dsk2': No such file or directory

Furthermore I'm not sure if the different zfs implementations all use the exact same command structure at this point.

With Geoffery stepping up to take ownership of this stuff, and some consensus around spitting out the zfs stuff, I think we should split out the zfs/zpool types and providers into puppet-community/puppet-module-zfs and give Geoffery commit access.

+1


We should do this with at least a ping to the FreeBSD folks in case they want to get involved as well.

Makes sense.


For more options, visit https://groups.google.com/d/optout.



--

Geoffrey Gardella

unread,
Nov 18, 2014, 3:27:28 PM11/18/14
to puppe...@googlegroups.com
Hi Josh,
I will plan to attend the PR triage on Wed.

On Thu, Nov 13, 2014 at 11:52 PM, Josh Cooper <jo...@puppetlabs.com> wrote:


On Thu, Nov 13, 2014 at 10:35 AM, Geoffrey Gardella <gard...@gmail.com> wrote:
Hi Josh!
I am with Oracle and have just been assigned to work on Puppet in Solaris. I am just getting oriented, but this looks like something we should be working on.

Welcome Geoffrey, that's great news!

Is this functionality being covered elsewhere? If not, I can open an internal ticket to get this covered.

We merged a fix for https://tickets.puppetlabs.com/browse/PUP-3388 recently, and I verified it worked as expected on Solaris 11.1. In the immediate term, could you take a look and confirm?
 
So, it looks like the fix was ded0852. I will have a look, but am still coming up to speed. Certainly the debug output below looks correct.


zpool { tstpool:
  ensure => present,
  mirror => ['/ztstpool/dsk1 /ztstpool/dsk2', '/ztstpool/dsk3 /ztstpool/dsk4']
}

root@yscuew2s4qea3eu:~/puppet# bundle exec puppet apply zpool.pp --debug
...
Debug: Executing '/usr/sbin/zpool create tstpool mirror /ztstpool/dsk1 /ztstpool/dsk2 mirror /ztstpool/dsk3 /ztstpool/dsk4'
Notice: /Stage[main]/Main/Zpool[tstpool]/ensure: created
...
root@yscuew2s4qea3eu:~/puppet# zpool status -v tstpool
  pool: tstpool
 state: ONLINE
  scan: none requested
config:

        NAME                STATE     READ WRITE CKSUM
        tstpool             ONLINE       0     0     0
          mirror-0          ONLINE       0     0     0
            /ztstpool/dsk1  ONLINE       0     0     0
            /ztstpool/dsk2  ONLINE       0     0     0
          mirror-1          ONLINE       0     0     0
            /ztstpool/dsk3  ONLINE       0     0     0
            /ztstpool/dsk4  ONLINE       0     0     0


So far, we aren't even pushing our changes back upstream, so fixing that that is my first order of business.

It would be great to get those fixes merged upstream. You can find us on #puppet-dev if you need assistance with the PR process. Also, every Weds from 10-12 PST, we hold a PR triage on google hangouts. It would be great to catch up at next week's PR triage and talk about next steps.
 
Sounds good. I'm sure I will need help as I learn the ropes here.


Also the following query roughly captures the current set of open Solaris issues:


It would be great if you could chime in on those tickets, and see if your changes address some of them.

We are also looking at moving non-core types and providers out of puppet, and giving community subject matter experts commit access. We are actively working on removing nagios, with the goal of establishing a pattern so that other non-core types and providers can be moved in a similar way. I imagine zpool could follow soon after.

This makes sense. My main concern here will be making sure that the testing/push process for our code is in place, so that we are prepared to take that on.
 

--
You received this message because you are subscribed to a topic in the Google Groups "Puppet Developers" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/puppet-dev/kLSiWghNrvY/unsubscribe.
To unsubscribe from this group and all its topics, send an email to puppet-dev+...@googlegroups.com.
 To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/CA%2Bu97umK_GEokBor_tDyrO4jG_Tv%3DnriQwD_2TYM16vuc_vxoA%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

Cheers,
Geoffrey
Reply all
Reply to author
Forward
0 new messages