hashicorp / setup-terraform

Sets up Terraform CLI in your GitHub Actions workflow.
https://developer.hashicorp.com/terraform/tutorials/automation/github-actions
Mozilla Public License 2.0
1.37k stars 240 forks source link

Changing the wrapper to show real-time output #395

Closed dannystaple closed 5 months ago

dannystaple commented 7 months ago

Context

For lengthy apply's, the wrapper will mean that no output is shown until the end. This is due to the code https://github.com/hashicorp/setup-terraform/blob/main/wrapper/terraform.js using a listener, which looks like it buffers up everything, and then prints it to the console at https://github.com/hashicorp/setup-terraform/blob/main/wrapper/terraform.js#L42.

Potential fix

What would work here, would be to update the listener https://github.com/hashicorp/setup-terraform/blob/main/wrapper/lib/output-listener.js#L28 so the constructor takes an output stream as an argument, like process.stdout, and when making the call this._buff.push(data);, also perform something like this.stream.write(data);, potentially with a this.stream.flush();.

Ie:

  get listener () {
    const listen = function listen (data) {
      this._buff.push(data);
      if (this.stream) {
        this.stream.write(data);
        this.stream.flush();
      }
    };
    return listen.bind(this);
  }

The effect

This will then ensure that every chunk of data gets sent on to the appropriate stream, while also buffering, similar to the tee command.

Testing

Unit tests could be sorted by perhaps using a mock, looking for the stream calls when applied. Something like expect(listener.contents).toHaveCalls(['foo', 'bar', 'baz']);.

ChristianSauer commented 7 months ago

This is super annoying. Especially if you want to diagnose why terraform is hung.

ben-clarke commented 7 months ago

This is super annoying. Especially if you want to diagnose why terraform is hung.

Agreed. I have downgraded to v2 now as that outputs in real-time. Was the only way I could diagnose what the issue was for our run.

vmaerten commented 7 months ago

:+1: It's very annoying

ChristianSauer commented 7 months ago

@ben-clarke I could actually set with: terraform_wrapper: false

and it outputs everything at once but that depends on your use case

SunsetYe66 commented 7 months ago

+1 The new way of dealing STDOUT/STDERR does not synchronous bump things. It leaves Actions page just like hang and we would be thinking if the runner died

justinmchase commented 7 months ago

This is such a major bug, I cannot tell you how many hours I've lost because of this. I had no idea there even was a wrapper being applied so its incredibly difficult to figure out where this issue was coming from.

Here is a little snippet of node code I am using that maybe you could borrow from. I don't think you need to use the action runner exec and then you can just inherit the stdout like normal and not have to worry about it printing the command.

const { spawn } = require('node:child_process')

const vars = process.env[`INPUT_VARS`] || ''
const workingDirectory = process.env[`INPUT_WORKING-DIRECTORY`] || ''
const args = [
  'apply',
  '-input=false',
  '-auto-approve',
  ...vars.split('\n').filter(v => v).map(v => ['--var', v]).flat()
]

console.log('terraform', ...args)
console.log(process.env)

const ctp = spawn('terraform', args, {
  stdio: 'inherit',
  env: process.env,
  cwd: workingDirectory,
})
ctp.ref()
ctp.on('exit', (code) => {
  process.exit(code)
})

const cancel = (signal) => {
  ctp.kill(signal)
}
process.on('SIGINT', cancel)
process.on('SIGTERM', cancel)
justinmchase commented 7 months ago

By the way if you really want to process the output stream I would strongly recommend using a class that extends the Transform class.

Its been a while since I've done this in node but something like this:

export class Wrapper extends Transform {
  _transform(data, encoding, callback) {
    // do whatever you want with data then push it
    // push will pass it through to the next stream
    this.push(data);
    callback();
  }
}
aleuziNC commented 6 months ago

+1 Am I the only one encountering an issue with the output sequence of the terraform apply command, particularly when errors occur? Specifically, I've noticed that detailed error information appears right at the beginning of the output, rather than following the error message itself.

justinmchase commented 6 months ago

Am I the only one encountering an issue with the output sequence of the terraform apply command...

No check out #405

ViacheslavKudinov commented 6 months ago

Same situation. Will be great to get it fixed.

github-actions[bot] commented 4 months ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.