-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Write out the image metadata to a file after build. #2227
Write out the image metadata to a file after build. #2227
Conversation
If the maven plugin is configured with an `imageName` output path: ```xml <configuration> <to> <image>gcr.io/some/path/to/image:tag</image> </to> <outputPaths> <imageName>target/jib-image.name</imageName> </outputPaths> </configuration> ``` The jib-maven-plugin will now create a file at that path with the image name during the build, like so: ``` $ mvn jib:build ... $ cat target/jib-image.name gcr.io/some/path/to/image:tag ``` This is handy if you generate the image name or tag in a dynamic fashion (eg, including a timstamp) and you have other tools in a CD pipeline that operate on the generated image. In our case, we would like to use the reference to the generated image to deploy to K8S, and our deployment tooling picks it up from a file. I have only implemented support in the maven plugin for now, since that is what we are using and I'm not really familiar with Gradle. That should be pretty straight-forward to add, if need be. When it comes to the implementation, I imitated how the image id and digests are written out for now. Changing to a json or yaml output format felt like a bigger (breaking) change. Related issue: GoogleContainerTools#1875
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.
I'm not sure this is something we want to do. I see the usecase, but this can be solved in plugins already - you can write a custom gradle task to do this, or use something like the Maven ant-run plug-in. It just feels strange to write out an input directly without modifying it.
@loosebazooka Agree that there are workarounds using other maven/gradle plugins. Two things I'm wary about with that approach:
How about combining the repository, digests, tags etc into a single |
@mbruggmann these are good points. I think we're still working out how we want jib-image.json to work, but this might be a good time to do it. |
@mbruggmann the changes look nice, I'll take a deeper look, can you also output the extra tags in the json file? |
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! Two little nits that I saw.
...gins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/ImageMetadataOutput.java
Outdated
Show resolved
Hide resolved
.thenReturn(mockJibContainer); | ||
testJibBuildRunner.writeImageJson(outputPath).runBuild(); | ||
|
||
final String outputJson = new String(Files.readAllBytes(outputPath), StandardCharsets.UTF_8); |
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.
I think it would be better to compare this output explicitly as a string to ensure we're producing expected output. (For example, to detect a situation where we change JSON libraries that produces slightly-invalid JSON.)
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.
Had a look at this. Beyond putting the full expected json into a string in the testcase (which seems highly specific) I didn't find a good way of doing that.
As of now, the next line of ImageMetadataOutput.fromJson(outputJson)
will at least make sure it properly parses into the expected type so it should catch any invalid JSON, serialization/deserialization differences and similar.
Added support for tags now if you'd like to take another look @loosebazooka @briandealwis |
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!
jib-core/src/test/java/com/google/cloud/tools/jib/api/JibContainerTest.java
Outdated
Show resolved
Hide resolved
...gins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/ImageMetadataOutput.java
Outdated
Show resolved
Hide resolved
Probably kind of annyoing, given our integration tests only run on internal machines (and you can't see the output). But can you run the integration tests yourself and fix the issues? |
The integration-tests caught an actual bug: the code was relying on |
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.
Thanks for your contribution!
The jib-maven-plugin will now create a json file with image
metadata during the build, like so:
This is handy if you generate the image name or tag in a dynamic
fashion (eg, including a timstamp) and you have other tools in a
CD pipeline that operate on the generated image. In our case, we
would like to use the reference to the generated image to deploy to
K8S, and our deployment tooling picks it up from a file.
The path to the output file is configurable in
outputPaths.imageJson
.Related issue: #1875