voxpupuli / puppet-augeasproviders_grub

Augeas-based grub type and providers
Apache License 2.0
9 stars 33 forks source link

grub_menuentry resource fail if directory /boot/grub doe not exist #53

Closed aursu closed 4 years ago

aursu commented 4 years ago

Hello, there is a bug inside grub2 provider for custom type Grub_menuentry which cause next behaviour:

Notice: /Stage[main]/Kernel/Grub_menuentry[Puppet-Managed Kernel Entry]/default_entry: default_entry changed false to true (corrective)
Debug: Executing: '/usr/sbin/grub2-mkconfig'
Error: /Grub_menuentry[Puppet-Managed Kernel Entry]: Could not evaluate: No such file or directory @ rb_sysopen - /boot/grub/grub.cfg
Notice: /Stage[main]/Kernel/File_line[grubenv saved_entry]: Dependency Grub_menuentry[Puppet-Managed Kernel Entry] has failures: true
Warning: /Stage[main]/Kernel/File_line[grubenv saved_entry]: Skipping because of failed dependencies

Bug is inside this block of method grub2_mkconfig

    cfg_paths.uniq.each do |cfg_path|
      File.open(cfg_path, 'w') do |fh|
        fh.puts(mkconfig_output)
        fh.flush
      end
    end

if folder /boot/grub is not existing it will fail with

Errno::ENOENT: No such file or directory @ rb_sysopen - /boot/grub/grub.cfg

Please review and fix it

trevor-vaughan commented 4 years ago

@aursu This is an accurate error. Nothing can be done if the grub configuration is not present.

Would you prefer a more clean early bail out if no target files can be found?

aursu commented 4 years ago

@aursu This is an accurate error. Nothing can be done if the grub configuration is not present.

Would you prefer a more clean early bail out if no target files can be found?

Hello, @trevor-vaughan

You are right. this issue should be categorised as feature request rather than bug

Let me explain. There is parameter cfg_paths for method grub2_mkconfig. It is defined as list of configuration files paths which all (without exceptions) must be writable. Otherwise block https://github.com/hercules-team/augeasproviders_grub/blob/316d74750a9c2f7674fdb9d5b8c85c6a7b2aaea3/lib/puppet/provider/grub_menuentry/grub2.rb#L788 will fail with exception I've mentioned above

The trouble is that path /boot/grub/grub.cfg is default path for vanilla Grub2 version and Debian based distros (I've found at least in Ubuntu documentation).

But RedHat and consequently CentOS default is /boot/grub2/grub.cfg therefore missed /boot/grub directory is not an issue on these distros. But : grub_menuentry requires (implicitly) this path to be existing.

Therefore I propose to add some conditional checks which will allow to not fail but just skip /boot/grub/grub.cfg on RedHat based distros

As workaround to this issue I've added resource file { '/boot/grub': ensure => directory, before => Grub_menuentry['Puppet-Managed Kernel Entry'], } into catalog but in my opinion it should be resolved on provider level

Thank you

trevor-vaughan commented 4 years ago

Interesting. It was tested primarily on CentOS systems!

I'll double check that the acceptance tests are picking it up and go from there.

trevor-vaughan commented 4 years ago

@aursu Determined that this is a legit bug. The logic isn't quite right. Prodding and will push a patch when things are in order.

trevor-vaughan commented 4 years ago

@aursu Can you try out the associated patch and let me know if it resolves your issue?