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

Unable to use the "--env" option on finch container run #827

Open
alecthegeek opened this issue Feb 21, 2024 · 3 comments
Open

Unable to use the "--env" option on finch container run #827

alecthegeek opened this issue Feb 21, 2024 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@alecthegeek
Copy link

alecthegeek commented Feb 21, 2024

Describe the bug
Cannot use --env option

Steps to reproduce

finch container run -it --rm --env="E=v" busybox env

Expected behavior

PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
TERM=xterm
HOME=/root
E=v

Screenshots or logs

Actual result

FATA[0000] unknown shorthand flag: 'e' in -e

Additional context

Note: Updated to Finch 1.1.2 -- error still present

As a test I ran nerdctl (installed via Rancher Desktop) and it worked as expected

$ nerdctl container run -it --rm --env="E=v" busybox env
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
E=v
TERM=xterm
HOME=/root

To help debug the issue as quickly as possible, we recommend generating a support bundle with finch support-bundle generate and attaching it to this issue. This packages all Finch-related configs and logs into one file.
finch-support-20240222101339.zip

@alecthegeek alecthegeek added the bug Something isn't working label Feb 21, 2024
@mharwani
Copy link
Member

Thanks for the issue. The command works properly when you use finch run, but fails with finch container run:

$ finch run -it --rm --env="E=v" busybox env
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
E=v
TERM=xterm
HOME=/root
$ finch container run -it --rm --env="E=v" busybox env
FATA[0000] unknown shorthand flag: 'e' in -e            
FATA[0000] exit status 1

The --debug option shows improper ordering of nerdctl arguments, where -e argument is passed before the run command:

$ finch --debug container run -it --rm --env="E=v" busybox env
DEBU[0000] Creating limactl command: ARGUMENTS: [shell finch sudo nerdctl container -e E=v run -it --rm busybox env], LIMA_HOME: /Applications/Finch/lima/data
...

@mharwani
Copy link
Member

In the following code:

limaArgs = append(limaArgs, append([]string{nerdctlCmdName}, strings.Fields(cmdName)...)...)

var finalArgs []string
for key, val := range envVars {
	finalArgs = append(finalArgs, "-e", fmt.Sprintf("%s=%s", key, val))
}
finalArgs = append(finalArgs, nerdctlArgs...)
// Add -E to sudo command in order to preserve existing environment variables, more info:
// https://stackoverflow.com/questions/8633461/how-to-keep-environment-variables-when-using-sudo/8633575#8633575
limaArgs = append(limaArgs, finalArgs...)

The environment variables (such as -e E=v) are appended first, followed by rest of the arguments. For most cases, such as finch run, this should be fine because run is used as the command appended before any arguments. However, for cases like finch container run, run is treated as another argument and appended after the environment variables.

@mharwani mharwani self-assigned this Feb 24, 2024
Shubhranshu153 added a commit that referenced this issue Mar 28, 2024
Issue #, if available: #827

Description of changes:
Addresses #827 by adding all
environment variables at the location of first occurrence of the
environment flag

Testing done:
Changes tested locally to ensure that the issue is fixed.

$ ./_output/bin/finch container run --debug -it --rm --env="E=v" busybox
env
DEBU[0000] Creating limactl command: ARGUMENTS: [shell finch sudo -E
nerdctl container run -e E=v -it --rm busybox env], LIMA_HOME:
/Users/mharwani/work/runfinch/finch/_output/lima/data
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
E=v
TERM=xterm
HOME=/root
*Testing done:*



- [ ] I've reviewed the guidance in CONTRIBUTING.md


#### License Acceptance

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

Signed-off-by: Shubharanshu Mahapatra <[email protected]>
@alecthegeek
Copy link
Author

Tested release 1.1.3

The original bug report is now fixed.

However as explained in the PR the following does not work

finch container run -it --rm -e "E=v" busybox echo -e "hello\tbye"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants