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.

Sign up for GitHub

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

Jump to bottom

System property 'jibSerialize' causes build steps to be executed serially #682

Merged
merged 7 commits into from
Jul 24, 2018
Merged

System property 'jibSerialize' causes build steps to be executed serially #682

merged 7 commits into from
Jul 24, 2018

Conversation

briandealwis
Copy link
Member

For #681. If system property jibSerialize is set then we run all build steps on the main thread.

Our lifecycle messages don't change however — I still see lifecycle messages before any HTTP traffic begins, and we still have some operations happening in parallel (you can see requests for distroless/java and to registry.hub.docker), suggesting that we still have some parallelism happening without the StepsRunner.

[INFO] --- jib-maven-plugin:0.9.8-SNAPSHOT:build (default-cli) @ first ---
[WARNING] Base image 'gcr.io/distroless/java' does not use a specific image digest - build may not be reproducible
[INFO] 
[INFO] Containerizing application to foo/bar/baz...
[INFO] 
[INFO] Retrieving registry credentials for registry.hub.docker.com...
[INFO] Getting base image gcr.io/distroless/java...
[INFO] Building dependencies layer...
[INFO] Building classes layer...
[INFO] Building resources layer...
Jul 20, 2018 2:25:29 PM com.google.api.client.http.HttpRequest execute
CONFIG: -------------- REQUEST  --------------
GET https://gcr.io/v2/distroless/java/manifests/latest
Accept: application/vnd.oci.image.manifest.v1+json,application/vnd.docker.distribution.manifest.v2+json,application/vnd.docker.distribution.manifest.v1+json
Accept-Encoding: gzip
User-Agent: jib 0.9.8-SNAPSHOT jib-maven-plugin Google-HTTP-Java-Client/1.23.0 (gzip)

Jul 20, 2018 2:25:29 PM com.google.api.client.http.HttpRequest execute
CONFIG: curl -v --compressed -H 'Accept: application/vnd.oci.image.manifest.v1+json,application/vnd.docker.distribution.manifest.v2+json,application/vnd.docker.distribution.manifest.v1+json' -H 'Accept-Encoding: gzip' -H 'User-Agent: jib 0.9.8-SNAPSHOT jib-maven-plugin Google-HTTP-Java-Client/1.23.0 (gzip)' -- 'https://gcr.io/v2/distroless/java/manifests/latest'
Jul 20, 2018 2:25:29 PM com.google.api.client.http.HttpRequest execute
CONFIG: -------------- REQUEST  --------------
GET https://registry.hub.docker.com/v2/
Accept: 
Accept-Encoding: gzip
User-Agent: jib 0.9.8-SNAPSHOT jib-maven-plugin Google-HTTP-Java-Client/1.23.0 (gzip)

coollog
coollog reviewed Jul 20, 2018
View reviewed changes
jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/StepsRunner.java Outdated
@@ -76,6 +76,12 @@ public StepsRunner(
this.sourceFilesConfiguration = sourceFilesConfiguration;
this.baseLayersCache = baseLayersCache;
this.applicationLayersCache = applicationLayersCache;

ExecutorService executorService =
System.getProperty("jibSerialized") == null
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we check for a specific value of jibSerialized for the serialized case (to avoid someone accidentally using serialized when not intentional)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Turned it into a boolean property that must be true.

coollog
coollog reviewed Jul 20, 2018
View reviewed changes
jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/StepsRunner.java Outdated
ExecutorService executorService =
System.getProperty("jibSerialized") == null
? Executors.newCachedThreadPool()
: MoreExecutors.newDirectExecutorService();
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice and simple :)

@briandealwis
Copy link
Member Author

#685 addresses some of the log interleaving. So this is ready.

@briandealwis
Copy link
Member Author

/me grumbles about having to continually merge head

@briandealwis briandealwis merged commit 237bf42 into master Jul 24, 2018
@briandealwis briandealwis deleted the serialize branch July 24, 2018 17:09
@TadCordle TadCordle mentioned this pull request Jul 24, 2018
Closed
chanseokoh
chanseokoh reviewed Jul 31, 2018
View reviewed changes
jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/StepsRunner.java
@@ -76,6 +76,12 @@ public StepsRunner(
this.sourceFilesConfiguration = sourceFilesConfiguration;
this.baseLayersCache = baseLayersCache;
this.applicationLayersCache = applicationLayersCache;

ExecutorService executorService =
Boolean.getBoolean("jibSerialized")
Copy link
Member

@chanseokoh chanseokoh Jul 31, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@briandealwis is this supposed to be jibSerialize or jibSerialized? If the latter, please update the title of the PR. Can you also update the CHANGELOG files?

@briandealwis
Copy link
Member Author

Hmm, it should be jibSerialize, I suppose.

@briandealwis briandealwis mentioned this pull request Jul 31, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chanseokoh chanseokoh chanseokoh left review comments

@coollog coollog coollog approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@briandealwis @coollog @chanseokoh