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

Propagate architecture and os from base to target images #1564

Merged
merged 4 commits into from
Mar 19, 2019
Merged

Conversation

briandealwis
Copy link
Member

  • Adds architecture and os fields to Image/ImageBuilder and ContainerConfigurationTemplate, defaulting to current amd64/linux
  • Modifies ImageToJsonTranslator and JsonToImageTranslator to propagate architecture/os
  • Modifies BuildImageStep to propagate architecture/os from the base image to the built image
  • Adds some explicit tests, and modifies existing tests to us wasm/js to ensure we're not just picking up the default amd64/linux
  • Verified by hand by building against image s390x/openjdk:11-stretch

Note that this PR does not expose any facilities for changing the arch/os. I'll leave that to a separate PR.

Fixes #1547

@@ -172,6 +172,28 @@ public void setCreated(@Nullable String created) {
this.created = created;
}

/**
* Set the architecture for which this container was built. See the <a
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Sets (same for setOs javadoc)

@chanseokoh
Copy link
Member

Note that this PR does not expose any facilities for changing the arch/os. I'll leave that to a separate PR.

I don't think there is a need to change the arch/os. If they wanted a different arch/os, they should use the image with the arch/os they want. For example, arm64v8/openjdk or winmad64/openjdk.

Now that said, I think now this PR undermines reproducibility, just a bit. That is, when the base image is openjdk, Jib will build an AMD image on AMD and an ARM image on ARM. We need to think about this.

@chanseokoh
Copy link
Member

Jib will build an AMD image on AMD and an ARM image on ARM.

Actually, I'm not sure if this is the case with Jib. OTOH, I believe Docker build automatically pulls in the matching os/arch.

@briandealwis
Copy link
Member Author

I don't think it undermines reproducibility: we propagate the os/arch from the base image. Docker Hub doesn't silently remap between architectures, so we should get the same base image for the same image ref.

I can totally get that docker build may silently swap os/arch. I don't see how it can work otherwise since it will be executing binaries for the current system.

But we should document the difference.

@chanseokoh
Copy link
Member

I don't think it undermines reproducibility: we propagate the os/arch from the base image. Docker Hub doesn't silently remap between architectures, so we should get the same base image for the same image ref.

Ah, OK. That makes sense.

@briandealwis briandealwis merged commit 985e0a1 into master Mar 19, 2019
@briandealwis briandealwis added this to the v1.1.0 milestone Mar 19, 2019
@chanseokoh chanseokoh deleted the i1547 branch March 19, 2019 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants