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

Bmoric/remove docker compose for build #7500

Merged
merged 47 commits into from
Nov 5, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
0f06d95
Remove the use of docker compose to build
benmoriceau Oct 29, 2021
e85f1a9
Update .dockerignore
benmoriceau Oct 29, 2021
dbbb298
Add explicit dependencies
benmoriceau Oct 29, 2021
9b00dc5
Allign .dockerfileingnore, rm implicit dependencies
benmoriceau Oct 29, 2021
0f049e7
improve scheduler gradle
benmoriceau Nov 1, 2021
8957cd7
Clean up migration gradle file
benmoriceau Nov 1, 2021
e3dd246
Clean up server gradle
benmoriceau Nov 1, 2021
8f83ea8
Merge branch 'master' of github.com:airbytehq/airbyte into bmoric/rem…
benmoriceau Nov 1, 2021
5ab190c
tmp
benmoriceau Nov 1, 2021
7d32b90
Look at diff
benmoriceau Nov 1, 2021
d693c5c
rm unwanted files
benmoriceau Nov 1, 2021
8084031
Make sure that we don't commit file that will be copied to the docker…
benmoriceau Nov 1, 2021
e63c0f2
Make the scheduler docker build to be increamental
benmoriceau Nov 1, 2021
1b6bc9b
Make the worker docker build to be incremental
benmoriceau Nov 1, 2021
3b9efe9
make a the server docker build to be incremental
benmoriceau Nov 1, 2021
5b3f566
Make the airbyte-migration to be incremental
benmoriceau Nov 1, 2021
dee7b07
Make the webapp docker build incremental
benmoriceau Nov 1, 2021
2976178
Make the config init docker build to be incremental
benmoriceau Nov 1, 2021
cb538db
Remove the dockerignorfile
benmoriceau Nov 1, 2021
fecb366
remove unused gradle block
benmoriceau Nov 1, 2021
71e79da
Make the airbyte cli docker build incremental
benmoriceau Nov 1, 2021
24a7cf8
Rm the use of composeBuild
benmoriceau Nov 1, 2021
c9251d9
Fix cli docker image name
benmoriceau Nov 2, 2021
d12c485
Merge branch 'master' of github.com:airbytehq/airbyte into bmoric/rem…
benmoriceau Nov 2, 2021
07d67f6
Fix migration build
benmoriceau Nov 2, 2021
b8bbbab
Merge branch 'master' of github.com:airbytehq/airbyte into bmoric/rem…
benmoriceau Nov 2, 2021
23ed511
Use the same version than prod
benmoriceau Nov 2, 2021
3acd34e
Add missing version update in the airbyte scheduler
benmoriceau Nov 2, 2021
d279ca2
Move all the docker build related files to the build folder
benmoriceau Nov 2, 2021
b32056c
Merge branch 'master' of github.com:airbytehq/airbyte into bmoric/rem…
benmoriceau Nov 2, 2021
d53150c
Remove outdated file updates
benmoriceau Nov 2, 2021
0c47eb7
Fix workers project
benmoriceau Nov 2, 2021
014a29a
Merge branch 'master' of github.com:airbytehq/airbyte into bmoric/rem…
benmoriceau Nov 2, 2021
cba2bce
update config docker build
benmoriceau Nov 2, 2021
fa899f9
Pr comments and factorization example
benmoriceau Nov 2, 2021
43bb8b0
Single definition of the incremental docker tasks
benmoriceau Nov 3, 2021
a52643f
Add documentation
benmoriceau Nov 3, 2021
b0ae593
Merge branch 'master' of github.com:airbytehq/airbyte into bmoric/rem…
benmoriceau Nov 3, 2021
8a3b1fc
test
benmoriceau Nov 3, 2021
317ae87
Make the factorize incremental to work
benmoriceau Nov 3, 2021
86b22b2
Merge branch 'master' of github.com:airbytehq/airbyte into bmoric/rem…
benmoriceau Nov 3, 2021
d64750a
Update docs and remove unused import
benmoriceau Nov 3, 2021
d42857a
Fix build and remove unused pluggins
benmoriceau Nov 3, 2021
24fd33c
Update doc based on PR comments
benmoriceau Nov 4, 2021
2bf9b18
Merge branch 'master' of github.com:airbytehq/airbyte into bmoric/rem…
benmoriceau Nov 4, 2021
0a378cc
Merge branch 'master' of github.com:airbytehq/airbyte into bmoric/rem…
benmoriceau Nov 5, 2021
1d3d22f
format
benmoriceau Nov 5, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions airbyte-config/init/.dockerignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
*
!src
!scripts
!build
!Dockerfile
14 changes: 14 additions & 0 deletions airbyte-config/init/build.gradle
Original file line number Diff line number Diff line change
@@ -1,9 +1,23 @@
import com.bmuschko.gradle.docker.tasks.image.DockerBuildImage
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I forget to update this file before moving it to ready to be review.


plugins {
id 'java'
id 'com.bmuschko.docker-remote-api'
}

dependencies {
implementation 'commons-cli:commons-cli:1.4'

implementation project(':airbyte-config:models')
}

def buildTag = System.getenv('VERSION') ?: 'dev'
// Use task types
task buildMyAppImage(type: DockerBuildImage) {
dependsOn compileTestJava
Copy link
Contributor

Choose a reason for hiding this comment

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

Are the test-related tasks still necessary to include when using the docker subdirectory? They shouldn't impact the contents anymore.

dependsOn jar
dependsOn processTestResources
inputDir = file('.')
images.add("airbyte/init:$buildTag")
}
build.dependsOn(buildMyAppImage)
2 changes: 2 additions & 0 deletions airbyte-db/lib/.dockerignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
*
!src
!Dockerfile
!build
14 changes: 14 additions & 0 deletions airbyte-db/lib/build.gradle
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import com.bmuschko.gradle.docker.tasks.image.*
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import com.bmuschko.gradle.docker.tasks.image.*
import com.bmuschko.gradle.docker.tasks.image.DockerBuildImage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


plugins {
id 'java-library'
id 'com.bmuschko.docker-remote-api'
}

dependencies {
Expand Down Expand Up @@ -69,3 +72,14 @@ task(dumpJobsSchema, dependsOn: 'classes', type: JavaExec) {
classpath = sourceSets.main.runtimeClasspath
args 'jobs', 'dump_schema'
}

def buildTag = System.getenv('VERSION') ?: 'dev'
// Use task types
task buildMyAppImage(type: DockerBuildImage) {
dependsOn compileTestJava
dependsOn jar
dependsOn processTestResources
inputDir = file('.')
images.add("airbyte/db:$buildTag")
}
build.dependsOn(buildMyAppImage)
1 change: 1 addition & 0 deletions airbyte-migration/.dockerignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
*
!src
!Dockerfile
!build
18 changes: 18 additions & 0 deletions airbyte-migration/build.gradle
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import com.bmuschko.gradle.docker.tasks.image.DockerBuildImage

plugins {
id 'application'
id 'com.bmuschko.docker-remote-api'
}

dependencies {
Expand All @@ -15,3 +18,18 @@ application {
mainClass = 'io.airbyte.migrate.MigrationRunner'
}

// Use task types
task buildMyAppImage(type: DockerBuildImage) {
dependsOn compileTestJava
dependsOn processResources
dependsOn processTestResources
dependsOn test
dependsOn jacocoTestReport
dependsOn distTar
dependsOn distZip
dependsOn jar
dependsOn startScripts
inputDir = file('.')
images.add("airbyte/migration:$version")
}
build.dependsOn(buildMyAppImage)
1 change: 1 addition & 0 deletions airbyte-scheduler/app/.dockerignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
*
!src
!Dockerfile
!build
23 changes: 23 additions & 0 deletions airbyte-scheduler/app/build.gradle
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import com.bmuschko.gradle.docker.tasks.image.*
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import com.bmuschko.gradle.docker.tasks.image.*
import com.bmuschko.gradle.docker.tasks.image.DockerBuildImage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


plugins {
id 'application'
id 'com.bmuschko.docker-remote-api'
}

dependencies {
Expand Down Expand Up @@ -57,3 +60,23 @@ run {
environment "TEMPORAL_HOST", "localhost:7233"

}

def buildTag = System.getenv('VERSION') ?: 'dev'
// Use task types
task buildMyAppImage(type: DockerBuildImage) {
dependsOn processResources
Copy link
Contributor

Choose a reason for hiding this comment

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

why does building the image depend on processTestResources, compileTestJava, test, jacocoTestReport? why wouldn't it just depend on what is required to create the artifacts. like, I'm wondering if it could just depend on assemble.

Copy link
Contributor

Choose a reason for hiding this comment

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

i'm asking this based on my understanding of the task dependency structure of the gradle java plugin: https://docs.gradle.org/current/userguide/java_plugin.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The plugin I am using has some implicit dependencies that will make the execution optimization to be ignore if the dependencies are not explicitly written. Like:

  - Gradle detected a problem with the following location: '/Users/benoitmoriceau/workspace/airbyte/airbyte-db/lib/build/reports/jacoco/test/html'. Reason: Task ':airbyte-db:lib:buildMyAppImage' uses this output of task ':airbyte-db:lib:jacocoTestReport' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed. Please refer to https://docs.gradle.org/7.2/userguide/validation_problems.html#implicit_dependency for more details about this problem.

I will try to only depend on build.

Copy link
Contributor Author

@benmoriceau benmoriceau Nov 1, 2021

Choose a reason for hiding this comment

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

@cgardens I tried to make it to depends only on jar and some optimization have gone away with because of that:

Execution optimizations have been disabled for task ':airbyte-scheduler:app:startScripts' to ensure correctness due to the following reasons:
  - Gradle detected a problem with the following location: '/Users/benoitmoriceau/workspace/airbyte/airbyte-scheduler/app/build/scripts'. Reason: Task ':airbyte-scheduler:app:buildMyAppImage' uses this output of task ':airbyte-scheduler:app:startScripts' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed. Please refer to https://docs.gradle.org/7.2/userguide/validation_problems.html#implicit_dependency for more details about this problem.

dependsOn processTestResources
dependsOn compileJava
dependsOn compileTestJava
dependsOn test
dependsOn jar
dependsOn startScripts
dependsOn distTar
dependsOn distZip
dependsOn jacocoTestReport
inputDir = file('.')
images.add("airbyte/scheduler:$buildTag")
noCache = false
quiet = false
}
assemble.dependsOn(buildMyAppImage)
1 change: 1 addition & 0 deletions airbyte-server/.dockerignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
*
!src
!Dockerfile
!build
31 changes: 31 additions & 0 deletions airbyte-server/build.gradle
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import com.bmuschko.gradle.docker.tasks.image.*
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import com.bmuschko.gradle.docker.tasks.image.*
import com.bmuschko.gradle.docker.tasks.image.DockerBuildImage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


plugins {
id 'application'
id 'maven-publish'
id 'com.github.johnrengelman.shadow' version '7.1.0'
id 'com.bmuschko.docker-remote-api'
}

shadowJar {
Expand Down Expand Up @@ -126,3 +129,31 @@ run {
environment "AIRBYTE_ROLE", System.getenv('AIRBYTE_ROLE')
environment "TEMPORAL_HOST", "localhost:7233"
}

def buildTag = System.getenv('VERSION') ?: 'dev'
// Use task types
task buildMyAppImage(type: DockerBuildImage) {
dependsOn copySeed
dependsOn processResources
dependsOn processTestResources
dependsOn jar
dependsOn shadowJar
dependsOn compileTestJava
dependsOn distTar
dependsOn distZip
dependsOn shadowDistTar
dependsOn shadowDistZip
dependsOn startScripts
dependsOn startShadowScripts
dependsOn test
dependsOn jacocoTestReport
inputDir = file('.')
images.add("airbyte/server:$buildTag")
}
build.dependsOn(buildMyAppImage)
startScripts.dependsOn(shadowJar)
Copy link
Contributor

Choose a reason for hiding this comment

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

These will all be cleaned up in the followup "make server build incemental" I hope?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hope too, I will remove them since that should be part of the other issue.

distTar.dependsOn(shadowJar)
distZip.dependsOn(shadowJar)
startShadowScripts.dependsOn(jar)
shadowDistTar.dependsOn(jar)
shadowDistZip.dependsOn(jar)
14 changes: 14 additions & 0 deletions airbyte-webapp/build.gradle
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import com.bmuschko.gradle.docker.tasks.image.*
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import com.bmuschko.gradle.docker.tasks.image.*
import com.bmuschko.gradle.docker.tasks.image.DockerBuildImage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


plugins {
id "base"
id "com.github.node-gradle.node" version "3.1.1"
id 'com.bmuschko.docker-remote-api'
}

def nodeVersion = System.getenv('NODE_VERSION') ?: '14.11.0'
Expand Down Expand Up @@ -40,3 +43,14 @@ task copyDocs(type: Copy) {

copyDocs.dependsOn npm_run_build
assemble.dependsOn copyDocs

// Use task types
task buildMyAppImage(type: DockerBuildImage) {
dependsOn copyDocs
dependsOn nodeSetup
dependsOn npmInstall
dependsOn npm_run_build
inputDir = file('.')
images.add("airbyte/webapp:$version")
}
build.dependsOn(buildMyAppImage)
4 changes: 4 additions & 0 deletions airbyte-workers/.dockerignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
*
!src
!Dockerfile
!build
19 changes: 19 additions & 0 deletions airbyte-workers/build.gradle
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import org.jsonschema2pojo.SourceType
import com.bmuschko.gradle.docker.tasks.image.*
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import com.bmuschko.gradle.docker.tasks.image.*
import com.bmuschko.gradle.docker.tasks.image.DockerBuildImage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


plugins {
id 'application'
id 'com.github.eirnym.js2p' version '1.0'
id 'airbyte-integration-test-java'
id 'com.bmuschko.docker-remote-api'
}

configurations {
Expand Down Expand Up @@ -62,3 +64,20 @@ application {
mainClass = mainClassName
applicationDefaultJvmArgs = ['-XX:MaxRAMPercentage=75.0']
}

def buildTag = System.getenv('VERSION') ?: 'dev'
// Use task types
task buildMyAppImage(type: DockerBuildImage) {
dependsOn compileTestJava
dependsOn distTar
dependsOn distZip
dependsOn jar
dependsOn processTestResources
dependsOn startScripts
dependsOn compileIntegrationTestJavaJava
dependsOn test
dependsOn jacocoTestReport
inputDir = file('.')
images.add("airbyte/worker:$buildTag")
}
assemble.dependsOn(buildMyAppImage)
70 changes: 30 additions & 40 deletions build.gradle
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
buildscript {
repositories {
maven {
url "https://plugins.gradle.org/m2/"
}
}
dependencies {
classpath 'com.bmuschko:gradle-docker-plugin:7.1.0'
}
}

plugins {
id 'base'
id 'pmd'
Expand Down Expand Up @@ -58,22 +69,22 @@ def createJavaLicenseWith = { license ->
// monorepo setup and it doesn't actually exclude directories reliably. This code makes the behavior predictable.
def createSpotlessTarget = { pattern ->
def excludes = [
'.gradle',
'node_modules',
'.eggs',
'.mypy_cache',
'.venv',
'*.egg-info',
'build',
'dbt-project-template',
'dbt-project-template-mssql',
'dbt-project-template-mysql',
'dbt-project-template-oracle',
'dbt_test_config',
'normalization_test_output',
'tools',
'secrets',
'charts' // Helm charts often have injected template strings that will fail general linting. Helm linting is done separately.
'.gradle',
'node_modules',
'.eggs',
'.mypy_cache',
'.venv',
'*.egg-info',
'build',
'dbt-project-template',
'dbt-project-template-mssql',
'dbt-project-template-mysql',
'dbt-project-template-oracle',
'dbt_test_config',
'normalization_test_output',
'tools',
'secrets',
'charts' // Helm charts often have injected template strings that will fail general linting. Helm linting is done separately.
]

if (System.getenv().containsKey("SUB_BUILD")) {
Expand Down Expand Up @@ -125,9 +136,9 @@ allprojects {
":airbyte-webapp",
].toSet().asImmutable()

if (project.getPath() in composeDeps) {
/*if (project.getPath() in composeDeps) {
composeBuild.dependsOn(project.getPath() + ':assemble')
}
}*/
}
}

Expand Down Expand Up @@ -233,6 +244,7 @@ subprojects {
testImplementation 'org.junit.jupiter:junit-jupiter-params:5.7.2'
testImplementation 'org.mockito:mockito-junit-jupiter:3.12.4'
testImplementation 'org.assertj:assertj-core:3.21.0'

}

tasks.withType(Tar) {
Expand All @@ -244,28 +256,6 @@ subprojects {
}
}

task composeBuild {
def buildTag = System.getenv('VERSION') ?: 'dev'
def buildPlatform = System.getenv('DOCKER_BUILD_PLATFORM') ?: 'linux/amd64'
def buildArch = System.getenv('DOCKER_BUILD_ARCH') ?: 'amd64'
def jdkVersion = System.getenv('JDK_VERSION') ?: '14.0.2'
Copy link
Contributor

Choose a reason for hiding this comment

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

We should try not to remove the functionality we have here.

Currently, someone following the instructions on https://docs.airbyte.io/contributing-to-airbyte/developing-locally#build-with-gradle to build for M1 won't be able to since these env vars aren't captured and don't make it to the docker build steps.

def dockerComposeFile = buildArch == 'arm64' ? 'docker-compose.build-m1.yaml' : 'docker-compose.build.yaml'
doFirst {
exec {
workingDir rootDir
commandLine 'docker-compose', '-f', dockerComposeFile, 'build', '--parallel', '--quiet'
environment 'VERSION', buildTag
environment 'DOCKER_BUILD_PLATFORM', buildPlatform
environment 'DOCKER_BUILD_ARCH', buildArch
environment 'JDK_VERSION', jdkVersion
}
}
}

if (!System.getenv().containsKey("SUB_BUILD") || System.getenv().get("SUB_BUILD") == "PLATFORM") {
build.dependsOn(composeBuild)
}

task('generate') {
dependsOn subprojects.collect { it.getTasksByName('generateProtocolClassFiles', true) }
dependsOn subprojects.collect { it.getTasksByName('generateJsonSchema2Pojo', true) }
Expand Down