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

(#732) puppet_agent::run: mark failed puppet runs as failed task #742

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bastelfreak
Copy link
Collaborator

without this change:

Started on localhost...
Finished on localhost:
  Info: Refreshing CA certificate
  Info: CA certificate is unmodified, using existing CA certificate
  Info: Using environment 'production'
  Info: Retrieving pluginfacts
  Info: Retrieving plugin
  Info: Loading facts
  Notice: Requesting catalog from puppet.bastel:8140 (127.0.0.1)
  Notice: Catalog compiled by puppet.local
  Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Failed when searching for node puppet.bastel: Exception while executing '/etc/puppetlabs/puppet/node.rb': Cannot run program "/etc/puppetlabs/puppet/node.rb" (in directory "."): error=0, Failed to exec spawn helper: pid: 2133925, exit value: 1
  Warning: Not using cache on failed catalog
  Error: Could not retrieve catalog; skipping run
  {
 *long json*
    },
    "exitcode": 1
  }
Successful on 1 target: localhost
Ran on 1 target in 11.19 sec

With this change:

Started on localhost...
Failed on localhost:
  Puppet agent run failed: Info: Refreshing CA certificate
  Info: CA certificate is unmodified, using existing CA certificate
  Info: Using environment 'production'
  Info: Retrieving pluginfacts
  Info: Retrieving plugin
  Info: Loading facts
  Notice: Requesting catalog from puppet.bastel:8140 (127.0.0.1)
  Notice: Catalog compiled by puppet.local
  Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Failed when searching for node puppet.bastel: Exception while executing '/etc/puppetlabs/puppet/node.rb': Cannot run program "/etc/puppetlabs/puppet/node.rb" (in directory "."): error=0, Failed to exec spawn helper: pid: 2133837, exit value: 1
  Warning: Not using cache on failed catalog
  Error: Could not retrieve catalog; skipping run
Failed on 1 target: localhost
Ran on 1 target in 11.35 sec

The task is now properly marked as failed, when the puppet agent run
failed itself.

By default, `Puppet::Util::Execution.execute` only returns stdout.
Puppet writes errors to stderr. When the run task fails, it only shows
the successful output:

```
$ bolt task run puppet_agent::run --targets puppet
Started on puppet...
Finished on puppet:
  Info: Using environment 'production'
  Info: Retrieving pluginfacts
  Info: Retrieving plugin
  Info: Loading facts
  Notice: Requesting catalog from puppet.local:8140 (10.0.1.1)
  Notice: Catalog compiled by puppet.local

  *long json here*
```

With the patch:

```
$ bolt task run puppet_agent::run --targets puppet
Started on puppet...
Finished on puppet:
  Info: Using environment 'production'
  Info: Retrieving pluginfacts
  Info: Retrieving plugin
  Info: Loading facts
  Notice: Requesting catalog from puppet.local:8140 (10.0.1.1)
  Notice: Catalog compiled by puppet.spirit-int.betadots.training
  Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: Error while evaluating a Function Call, Failed to execute '/pdb/query/v4' on at least 1 of the following 'server_urls': https://puppetdb:8081 (file: /opt/puppetlabs/puppet/modules/infrastructure/manifests/puppet/puppetserver.pp, line: 11, column: 14) on node puppet.local
  Warning: Not using cache on failed catalog
  Error: Could not retrieve catalog; skipping run

  *long json here*
``
@bastelfreak bastelfreak added bug Something isn't working backwards-incompatible Used to mark work that's backwards incompatible, should be a major release. labels Nov 21, 2024
@bastelfreak bastelfreak self-assigned this Nov 21, 2024
@bastelfreak bastelfreak requested a review from a team as a code owner November 21, 2024 19:27
@bastelfreak bastelfreak changed the title Combine2 (#732) puppet_agent::run: mark failed puppet runs as failed task Nov 21, 2024
tasks/run.rb Outdated
@@ -99,6 +99,13 @@ def get_result_from_report(last_run_report, run_result, start_time)
obj.tag = nil if obj.respond_to?(:tag=)
end

# check if the run is marked as failed
if report.to_ruby['status'] == 'failed'
Copy link
Contributor

Choose a reason for hiding this comment

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

nit reports can be large, so maybe only call to_ruby once?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I updated it to call to_ruby only once. But I think it doesn't matter, because both code paths cannot be hit during the same run. when if report.to_ruby['status'] == 'failed' is true, the following return statement will exit the method, so we don't hit the second to_ruby.

# check if the run is marked as failed
if report.to_ruby['status'] == 'failed'
return error_result(
'puppet_agent/agent-run-error',
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we free to add new error_types? Is there a schema for error types? Or is there some translation that happens to map this to a human readable version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've really no idea :( My impression is that we can just add random text here: https://www.puppet.com/docs/bolt/latest/writing_tasks#returning-errors-in-tasks

… task

without this change:

```
Started on localhost...
Finished on localhost:
  Info: Refreshing CA certificate
  Info: CA certificate is unmodified, using existing CA certificate
  Info: Using environment 'production'
  Info: Retrieving pluginfacts
  Info: Retrieving plugin
  Info: Loading facts
  Notice: Requesting catalog from puppet.bastel:8140 (127.0.0.1)
  Notice: Catalog compiled by puppet.local
  Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Failed when searching for node puppet.bastel: Exception while executing '/etc/puppetlabs/puppet/node.rb': Cannot run program "/etc/puppetlabs/puppet/node.rb" (in directory "."): error=0, Failed to exec spawn helper: pid: 2133925, exit value: 1
  Warning: Not using cache on failed catalog
  Error: Could not retrieve catalog; skipping run
  {
 *long json*
    },
    "exitcode": 1
  }
Successful on 1 target: localhost
Ran on 1 target in 11.19 sec
```

With this change:

```
Started on localhost...
Failed on localhost:
  Puppet agent run failed: Info: Refreshing CA certificate
  Info: CA certificate is unmodified, using existing CA certificate
  Info: Using environment 'production'
  Info: Retrieving pluginfacts
  Info: Retrieving plugin
  Info: Loading facts
  Notice: Requesting catalog from puppet.bastel:8140 (127.0.0.1)
  Notice: Catalog compiled by puppet.local
  Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Failed when searching for node puppet.bastel: Exception while executing '/etc/puppetlabs/puppet/node.rb': Cannot run program "/etc/puppetlabs/puppet/node.rb" (in directory "."): error=0, Failed to exec spawn helper: pid: 2133837, exit value: 1
  Warning: Not using cache on failed catalog
  Error: Could not retrieve catalog; skipping run
Failed on 1 target: localhost
Ran on 1 target in 11.35 sec
```

The task is now properly marked as failed, when the puppet agent run
failed itself.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-incompatible Used to mark work that's backwards incompatible, should be a major release. bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants