Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(MODULES-8823) Add support for overlay option in zfs on Linux #22

Merged
merged 2 commits into from
Oct 31, 2019

Conversation

GabrielNagy
Copy link
Contributor

Fixed up @kali-hernandez's commit by only adding this parameter if we're running on Linux.

Adds back #20

kali-hernandez and others added 2 commits October 31, 2019 14:40
This option allows mounting over non empty directories. The current
master of zfs_core simply doesn't implement such option on the zfs
resource. This change allows for specification of such option in
puppet modules:

zfs { 'my_pool/my_volume':
    ensure => present,
    mountpoint => '/mnt/non_empty_dir',
    overlay => 'on'
}
@GabrielNagy GabrielNagy requested review from mihaibuzgau and a team October 31, 2019 12:59
@GabrielNagy
Copy link
Contributor Author

image

@mihaibuzgau mihaibuzgau merged commit 7ca0db2 into master Oct 31, 2019
@GabrielNagy GabrielNagy added maintenance Maintenance chores are typically excluded from changelogs feature and removed feature maintenance Maintenance chores are typically excluded from changelogs labels Oct 31, 2019
newproperty(:overlay) do
desc 'The overlay property. Valid values are `on`, `off`.'
end
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is unfortunately executed on the server during compilation and the agent during catalog application. So this unfortunately means you can only set this property if the puppetserver is also running on Linux. I think you'd be better off always defining the overlay property, and then add a validate method within the overlay property to fail if the agent kernel is not Linux:

newproperty(:overlay) do
  desc ...
  validate do |value|
    if Facter.value(:kernel) != 'Linux'
      fail("The overlay property is unsupported")
    end
  end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshcooper isn't puppetserver only available on Linux platforms though?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried at first with your suggestion, but due to the way the properties are treated in the provider it still gets executed on Solaris... oh well

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We provide puppetserver packages for linux variants, but you can run puppetserver from any POSIX server provided you have Java.

It seems like the zfs module already handles "conditional" properties. Probably should add overlay to

[:aclmode, :acltype, :shareiscsi].each do |field|

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll give that a try on Monday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants