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

error: fork/exec /bin/sh: argument list too long #285

Closed
glennpratt opened this issue Oct 24, 2023 · 11 comments · Fixed by #355
Closed

error: fork/exec /bin/sh: argument list too long #285

glennpratt opened this issue Oct 24, 2023 · 11 comments · Fixed by #355
Assignees
Labels
good-first-issue Start here if you'd like to start contributing to Pulumi help-wanted We'd love your contributions on this issue kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed

Comments

@glennpratt
Copy link

What happened?

Running pulumi up on an existing stack on Linux fails.

Poking at the code, I see this is because of PULUMI_COMMAND_STDOUT/PULUMI_COMMAND_STDERR and the previous run containing rather long output.

if out.Stdout != "" {
cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", util.PULUMI_COMMAND_STDOUT, out.Stdout))
}
if out.Stderr != "" {
cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", util.PULUMI_COMMAND_STDERR, out.Stderr))
}

Since I don't use this feature and I don't always have the option of adjusting limits, I'd like an option to turn it off. Furthermore, I'd prefer if the option could be made off by default as well, as I don't generally need this information in state files.

Example

strace

sudo -E strace -E PATH=$PATH -u $USER -f -e trace=execve -s 1000 -v --signals=SIGPWR pulumi up 2> log

From log above, we can tell our command is not particularly long, the environment is massive.

execve("/bin/sh", ["/bin/sh", "-c", "ansible REDACTED"], ["SHELL=/bin/bash", ...REDACTED..., "PULUMI_COMMAND_STDOUT=ansible-playbook [core 2.14.6]\n  config file REDACTED"...]) = -1 E2BIG (Argument list too long)

Output of pulumi about

CLI
Version      3.75.0
Go Version   go1.20.5
Go Compiler  gc

Host
OS       ubuntu
Version  20.04
Arch     x86_64

Backend
Name           REDACTED
URL            file://~
User           REDACTED
Organizations

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@glennpratt glennpratt added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Oct 24, 2023
@mikhailshilkov mikhailshilkov added good-first-issue Start here if you'd like to start contributing to Pulumi help-wanted We'd love your contributions on this issue and removed needs-triage Needs attention from the triage team labels Oct 26, 2023
@AMKamel
Copy link

AMKamel commented Oct 31, 2023

Same here:
image

CLI          
Version      3.91.1
Go Version   go1.21.3
Go Compiler  gc

Plugins
NAME    VERSION
nodejs  unknown

Host     
OS       ubuntu
Version  22.04
Arch     x86_64

@AMKamel
Copy link

AMKamel commented Nov 6, 2023

I was only able to work around this by pulumi stack export then edit the stack manually to make command "stdout": "" then pulumi stack import.

This is the reason, also I am heading towards always using no_log:true in ansible playbook to avoid this, but what was the objective of storing this stdout/stderr in state?

@glennpratt
Copy link
Author

@antdking pointed me to #167 in Slack, thanks!

I think as a first change, it just needs to become opt-out.

I think it should be opt-in long term because it's very surprising and I'm not sure it can be reliably detected - and even if it can it's presumably going to require a pile of platform specific code. Also, it balloons the state file for something most probably don't depend on.

Personally, I'd consider deprecating it; it would seam preferable for users to immediately parse stdout into assets or something else to track after create, not at delete time. Stderr is for error messages and diagnostics, it doesn't match the spirit of the stream to implicitly store for future parsing.

julsemaan pushed a commit to julsemaan/pulumi-command that referenced this issue Mar 8, 2024
iwahbe added a commit that referenced this issue Apr 11, 2024
fixes #285

---------

Co-authored-by: Your Name <[email protected]>
Co-authored-by: Matthew Jeffryes <[email protected]>
Co-authored-by: Ian Wahbe <[email protected]>
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Apr 11, 2024
@theneva
Copy link
Contributor

theneva commented May 21, 2024

@iwahbe I've just encountered this issue in @pulumi/command version 0.11.1 with a remote.Command. After some digging, it seems to me like #355 only addresses local.Commands. Am I completely missing something?

If I'm not missing anything, is this issue the right place to ask nicely for opt-out support for remote.Command as well?

@julsemaan
Copy link
Contributor

I originally authored the fix and you're right, this only deals with local.command as this is what I used and had the ability to test easily. I have 0 clue how to use the remote functionality but if you provide a minimal example to use it, I can give a shot at creating a PR with the fix

@iwahbe
Copy link
Member

iwahbe commented May 22, 2024

CC @thomas11

@thomas11
Copy link
Contributor

Thank you @julsemaan, a PR would be awesome! At first glance it seems the code for remote works the same way. Here is the equivalent place to where you added if AddPreviousOutputInEnv ... for local.

Possibly we can share some code between local and remote but we can look at that later.

@julsemaan
Copy link
Contributor

Opened a PR: #451

It's lacking a test though as I couldn't mock things appropriately for remote.Command

thomas11 added a commit that referenced this issue Jun 17, 2024
#451)

# Context

At the moment, stdout and stderr of previous runs are passed as
environment variables on future invocations. When stdout or stderr is
too large, this cause an error.

# Proposed change

#355 fixed it for local commands, this fixed it for remote commands. See
#285 for full details on the issue

See this comment for details around local vs remote functionality :
#285 (comment)

---------

Co-authored-by: Thomas Kappler <[email protected]>
Co-authored-by: Martin Lehmann <[email protected]>
@MLOpsGuy
Copy link

This issue was closed but I don't understand your answers. I used local.Command and I have the argument list too long error on update and delete states. Someone can help me, please?

@thomas11
Copy link
Contributor

Hi @MLOpsGuy, the previous behavior that can lead to this error is still the default. You need to set addPreviousOutputInEnv = false on your local.Command resource.

@MLOpsGuy
Copy link

Thanks @thomas11, I really appreciate your help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-first-issue Start here if you'd like to start contributing to Pulumi help-wanted We'd love your contributions on this issue kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants