puppetlabs / puppetlabs/puppet-resource_api

Improve difference reporting in debug log

Open
#282 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
Ruby
Stars
27
Forks
42
PR merge metrics
No merged PRs in 30d

Description

Use Case

As reported in slack, the resource API's debug output could be more helpful when applying changes. Here's the original report:

[1] pry(#<Puppet::Provider::CalicoGlobalNetworkPolicy::CalicoGlobalNetworkPolicy>)> continue                                                                                                [34/38693]
Debug: Current State: {:name=>"web", :order=>10, :ingress=>[{"action"=>"Allow", "protocol"=>"TCP", "source"=>{"nets"=>["10.0.2.0/24"]}, "destination"=>{"ports"=>[443]}}], :egress=>[{"action"=>"Allow", "protocol"=>"TCP", "source"=>{}, "destination"=>{"nets"=>["0.0.0.0/0"]}}], :selector=>"app == \"web\"", :types=>["Ingress", "Egress"], :pre_dnat=>false, :apply_on_forward=>false, :ensure=>"present"}                                                                                                                                                                                                   Notice: /Stage[main]/Main/Node[default]/Calico_global_network_policy[web]/ingress: ingress changed [
  {
    'action' => 'Allow',
    'protocol' => 'TCP',
    'source' => {
      'nets' => ['10.0.2.0/24']
    },
    'destination' => {
      'ports' => [443]
    }
  }] to [
  {
    'action' => 'Allow',
    'protocol' => 'TCP',
    'source' => {
      'nets' => ['10.0.2.0/24']
    },
    'destination' => {
      'ports' => ['443']
    }
  }]
Debug: Target State: {:name=>"web", :types=>["Ingress", "Egress"], :selector=>"app == \"web\"", :ingress=>[{"action"=>"Allow", "protocol"=>"TCP", "source"=>{"nets"=>["10.0.2.0/24"]}, "destination"=> {"ports"=>["443"]}}], :egress=>[{"action"=>"Allow", "protocol"=>"TCP", "source"=>{}, "destination"=>{"nets"=>["0.0.0.0/0"]}}], :order=>10, :ensure=>"present", :pre_dnat=>false, :apply_on_forward=>false}
Debug: calico_global_network_policy[web]: Updating: Start
Notice: calico_global_network_policy[web]: Updating: Updating 'web' with {:name=>"web", :types=>["Ingress", "Egress"], :selector=>"app == \"web\"", :ingress=>[{"action"=>"Allow", "protocol"=>"TCP", "source"=>{"nets"=>["10.0.2.0/24"]}, "destination"=>{"ports"=>["443"]}}], :egress=>[{"action"=>"Allow", "protocol"=>"TCP", "source"=>{}, "destination"=>{"nets"=>["0.0.0.0/0"]}}], :order=>10, :ensure=>"present", :pre_dnat=>false, :apply_on_forward=>false}
Debug: Executing: '/usr/local/bin/calicoctl patch globalnetworkpolicy web -p '{"spec":{"order":10,"ingress":[{"action":"Allow","protocol":"TCP","source":{"nets":["10.0.2.0/24"]},"destination":{"ports":["443"]}}],"egress":[{"action":"Allow","protocol":"TCP","source":{},"destination":{"nets":["0.0.0.0/0"]}}],"selector":"app == \"web\"","types":["Ingress","Egress"],"preDNAT":false,"applyOnForward":false}}''
Notice: calico_global_network_policy[web]: Updating: Finished in 0.033700 seconds

Try finding why above is triggering a change without looking at the solution below.

Describe the Solution You Would Like

Around

https://github.com/puppetlabs/puppet-resource_api/blob/bd93dbac616baa4ffa18c99a0b45946c9d474fec/lib/puppet/resource_api.rb#L330-L334

implement a debug-optional diff on the full data structures in @rsapi_current_state vs target_state using a similar technique as rspec's matchers:

  expected: {"action"=>"Allow", "destination"=>{"ports"=>["443"]}, "protocol"=>"TCP", "source"=>{"nets"=>["10.0.2.0/24"]}}
      got: {"action"=>"Allow", "destination"=>{"ports"=>[443]}, "protocol"=>"TCP", "source"=>{"nets"=>["10.0.2.0/24"]}}

  (compared using ==)

  Diff:
  @@ -1,5 +1,5 @@
   "action" => "Allow",
  -"destination" => {"ports"=>["443"]},
  +"destination" => {"ports"=>[443]},
   "protocol" => "TCP",
   "source" => {"nets"=>["10.0.2.0/24"]},

produced by

it {
  expect({
    'action' => 'Allow',
    'protocol' => 'TCP',
    'source' => {
      'nets' => ['10.0.2.0/24']
    },
    'destination' => {
      'ports' => [443]
    }
  }).to eq({
    'action' => 'Allow',
    'protocol' => 'TCP',
    'source' => {
      'nets' => ['10.0.2.0/24']
    },
    'destination' => {
      'ports' => ['443']
    }
  })
}

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in lib/puppet/resource_api.rb around lines 330-334, where @rsapi_current_state is compared with target_state. Review the requested RSpec-style expected, got, and diff output and determine how it should be enabled for debug logging. Done means differing nested structures produce a focused diff instead of only the broad change report.

Written by the indexing model from the issue text.

Assessment

Tech stack
ruby
Domain
backend
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.