-
Notifications
You must be signed in to change notification settings - Fork 163
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
Optional isolated environment #673
base: main
Are you sure you want to change the base?
Optional isolated environment #673
Conversation
…nt of host-system;
ede6eef
to
caa8b20
Compare
caa8b20
to
eacc737
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
I'd prefer to keep this complexity out of the bosh cli since the issue this appears to be addressing could also be side-stepped by running the bosh CLI in a docker container. Perhaps there is some broader issue I'm missing here though? Regarding the code changes, it seems like there are a number of additions, specifically for nix, beyond a "don't isolate my environment" flag, I'm wondering if those need to be in the Regarding the feature I am concerned that allowing more variation in the local environment will increase the surface area we will have to consider when triaging issues with the CLI. |
"PATH": "/usr/local/bin:/usr/bin:/bin:/sbin", | ||
"PATH": os.Getenv("PATH"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps a default value of "/usr/local/bin:/usr/bin:/bin:/sbin"
should remain here and we allow an override with something like BOSH_CPI_COMAND_PATH
.
I think there are two core changes lurking in this PR
The other Side note: bosh-cli appears to be the only project in the |
@fmoehler @aramprice
The broader issue as far as i remember is that the isolation feature in case of create-release is somewhat pointless on systems other than ubuntu. It basically boils down to bosh-cli does not accept the environment variable the OS gives it - however then it proceeds calling OS packages (gcc etc.) and fails either not finding those as it looks not in the path the OS told the process to look in but in a hardcoded one. Or It cannot find the proper libraries development headers when compiling with GCC since it also does not pass the information the OS has given via ENV variables to the GCC process. In this state afaik the current create-release with enabled isolation(default?) does not work on every linux/unix system that has all the needed tools/libraries installed, it just works on systems that have the tools installed how ubuntu places them into its filesystem specifically. One way would also be to state that just ubuntu is supported and one should run this inside a ubuntu container/vm - however compared to whats needed to make it work on all unix systems it might be easy to enable all linux/unix systems when holding on to basic POSIX concepts - but the exclusion exists for windows it seems already #626 However now looking into the code i saw there is also BOSH_CPI_USE_ISOLATED_ENV - maybe it can be used to disable isolation of env entirely then building a bosh release would also work on a non ubuntu linux as bosh and all build tools behind it would respect the OS env. WDYT ? Also disabling that feature and using all OS Envs would be ok as a solution maybe BOSH_CPI_USE_ISOLATED_ENV does this need to test that maybe. |
This struct attribute is only ever set to `true` by the bosh-cli, and the value if this use case is in question, see: - cloudfoundry/bosh-cli#660 - cloudfoundry/bosh-cli#663 - cloudfoundry/bosh-cli#673 In addition the `UseIsolatedEnv` capability can be wholely handled within the `bosh-cli` codebase itself without the need for this featuer in `bosh-utils`.
This struct attribute is only ever set to `true` by the bosh-cli, and the value if this use case is in question, see: - cloudfoundry/bosh-cli#660 - cloudfoundry/bosh-cli#663 - cloudfoundry/bosh-cli#673 In addition the `UseIsolatedEnv` capability can be wholely handled within the `bosh-cli` codebase itself without the need for this featuer in `bosh-utils`.
We talked about this at the FIWG meeting. We think a good direction is to just always enable the isolated environment within the bosh cli. Even removing the current flag that allows you to disable it. And then have the CLI pass in the full set of ENV variables that a CPI might need to run (PATH, LIBRARY_PATH, etc) This might cause some breakages, but we can just deal with those if they arise. |
The bosh-cli wipes parts of the environment provided by the host-system which are in some cases essential for compilation of bosh-releases. This PR follows a conservative fix-approach by adding an option to opt-out of this default. This can (and maybe should) be extended by a fix of the “isolation” from the host-environment. For more details, see #660