Skip to content

(MODULES-11788) Pass converted boolean parameter#379

Merged
amitkarsale merged 3 commits intomainfrom
bool-11788
Apr 29, 2026
Merged

(MODULES-11788) Pass converted boolean parameter#379
amitkarsale merged 3 commits intomainfrom
bool-11788

Conversation

@joshcooper
Copy link
Copy Markdown
Contributor

@joshcooper joshcooper commented Apr 28, 2026

Summary

Previously, we converted true/false to yes/no, but passed the original value to
the crfs command, resulting in

crfs: 0507-587 Invalid value for isnapshot option specified, true.

Adds missing tests and CLAUDE.md

Checklist

  • 🟢 Spec tests.
  • 🟢 Acceptance tests.
  • Manually verified. (For example puppet apply)

Before

filesystem { '/mnt/test':
  ensure       => present,
  fs_type      => 'jfs2',
  volume_group => 'rootvg',
  size         => '32M',
  isnapshot    => false,
  atboot       => false,
}
# /opt/puppetlabs/puppet/bin/puppet apply --debug manifest.pp
...
Debug: Executing: '/usr/sbin/crfs -a size=32M -a isnapshot=false -v jfs2 -m /mnt/test -g rootvg -A no'
Error: Execution of '/usr/sbin/crfs -a size=32M -a isnapshot=false -v jfs2 -m /mnt/test -g rootvg -A no' returned 1: crfs: 0507-587 Invalid value for isnapshot option specified, false.
        Valid values for the isnapshot option: yes, no
Error: /Stage[main]/Main/Filesystem[/mnt/test]/ensure: change from 'absent' to 'present' failed: Execution of '/usr/sbin/crfs -a size=32M -a isnapshot=false -v jfs2 -m /mnt/test -g rootvg -A no' returned 1: crfs: 0507-587 Invalid value for isnapshot option specified, false.
        Valid values for the isnapshot option: yes, no

After

# /opt/puppetlabs/puppet/bin/puppet apply --debug manifest.pp
...
Debug: Executing: '/usr/sbin/crfs -a size=32M -a isnapshot=no -v jfs2 -m /mnt/test -g rootvg -A no'
Notice: /Stage[main]/Main/Filesystem[/mnt/test]/ensure: created

Also

# /opt/puppetlabs/puppet/bin/puppet resource filesystem --debug /mnt/test ensure=present fs_type=jfs2 volume_group=rootvg size=32M isnapshot=true atboot=false
...
Debug: Executing: '/usr/sbin/crfs -a size=32M -a isnapshot=yes -v jfs2 -m /mnt/test -g rootvg -A no'
Notice: /Filesystem[/mnt/test]/ensure: created

@joshcooper joshcooper marked this pull request as ready for review April 28, 2026 17:06
@joshcooper joshcooper requested review from a team and bastelfreak as code owners April 28, 2026 17:06
joshcooper and others added 3 commits April 28, 2026 10:09
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously, we converted true/false to yes/no, but passed the original value to
the crfs command, resulting in

    crfs: 0507-587 Invalid value for isnapshot option specified, true.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
it 'converts :true attribute values to "yes"' do
# large_files has no entry in attribute_flag's rename map (only large_file does),
# so the attribute name passes through as-is: "-a large_files=yes"
resource[:large_files] = :true
Copy link
Copy Markdown
Contributor Author

@joshcooper joshcooper Apr 28, 2026

Choose a reason for hiding this comment

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

There are multiple bugs with large_files unrelated to this PR:

  • It should pass bf not large_files. The crfs command ignores the unknown large_files option.
  • The bf parameter accepts true/false, not yes/no
# crfs -a bf=yes -a size=32M -v jfs -m /mnt/test -g rootvg -A no                                                               
crfs: 0506-974 Please specify one of the following values for the -a bf option: true, false.  
  • The bf parametr is not supported in JFS2 (because large files are already enabled)
# crfs -a bf=yes -a size=32M -v jfs2 -m /mnt/test -g rootvg -A no                                                            
crfs: 0506-940 JFS2 does not support attribute bf.

Since large_files is not an option in modern JFS2 and is a special case when it comes to parsing boolean values, I left it as-is.

Copy link
Copy Markdown
Collaborator

@cthorn42 cthorn42 left a comment

Choose a reason for hiding this comment

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

Looks good to me, validated the bug with this manifest and then applied this fix and it is now compiling:

  filesystem { '/mnt/test':
    ensure       => present,
    fs_type      => 'jfs2',
    volume_group => 'rootvg',
    size         => '32M',
    isnapshot    => false,
    atboot       => false,
  }

/opt/puppetlabs/puppet/bin/puppet apply bah.pp
Notice: Compiled catalog for aix73-1-pix.delivery.puppetlabs.net in environment production in 0.10 seconds
Error: Execution of '/usr/sbin/crfs -a size=32M -a isnapshot=false -v jfs2 -m /mnt/test -g rootvg -A no' returned 1: crfs: 0507-587 Invalid value for isnapshot option specified, false.
        Valid values for the isnapshot option: yes, no
Error: /Stage[main]/Main/Filesystem[/mnt/test]/ensure: change from 'absent' to 'present' failed: Execution of '/usr/sbin/crfs -a size=32M -a isnapshot=false -v jfs2 -m /mnt/test -g rootvg -A no' returned 1: crfs: 0507-587 Invalid value for isnapshot option specified, false.
        Valid values for the isnapshot option: yes, no

/opt/puppetlabs/puppet/bin/puppet apply bah.pp
Notice: Compiled catalog for aix73-1-pix.delivery.puppetlabs.net in environment production in 0.09 seconds
Notice: /Stage[main]/Main/Filesystem[/mnt/test]/ensure: created
Notice: Applied catalog in 1.33 seconds

@amitkarsale amitkarsale merged commit 55a52d1 into main Apr 29, 2026
15 checks passed
@amitkarsale amitkarsale deleted the bool-11788 branch April 29, 2026 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants