-
Notifications
You must be signed in to change notification settings - Fork 759
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
[wip] Added containerId to etc/hosts so as to resolve hostname. Closes #4446 #4816
Conversation
…ainers#4446 Signed-off-by: Saurabh Ahuja <nsit.saurabh@gmail.com>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: SaurabhAhuja1983 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@SaurabhAhuja1983 Thanks for the PR! A couple of things. First, how did the file in the ./vendor directory change? If you did so "by hand", that won't work. The next time common is vendored in, it will be overwritten. That fix, if it is not already, needs to be made in the containers/common project and then vendored into Buildah. Second, you did not add a test for this change. I think that's OK, but it would be best to add one if possible. If not, please add the tag |
Thank You @TomSweeneyRedHat for the feedback.
Yes, i did it by hand.
Alright. let me read more around it and will make the change in containers/common project.
I will go through the code once again and will try to add test if i can, otherwise will do as you suggested. |
Created #1491 in containers/common for vendor specific code.
|
The java reproducer works fine for me on the latest buildah version. I don't understand what this is trying to achieve? The hostname is already in /etc/hosts in the current version so the reporducer works fine. |
@Luap99 I tried to reproduce the problem and i am able to reproduce it even with the current version. Problem is it's unable to resolve hostname because for buildah intermediate RUN step hostname is containerId itself and there is no entry for ip->contatinerId mapping in /etc/hosts. I see there is an entry for ip->host.containers.internal but not for ip-> containerId. |
@Luap99
|
How do you run buildah? Is this rootless or as root? Do you have any containers.conf options changed, i.e. The hostname is already added here: Lines 1223 to 1236 in 7468369
|
Yes.. it's rootless. I already printed the env. in the above comment.
|
You are running inside a container? Check the containers.conf file in that container, I assume you run with host networking. It should work if you set We currently do not add the hostname in /etc/hosts when host networking is used, the same goes for podman. |
Yes, i am running rootless buildah inside a container to build apps.
yes, i want to run with --network host option. I didn't find anything in containers.conf, i believe buildah uses --network host by default if we don't specify anything.
I tried both --network private and --network host but getting the same problem.
Alright. Any reason for that.
oks.. Got it. but i am thinking when intermediate build hostname is containerId shouldn't the mapping be
|
@Luap99 did you get a chance to read my comments. I would really appreciate your guidance here. We don't want to maintain the patch on buildah in our codebase and it should help others as well who are trying to build gradle/jave applications and using rootless buildah in a container. |
I don't see that behaviour, I can confirm the the host entry is missing for --network host case but in the normal case it should really be there. That is something we can and maybe should address. I think in your specific case the problem is you are running in a container that has When using cli flags this errors out:
However when using the env var this does not produce an error, bug!
You haven't shown what image you are using but the newer openjdk versions I tried always worked correctly regardless of the entry in /etc/hosts. The hostname is stored in /etc/hostname or can be retired via gethostname() syscall after all so I have no clue why the jdk even starts reading /etc/hosts. Anyway I am well aware that switching jdk versions may not be possible for you and the missing entry can be considered a bug regardless of what java is doing. |
@SaurabhAhuja1983 Can you give #4869 a try? I think this should solve your problem. |
@Luap99 Thank you for submitting #4869 I will give it a try. Yes, you are right, i am using buildah_isolation=chroot but i am also using --network=host. I have used public openjdk image to reproduce the problem... Please use the dockerfile and MyClass.java and run rootless buildah in a container, you should be able to reproduce the problem. 1. As you can from the output below see the only problem is As long as i am able to resolve hostname, i think we are good.
Dockerfile
MyClass.java
Env
|
@Luap99 Just verified #4869 and it worked fine. Thank you for fixing it. I can see hostname entry in /etc/hosts
Please do update once #4869 is merged to main and when new version is released. Here is the output for the same Dockerfile and MyClass.java
|
PR needs rebase. 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. |
It's taken care here #4869 |
What type of PR is this?
#4446
What this PR does / why we need it:
To fix java builds. Hostname (containerId) was unable to resolve.
How to verify it
Run the script provided here before and after the fix.
#4446 (comment)
Which issue(s) this PR fixes:
#4446
Fixes #4446
Special notes for your reviewer:
I would appreciate if you can provide any feedback, this is my first commit to buildah
Does this PR introduce a user-facing change?
No