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

Export proxy env vars inside docker build #91

Merged

Conversation

marquiz
Copy link
Contributor

@marquiz marquiz commented Feb 1, 2018

Without correct proxy settings propagated to docker build it is not possible to
build node-feature-discovery from behind a proxy server, e.g. in corporate
networks. This patch fixes the issue by exporting all http(s) proxy related
environment variables as build-time variables.

@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 1, 2018
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Feb 2, 2018
@@ -12,4 +12,10 @@ all: docker
# QUAY_REGISTRY_USER=<my-username> make docker -e.
docker:
docker build --build-arg NFD_VERSION=$(VERSION) \
--build-arg http_proxy=$(http_proxy) \
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this change is required. Usually developers use software mechanisms to bypass firewall and proxy settings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you see that this change would cause some problems? There are developers, like me, who do not use these software mechanisms or do not want to deploy those in every development environment. These proxy settings are ubiquitous all over kubernetes so why couldn't nfd build support these, as well?

@marquiz marquiz force-pushed the feature/proxy-settings branch 2 times, most recently from 1bbb109 to 4b53f0b Compare March 22, 2018 08:08
@marquiz
Copy link
Contributor Author

marquiz commented Mar 22, 2018

ping @balajismaniam, any further comments?

@marquiz
Copy link
Contributor Author

marquiz commented Mar 29, 2018

/retest

Without correct proxy settings propagated to docker build it is not possible to
build node-feature-discovery from behind a proxy server, e.g. in corporate
networks. This patch fixes the issue by exporting all http(s) proxy related
environment variables as build-time variables.
@marquiz
Copy link
Contributor Author

marquiz commented Apr 5, 2018

ping @balajismaniam! This is biting me all the time

@balajismaniam balajismaniam merged commit 1314a5f into kubernetes-sigs:master Apr 9, 2018
@marquiz marquiz deleted the feature/proxy-settings branch April 12, 2018 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants