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

When all_proxy set, use git proxy to build xen-tools #255

Merged
merged 1 commit into from
Sep 11, 2019

Conversation

deitch
Copy link
Contributor

@deitch deitch commented Sep 7, 2019

Signed-off-by: Avi Deitcher avi@deitcher.net

When all_proxy is set, use a git proxy for building pkg/xen-tools.

Fixes #150

Please test @jren1

cc @rvs

Copy link
Contributor

@rvs rvs left a comment

Choose a reason for hiding this comment

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

Hey @deitch -- I think this is moving in the right direction, but I'm not a big fan of adding oe-git-proxy since we will be forced to add it to all the containers using git (and that unfortunately includes all the ones relying on go get).

So... two things:

  1. I am wondering why git config --global http.proxy XXX is not sufficient ?
  2. If it isn't I am wondering if we could explore an option of running a cntlm https://hub.docker.com/r/jaschac/cntlm/ container and then teaching linuxkit to add --link to its invocation of docker build commands.

@deitch
Copy link
Contributor Author

deitch commented Sep 8, 2019

we will be forced to add it to all the containers using git (and that unfortunately includes all the ones relying on go get

Not really. Only pkg/xen-tools is the problematic one, since it is the only one that insists on using the git:// protocol. Every other one (including all of the ones doing go get) use http protocol. If http_proxy or https_proxy (depending on use case) is set, then go get works fine. And since we now pass http_proxy/https_proxy to docker build, and linuxkit pkg build properly respects it, they all work. This is the troublesome child.

I believe that is why @jren1 put this only in pkg/xen-tools and nowhere else.

I am wondering why git config --global http.proxy XXX is not sufficient ?

Because it only works for http. git:// protocol doesn't work with any of that AFAIK, hence the socks proxy. This really is a unique case.

If it isn't I am wondering if we could explore an option of running a cntlm https://hub.docker.com/r/jaschac/cntlm/ container and then teaching linuxkit to add --link to its invocation of docker build commands.

docker build with another container running isn't officially an option. You sort of can hack it by using docker build --network, but it is messy and not recommended unless you really have no choice. With the ability to just tell it, "use this proxy", this is a much cleaner option.

@deitch
Copy link
Contributor Author

deitch commented Sep 8, 2019

In the meantime, I need to rebase.

I also don't know why yetus failed. I added the downloaded script to .yetus-excludes. @aw-was-here any idea? Is this that issue from earlier, about having to add to excludes and merge in, and then have a later PR consume it?

@deitch
Copy link
Contributor Author

deitch commented Sep 8, 2019

Rebased.

@rvs
Copy link
Contributor

rvs commented Sep 8, 2019

Not really. Only pkg/xen-tools is the problematic one, since it is the only one that insists on using the git:// protocol. Every other one (including all of the ones doing go get) use http protocol. If http_proxy or https_proxy (depending on use case) is set, then go get works fine. And since we now pass http_proxy/https_proxy to docker build, and linuxkit pkg build properly respects it, they all work. This is the troublesome child.

Ah! Ok. Then I simply suggest we add GIT_HTTP=y to our Xen build

This is supposed to activate this part and remove the need for oe-git-proxy (as per your explanation): http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=Config.mk;h=0fa4591379911e56049eef5684a2d7f08995b7b3;hb=HEAD#l264

Makes sense @deitch ?

@deitch
Copy link
Contributor Author

deitch commented Sep 9, 2019

Seriously? They have an http endpoint? I had thought they had insisted on git-protocol only. Ha!

That makes these changes irrelevant. I am going to change this PR to always use HTTP_PROXY for xen-tools and be done.

@deitch
Copy link
Contributor Author

deitch commented Sep 9, 2019

Just redid it with that simple change.

@jren1 can you please validate?

Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

If this addresses the problem its fine with me.

@deitch
Copy link
Contributor Author

deitch commented Sep 10, 2019

I will rebase to latest master. @rvs can you approve changes? @jren1 can you confirm it works?

Signed-off-by: Avi Deitcher <avi@deitcher.net>
@rvs
Copy link
Contributor

rvs commented Sep 10, 2019

LGTM! But it would be great to get @jren1 sign-off on this.

@deitch
Copy link
Contributor Author

deitch commented Sep 11, 2019

We just got confirmation from @jren1 by email that we now have full behind-the-proxy support. @rvs please merge in.

@rvs rvs merged commit d0d89c9 into lf-edge:master Sep 11, 2019
@rvs
Copy link
Contributor

rvs commented Sep 11, 2019

Awesome job @deitch ! Thank you!

@deitch deitch deleted the git-proxy branch September 11, 2019 13:17
milan-zededa pushed a commit to milan-zededa/eve that referenced this pull request Jan 22, 2024
add more debug level to virtctl image uploading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed to build xen-tools
3 participants