Native Chef Helpers functions duplicated as node methods

[ChefAustin]

I don't understand why some of the native Chef helper functions [1] -- like def linux?, def macos?, def debian_family?, et al -- are being duplicated in fb_helpers, cpe_helpers.

Rather than extending the node object to contain functions already available to the Chef DSL, why not just use the built-in ones?

Frame number: 0/26

From: /etc/chef/local-mode-cache/cache/cookbooks/cpe_my_org_init/recipes/default.rb:17 Chef::Mixin::FromFile#from_file:

    12: # of patent rights can be found in the PATENTS file in the same directory.
    13: #
    14: require 'pry'
    15: binding.pry
    16: # Start an empty run_list so we can append to it
 => 17: run_list = []
    18:
    19: # All machines
    20: run_list += [
    21:   'global_base',
    22:   'my_org_base',

[1] pry(#<Chef::Recipe>)> show-method linux?

From: /opt/chef/embedded/lib/ruby/gems/3.0.0/gems/chef-utils-17.7.29/lib/chef-utils/dsl/os.rb:28:
Owner: ChefUtils::DSL::OS
Visibility: public
Signature: linux?(node=?)
Number of lines: 3

def linux?(node = __getnode)
  node["os"] == "linux"
end
[2] pry(#<Chef::Recipe>)> show-method node.linux?

From: /etc/chef/local-mode-cache/cache/cookbooks/fb_helpers/libraries/node_methods.rb:50:
Owner: Chef::Node
Visibility: public
Signature: linux?()
Number of lines: 3

def linux?
  self['os'] == 'linux'
end

Can anyone explain why this is necessary/beneficial?

[1] See: https://github.com/chef/chef/blob/main/chef-utils/lib/chef-utils/dsl/os.rb & https://github.com/chef/chef/blob/main/chef-utils/lib/chef-utils/dsl/platform.rb & https://github.com/chef/chef/blob/main/chef-utils/lib/chef-utils/dsl/platform_family.rb

[nmcspadden]

The darwin? one, for example, was added in 15.5: https://github.com/chef/chef/blob/main/chef-utils/lib/chef-utils/dsl/os.rb#L47

These predate nearly all of the built-in helper functions. We've just never migrated because it didn't make a difference.

On Thu, Nov 4, 2021 at 7:35 AM Austin Culter @.***> wrote:

I don't understand why some of the native Chef helper functions [1] -- like def linux?, def macos?, def debian_family?, et al -- are being duplicated in fb_helpers, cpe_helpers.

Rather than extending the node object to contain functions already available to the Chef DSL, why not just use the built-in ones?

Frame number: 0/26

From: /etc/chef/local-mode-cache/cache/cookbooks/cpe_my_org_init/recipes/default.rb:17 Chef::Mixin::FromFile#from_file:

12: # of patent rights can be found in the PATENTS file in the same directory.
13: #
14: require 'pry'
15: binding.pry
16: # Start an empty run_list so we can append to it

=> 17: run_list = [] 18: 19: # All machines 20: run_list += [ 21: 'global_base', 22: 'my_org_base',

[1] pry(#)> show-method linux?

From: /opt/chef/embedded/lib/ruby/gems/3.0.0/gems/chef-utils-17.7.29/lib/chef-utils/dsl/os.rb:28: Owner: ChefUtils::DSL::OS Visibility: public Signature: linux?(node=?) Number of lines: 3

def linux?(node = __getnode) node["os"] == "linux" end [2] pry(#)> show-method node.linux?

From: /etc/chef/local-mode-cache/cache/cookbooks/fb_helpers/libraries/node_methods.rb:50: Owner: Chef::Node Visibility: public Signature: linux?() Number of lines: 3

def linux? self['os'] == 'linux' end

Can anyone explain why this is necessary/beneficial?

[1] See: https://github.com/chef/chef/blob/main/chef-utils/lib/chef-utils/dsl/os.rb & https://github.com/chef/chef/blob/main/chef-utils/lib/chef-utils/dsl/platform.rb & https://github.com/chef/chef/blob/main/chef-utils/lib/chef-utils/dsl/platform_family.rb

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/facebook/IT-CPE/issues/265, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJFTX6VT4DYQ4LGDMT56MLUKKKZ7ANCNFSM5HLS6RBA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

--

Nick McSpadden @.***

[ChefAustin]

Would you entertain a PR to remove them? Or is your fleet of devices running pre-15.5 client versions and such a PR would break them?

[nmcspadden]

We likely wouldn't accept that PR without pretty thorough testing in our environment. I'd encourage you to put one up to kickstart the conversation, but migrating that is also pretty low-pri unless there's a demonstrable difference in performance/time - or some other indication of problem that this is causing.

On Thu, Nov 4, 2021 at 8:51 AM Austin Culter @.***> wrote:

Would you entertain a PR to remove them? Or is your fleet of devices running pre-15.5 client versions and such a PR would break them?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/facebook/IT-CPE/issues/265#issuecomment-961176396, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJFTX3H6EKZXKBSUL5TRLDUKKTZVANCNFSM5HLS6RBA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

--

Nick McSpadden @.***

[ChefAustin]

We likely wouldn't accept that PR without pretty thorough testing in our environment. I'd encourage you to put one up to kickstart the conversation, but migrating that is also pretty low-pri unless there's a demonstrable difference in performance/time - or some other indication of problem that this is causing.

PR opened. Let me know if you have any concerns or discover any issues when you get around to testing internally.

[erikng]

Yesterday I discovered namespace collisions breaking a lot of our linux logic. https://github.com/facebook/chef-cookbooks/issues/211