You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2019/01/07 20:47:39 UTC
[kudu-CR] WIP: Add an intial docker integration
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12173
Change subject: WIP: Add an intial docker integration
......................................................................
WIP: Add an intial docker integration
This patch adds an initial Docker file which defines a
multistage build that supports creating various Docker
images for Apache Kudu development.
More orchestration work is needed to create useful
runtime environments that can be easily build and
uploaded to docker hub. Once that is done, more work
can be done to leverage these images in examples
and demos.
Some of the open tasks I am considering are:
- Add kudu-master and kudu-tserver images with ENTRYPOINT’s
- Add build.sh and upload.sh scripts for orchestration
- Add labels (version, description, etc)
- Handle Sanitizer build types.
- Support Ubuntu and SUSE (us a bootstrap-env.sh script)
- Finalize dockerignore
- Optimize image sizes
Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
---
A .dockerignore
A docker/Dockerfile
M thirdparty/build-definitions.sh
3 files changed, 217 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/12173/1
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
[kudu-CR] [docker] Add an initial docker build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Attila Bukor, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12173
to look at the new patch set (#3).
Change subject: [docker] Add an initial docker build
......................................................................
[docker] Add an initial docker build
This patch adds an initial Docker file which defines a
multistage build that supports creating various Docker
images for Apache Kudu development.
This work is experimental and the choices and structure
may change wih follow on commits.
Some things these images could be used for include:
- demos/examples
- pre-built binaries including thirdparty
- multi-os testing
- multi-os tools like build_mini_cluster_binaries.sh
- external integration testing
Some of the open tasks for the future are:
- Handle all build types.
- Optimize image sizes.
- Add kudu user to runtime images.
- Add an upload script with good tagging.
- Add healthchecks.
Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
---
A .dockerignore
A docker/Dockerfile
A docker/Dockerfile-files
A docker/README.adoc
A docker/bootstrap-env.sh
A docker/docker-build.sh
A docker/kudu-entrypoint.sh
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
9 files changed, 786 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/12173/3
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [docker] Add an initial docker build
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12173/2/thirdparty/build-thirdparty.sh
File thirdparty/build-thirdparty.sh:
http://gerrit.cloudera.org:8080/#/c/12173/2/thirdparty/build-thirdparty.sh@248
PS2, Line 248: # Install Hadoop, Hive, and Sentry by copying their source directories into $PREFIX/opt.
: # Copying allows the source directory to be removed once thirdparty is built.
> This will add ~400MB of space. But it allows me to save almost 2 GiB in the
Hmm, that's a fair amount of space relative to the size of the thirdparty install tree right now: counting both common and uninstrumented, we're sitting at 1.8 GB. Not to mention it appears to have broken dist-test in some way.
Can we keep the symlinks but special-case the docker build to remove them and copy the files in?
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Jan 2019 05:34:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Attila Bukor, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12173
to look at the new patch set (#2).
Change subject: [docker] Add an initial docker build
......................................................................
[docker] Add an initial docker build
This patch adds an initial Docker file which defines a
multistage build that supports creating various Docker
images for Apache Kudu development.
This work is experimental and the choices and structure
may change wih follow on commits.
Some things these images could be used for include:
- demos/examples
- pre-built binaries including thirdparty
- multi-os testing
- muti-os tools like build_mini_cluster_binaries.sh
- external integration testing
Some of the open tasks for the future are:
- Handle all build types.
- Optimize image sizes.
- Add kudu user to runtime images.
- Add an upload script with good tagging.
- Add healthchecks.
Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
---
A .dockerignore
A docker/Dockerfile
A docker/Dockerfile-files
A docker/README.adoc
A docker/bootstrap-env.sh
A docker/docker-build.sh
A docker/kudu-entrypoint.sh
M thirdparty/build-definitions.sh
M thirdparty/build-thirdparty.sh
9 files changed, 782 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/12173/2
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [docker] Add an initial docker build
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/bootstrap-env.sh
File docker/bootstrap-env.sh:
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/bootstrap-env.sh@30
PS3, Line 30: if [[ -n $(which yum) ]];
> why not just
Whoops, ignore this one.
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Jan 2019 18:20:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12173
to look at the new patch set (#5).
Change subject: [docker] Add an initial docker build
......................................................................
[docker] Add an initial docker build
This patch adds an initial Docker file which defines a
multistage build that supports creating various Docker
images for Apache Kudu development.
This work is experimental and the choices and structure
may change wih follow on commits.
Some things these images could be used for include:
- demos/examples
- pre-built binaries including thirdparty
- multi-os testing
- multi-os tools like build_mini_cluster_binaries.sh
- external integration testing
Some of the open tasks for the future are:
- Handle all build types.
- Optimize image sizes.
- Add kudu user to runtime images.
- Add an upload script with good tagging.
- Add healthchecks.
Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
---
A .dockerignore
A docker/Dockerfile
A docker/Dockerfile-files
A docker/README.adoc
A docker/bootstrap-env.sh
A docker/docker-build.sh
A docker/kudu-entrypoint.sh
M thirdparty/build-definitions.sh
8 files changed, 817 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/12173/5
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [docker] Add an initial docker build
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12173/6/docker/bootstrap-env.sh
File docker/bootstrap-env.sh:
http://gerrit.cloudera.org:8080/#/c/12173/6/docker/bootstrap-env.sh@165
PS6, Line 165: "Unsupported OS"
What about SLES? We don't want to support it at all or it's just coming later?
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Jan 2019 23:20:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12173
to look at the new patch set (#4).
Change subject: [docker] Add an initial docker build
......................................................................
[docker] Add an initial docker build
This patch adds an initial Docker file which defines a
multistage build that supports creating various Docker
images for Apache Kudu development.
This work is experimental and the choices and structure
may change wih follow on commits.
Some things these images could be used for include:
- demos/examples
- pre-built binaries including thirdparty
- multi-os testing
- multi-os tools like build_mini_cluster_binaries.sh
- external integration testing
Some of the open tasks for the future are:
- Handle all build types.
- Optimize image sizes.
- Add kudu user to runtime images.
- Add an upload script with good tagging.
- Add healthchecks.
Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
---
A .dockerignore
A docker/Dockerfile
A docker/Dockerfile-files
A docker/README.adoc
A docker/bootstrap-env.sh
A docker/docker-build.sh
A docker/kudu-entrypoint.sh
M thirdparty/build-definitions.sh
8 files changed, 817 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/12173/4
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [docker] Add an initial docker build
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/Dockerfile
File docker/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/Dockerfile@57
PS3, Line 57: #
: # ---- Thirdparty ----
: # Builds an image that has Kudu's thirdparty dependencies built.
: # This is done in it's own stage so that docker can cache it and only
: # run it when thirdparty has changes.
: #
While not necessary, maybe we should consider installing ccache/ninja (though maybe as a follow-on patch if it turns out being a headache). This could be super useful as a developer tool wanting to develop/test on different environments, so it might be helpful to include iterative dev tools out of the box.
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/bootstrap-env.sh
File docker/bootstrap-env.sh:
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/bootstrap-env.sh@28
PS3, Line 28: # Install the prerequisite libraries, if they are not installed.
I know it's not in the upstream installation docs, but it's probably worth adding nscd, or a TODO to add it in case people use this and run into issues:
https://www.cloudera.com/documentation/enterprise/latest/topics/kudu_troubleshooting.html#slow_dns_nscd
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Jan 2019 19:51:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12173/5/docker/bootstrap-env.sh
File docker/bootstrap-env.sh:
http://gerrit.cloudera.org:8080/#/c/12173/5/docker/bootstrap-env.sh@25
PS5, Line 25: # https://www.cloudera.com/documentation/enterprise/latest/topics/kudu_troubleshooting.html#slow_dns_nscd
> Let's link to upstream docs instead: https://kudu.apache.org/docs/troublesh
Done
http://gerrit.cloudera.org:8080/#/c/12173/5/docker/bootstrap-env.sh@85
PS5, Line 85: then
> Nit: add a space before
Done
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Jan 2019 22:48:42 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12173/5/docker/bootstrap-env.sh
File docker/bootstrap-env.sh:
http://gerrit.cloudera.org:8080/#/c/12173/5/docker/bootstrap-env.sh@25
PS5, Line 25: # https://www.cloudera.com/documentation/enterprise/latest/topics/kudu_troubleshooting.html#slow_dns_nscd
Let's link to upstream docs instead: https://kudu.apache.org/docs/troubleshooting.html#slow_dns_nscd
http://gerrit.cloudera.org:8080/#/c/12173/5/docker/bootstrap-env.sh@85
PS5, Line 85: then
Nit: add a space before
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Jan 2019 22:45:27 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12173
to look at the new patch set (#6).
Change subject: [docker] Add an initial docker build
......................................................................
[docker] Add an initial docker build
This patch adds an initial Docker file which defines a
multistage build that supports creating various Docker
images for Apache Kudu development.
This work is experimental and the choices and structure
may change wih follow on commits.
Some things these images could be used for include:
- demos/examples
- pre-built binaries including thirdparty
- multi-os testing
- multi-os tools like build_mini_cluster_binaries.sh
- external integration testing
Some of the open tasks for the future are:
- Handle all build types.
- Optimize image sizes.
- Add kudu user to runtime images.
- Add an upload script with good tagging.
- Add healthchecks.
Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
---
A .dockerignore
A docker/Dockerfile
A docker/Dockerfile-files
A docker/README.adoc
A docker/bootstrap-env.sh
A docker/docker-build.sh
A docker/kudu-entrypoint.sh
M thirdparty/build-definitions.sh
8 files changed, 817 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/12173/6
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [docker] Add an initial docker build
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 8: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile
File docker/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@79
PS7, Line 79: /*
> Given "hidden" directories starting with '.' actually never exist, are you
That was just a nit, frankly. At my machine I used to rename some of the old builds like that in the thirdparty. However, since our build doesn't do anything like that, I'm OK with keeping it as is.
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 8
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 25 Jan 2019 23:06:27 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 6: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Jan 2019 22:51:49 +0000
Gerrit-HasComments: No
[kudu-CR] [docker] Add an initial docker build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 3:
(17 comments)
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile
File docker/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile@71
PS2, Line 71: # TODO: Remove build dir too?
> Right, that's the issue I ran into and I didn't try to fix it yet. Being ab
I removed all but the llvm directory.
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/Dockerfile
File docker/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/Dockerfile@32
PS3, Line 32: ARG BASE_OS=ubuntu:xenial
> I'm curious, why do we default to ubuntu:xenial? In patch set one we used c
ubuntu seems to be a more common choice among ecosystem images (impala for example) so I set it as the default now that it is supported. My next patch update adds more support.
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/Dockerfile@57
PS3, Line 57: #
: # ---- Thirdparty ----
: # Builds an image that has Kudu's thirdparty dependencies built.
: # This is done in it's own stage so that docker can cache it and only
: # run it when thirdparty has changes.
: #
> While not necessary, maybe we should consider installing ccache/ninja (thou
I had considered this but wanted to keep things minimal.
ccache and ninja are likely most useful on the kudu-build image which I imagine would be used as psuedo dev/experimentation containers. Otherwise the standard build tools/process will be used and the cache will be fresh each build unless we setup volumes.
Lets address this in a follow up change.
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/README.adoc
File docker/README.adoc:
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/README.adoc@72
PS3, Line 72:
: === kudu-master
: A runtime image with kudu-master as the ENTRYPOINT.
: Uses the kudu-runtime image as a base.
:
: === kudu-tserver
: A runtime image with kudu-tserver as the ENTRYPOINT.
: Uses the kudu-runtime image as a base.
> You might not have run into this if your docker version is new, but it migh
I think I workaround this by setting `--use_hybrid_clock=false` so that `ntp` isn't needed. That is something that will be addressed when those TODO's are done.
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/bootstrap-env.sh
File docker/bootstrap-env.sh:
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/bootstrap-env.sh@28
PS3, Line 28: # Install the prerequisite libraries, if they are not installed.
> I know it's not in the upstream installation docs, but it's probably worth
I added this as a todo. This patch is large and will have many improvements going forward.
I have found and included a few things that are not in the upstream docs and planned to add them at some point. Alternatively we could point people to this script to setup their environment, though I am not sure we want to mix it's purpose.
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/bootstrap-env.sh@58
PS3, Line 58: \
> drop
Why drop? Not sure what this means?
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/bootstrap-env.sh@36
PS3, Line 36: autoconf \
: automake \
: cyrus-sasl-devel \
: cyrus-sasl-gssapi \
: cyrus-sasl-plain \
: flex \
: gcc \
: gcc-c++ \
: gdb \
: git \
: java-1.8.0-openjdk-devel \
: krb5-server \
: krb5-workstation \
: libtool \
: make \
: openssl-devel \
: patch \
: pkgconfig \
: redhat-lsb-core \
: rsync \
: unzip \
: vim-common \
: whic
> nit here and below: add an extra indent for the continuation of the command
Done
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/docker-build.sh
File docker/docker-build.sh:
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/docker-build.sh@37
PS3, Line 37: --build-arg MAINTAINER="Apache Kudu<de...@kudu.apache.org>"
> nit: I think there should be a space between the name and the email
Done
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/docker-build.sh@45
PS3, Line 45: docker build "${BUILD_ARGS[@]}" -f $ROOT/docker/Dockerfile --target kudu-base -t kudu-base $ROOT
> I believe the tag should be kudu:* instead of kudu-* and each image should
One of my open follow up items listed in the commit message is "Add an upload script with good tagging."
I have a lot of options/thoughts on tagging, but given others might too I wanted to handle that in a separate patch.
I think tagging and uploading thirdparty is useful because you can pull a pre-built thirdparty instead of building it yourself. The same is true for the kudu-build. You can pull it and use it to copy any artifacts out for a derived image.
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/kudu-entrypoint.sh
File docker/kudu-entrypoint.sh:
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/kudu-entrypoint.sh@27
PS3, Line 27: resolved
> ... reached by their DNS name ...
This was an easy way to get past the failures mentioned in the doc below. We can improve it going forward.
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/kudu-entrypoint.sh@51
PS3, Line 51: /var/lib/kudu/master
> Turn into a variable to re-use elsewhere?
I don't want to over engineer this yet. It's doesn't have a lot of real use and will surely evolve in my follow on patches.
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/kudu-entrypoint.sh@52
PS3, Line 52: $KUDU_FLAGS
> KUDU_FLAGS is common for both kudu-master and kudu-tservers. Does it make
Based on current projected usage, in docker compose files, it's defined per service so this is okay. It can change over time if needed. This is experimental right now.
good, point. I can move it to the end.
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/kudu-entrypoint.sh@55
PS3, Line 55: /opt/kudu/www
> Is it guaranteed to exist? Or that doesn't matter?
same as other locations.
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/kudu-entrypoint.sh@62
PS3, Line 62: /var/lib/kudu/tserver
> Turn into a variable to re-use elsewhere?
same as above.
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/kudu-entrypoint.sh@66
PS3, Line 66: /opt/kudu/www
> Is it guaranteed to exist? Or that doesn't matter?
yes it's guaranteed. The kudu-runtime image places it there.
http://gerrit.cloudera.org:8080/#/c/12173/2/thirdparty/build-thirdparty.sh
File thirdparty/build-thirdparty.sh:
http://gerrit.cloudera.org:8080/#/c/12173/2/thirdparty/build-thirdparty.sh@248
PS2, Line 248: # Install Hadoop, Hive, and Sentry by copying their source directories into $PREFIX/opt.
: # Copying allows the source directory to be removed once thirdparty is built.
> Hmm, that's a fair amount of space relative to the size of the thirdparty i
Done
http://gerrit.cloudera.org:8080/#/c/12173/3/thirdparty/build-thirdparty.sh
File thirdparty/build-thirdparty.sh:
http://gerrit.cloudera.org:8080/#/c/12173/3/thirdparty/build-thirdparty.sh@252
PS3, Line 252: rm -rf $PREFIX/opt/hadoop
> nit: I'm not sure removing data automatically is a good thing to do. Maybe
I undid this change.
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Jan 2019 20:17:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile
File docker/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@79
PS7, Line 79: /*
> The example is below:
Ah, and it requires '-mindepth 1' then.
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 25 Jan 2019 22:03:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile
File docker/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@79
PS7, Line 79: /*
> Ah, and it requires '-mindepth 1' then.
To match that '-maxdepth 0' with removed globing, that should be replaced with '-mindepth 1 -maxdepth 1'.
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 25 Jan 2019 22:07:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 3:
(3 comments)
I tried to build it on my Mac and kudu-thirdparty fails for some reason:
The command '/bin/sh -c thirdparty/build-if-necessary.sh && rm -rf thirdparty/src' returned a non-zero code: 137
Chargers-MacBook-Pro:kudu charger$
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/Dockerfile
File docker/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/Dockerfile@32
PS3, Line 32: ARG BASE_OS=ubuntu:xenial
I'm curious, why do we default to ubuntu:xenial? In patch set one we used centos7.
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/docker-build.sh
File docker/docker-build.sh:
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/docker-build.sh@37
PS3, Line 37: --build-arg MAINTAINER="Apache Kudu<de...@kudu.apache.org>"
nit: I think there should be a space between the name and the email
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/docker-build.sh@45
PS3, Line 45: docker build "${BUILD_ARGS[@]}" -f $ROOT/docker/Dockerfile --target kudu-base -t kudu-base $ROOT
I believe the tag should be kudu:* instead of kudu-* and each image should have a tag with and one without the version, e.g.
"... -t kudu:thirdparty -t kudu:$VERSION-thirdparty kudu:latest-thirdparty $ROOT"
We should probably tag the kudu-runtime image without the runtime part to serve as a "default" image:
"... --target kudu-runtime -t kudu:latest -t kudu:$VERSION $ROOT"
Also I'm not convinced we need to tag kudu-thirdparty and kudu-build. It's good that they're in separate stages and If anyone wants to build them for some reason, they can still do it.
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Jan 2019 12:19:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 3:
If your build failed with exit code 137 it's likely not due to the build but the memory available to docker not being enough. See here: https://success.docker.com/article/what-causes-a-container-to-exit-with-code-137
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Jan 2019 14:21:11 +0000
Gerrit-HasComments: No
[kudu-CR] [docker] Add an initial docker build
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 7:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12173/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12173/7//COMMIT_MSG@10
PS7, Line 10: various Docker
: images
> It's in the code and "supported" isn't really a thing yet anyway given this
OK
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile
File docker/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@31
PS7, Line 31: TODO
> I actually really dislike the TODO(user) model. Given this is an open sourc
Sometimes annotate/blame can be not very straightforward when the code moves around (file renaming, moving just code around). If you don't like that TODO(user), that's totally fine with me.
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@79
PS7, Line 79: /*
> Not sure what you mean. This isn't leaving any subdirectories other than th
The example is below:
dhcp-10-16-3-47:tmp$ find /tmp/m
/tmp/m
/tmp/m/.x
/tmp/m/a
/tmp/m/x
dhcp-10-16-3-47:tmp$ find /tmp/m/*
/tmp/m/a
/tmp/m/x
So, my question is: do you need that '*' globing to keep directories like '.x' (if any exist)? If not, then I suggest to drop that useless '*' globing.
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 25 Jan 2019 21:59:32 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP: Add an intial docker integration
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has removed a vote on this change.
Change subject: WIP: Add an intial docker integration
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [docker] Add an initial docker build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 2:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/12173/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12173/2//COMMIT_MSG@20
PS2, Line 20: muti
> multi
Done
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile
File docker/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile@71
PS2, Line 71: # TODO: Remove build dir too?
> In theory you could remove thirdparty/build too, except that the clang-tool
Right, that's the issue I ran into and I didn't try to fix it yet. Being able to remove this directory would make the docker image ~2.5 GiB smaller.
Here is the code comment that explains why it's there:
> # Create a link from Clang to thirdparty/clang-toolchain. This path is used
# for all Clang invocations. The link can't point to the Clang installed in
# the prefix directory, since this confuses CMake into believing the
# thirdparty prefix directory is the system-wide prefix, and it omits the
# thirdparty prefix directory from the rpath of built binaries.
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile@110
PS2, Line 110: COPY ./docs/support docs/support
This is actually used in the standard build, without it the build below fails. The first failure is:
> CMake Error: File /kudu/docs/support/doxygen/client_api.doxy.in does not exist.
> CMake Error at CMakeLists.txt:1418 (configure_file):
configure_file Problem configuring file
It looks like if doxygen is installed it is required, otherwise it's ignored. I installed doxygen in the base image so it could be used to build docs if needed.
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile-files
File docker/Dockerfile-files:
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile-files@18
PS2, Line 18: # This docker file is a minimal build usefull for debugging
> useful
Done
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/kudu-entrypoint.sh
File docker/kudu-entrypoint.sh:
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/kudu-entrypoint.sh@49
PS2, Line 49: KUDU_OPTS="$KUDU_FLAGS \
> Is the idea that KUDU_FLAGS can be set by users to do more configuration? M
Done
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/kudu-entrypoint.sh@52
PS2, Line 52: --fs_data_dirs=/var/lib/kudu/master \
> You can omit this if the intent is to use the same value as --fs_wal_dir. B
Done
http://gerrit.cloudera.org:8080/#/c/12173/2/thirdparty/build-thirdparty.sh
File thirdparty/build-thirdparty.sh:
http://gerrit.cloudera.org:8080/#/c/12173/2/thirdparty/build-thirdparty.sh@248
PS2, Line 248: # Install Hadoop, Hive, and Sentry by copying their source directories into $PREFIX/opt.
: # Copying allows the source directory to be removed once thirdparty is built.
> How much additional disk space will this consume?
This will add ~400MB of space. But it allows me to save almost 2 GiB in the image by removing the source after the build.
This change does break the build, I will add a call to ensure the existing directory is wiped before installing.
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Jan 2019 03:51:27 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/bootstrap-env.sh
File docker/bootstrap-env.sh:
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/bootstrap-env.sh@58
PS3, Line 58: \
> That meant the ending backslash is not needed for the very last line of the
Ah cool, my latest patches took care of that.
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Jan 2019 23:15:53 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
[docker] Add an initial docker build
This patch adds an initial Docker file which defines a
multistage build that supports creating various Docker
images for Apache Kudu development.
This work is experimental and the choices and structure
may change with follow on commits.
Some things these images could be used for include:
- demos/examples
- pre-built binaries including thirdparty
- multi-os testing
- multi-os tools like build_mini_cluster_binaries.sh
- external integration testing
Some of the open tasks for the future are:
- Handle all build types.
- Optimize image sizes.
- Add kudu user to runtime images.
- Add an upload script with good tagging.
- Add healthchecks.
Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Reviewed-on: http://gerrit.cloudera.org:8080/12173
Tested-by: Kudu Jenkins
Reviewed-by: Attila Bukor <ab...@apache.org>
Tested-by: Attila Bukor <ab...@apache.org>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
A .dockerignore
A docker/Dockerfile
A docker/Dockerfile-files
A docker/README.adoc
A docker/bootstrap-env.sh
A docker/docker-build.sh
A docker/kudu-entrypoint.sh
M thirdparty/build-definitions.sh
8 files changed, 817 insertions(+), 0 deletions(-)
Approvals:
Kudu Jenkins: Verified
Attila Bukor: Looks good to me, but someone else must approve; Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 9
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [docker] Add an initial docker build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 2:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/12173/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12173/1//COMMIT_MSG@7
PS1, Line 7: an in
> initial
Done
http://gerrit.cloudera.org:8080/#/c/12173/1//COMMIT_MSG@20
PS1, Line 20: - muti-os tools like build_mini_cluster_binaries.sh
> Is this an incomplete thought or is the apostrophe in ENTRYPOINT unnecessar
just a typo.
http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile
File docker/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@1
PS1, Line 1: # Licensed to the Apache Software Foundation (ASF) under one
> Nit: remove empty line?
Done
http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@22
PS1, Line 22: ocs.dock
> editing
Done
http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@27
PS1, Line 27:
> Nit: two spaces here
Done
http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@35
PS1, Line 35: COPY ./docker/bootstrap-env.sh /
> Curious why you chose to start with CentOS 7.6 and something like 6.6 (whic
I planned to start with current/common versions of CentOS and Ubuntu for the sake of not dealing with older version issues while fleshing out the build.
http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@50
PS1, Line 50: org.label-schema.dockerfile=$DOCKERFILE \
> Nit: looks like you're listing one dependency per line except here and in a
Done
http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@88
PS1, Line 88: org.label-schema.url=$URL \
> Does COPY operate recursively? Will it grab thirdparty/patches, for example
Yes, copy will grab everything in/below this directory.
http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@100
PS1, Line 100: FROM kudu-thirdparty AS kudu-build
> Is it safe to run `nproc` here to derive the parallelism?
It might be, I think that impacts the ability for the image to be cached though because different nproc results can invalidate an image.
http://gerrit.cloudera.org:8080/#/c/12173/1/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:
http://gerrit.cloudera.org:8080/#/c/12173/1/thirdparty/build-definitions.sh@746
PS1, Line 746: mkdir -p $TP_DIR/../www/
> Curious why this is needed: the 'www' subdirectory is checked into source c
I need this because in the docker build I copy and build thirdparty first without copying the rest or the source.
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 22 Jan 2019 04:26:44 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP: Add an intial docker integration
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: WIP: Add an intial docker integration
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12173/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12173/1//COMMIT_MSG@7
PS1, Line 7: intial
initial
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Jan 2019 06:08:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP: Add an intial docker integration
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: WIP: Add an intial docker integration
......................................................................
Patch Set 1: Verified+1
Overriding Jenkins, the failure had nothing to do with the patch at hand (I filed KUDU-2656 to track it).
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Jan 2019 05:53:27 +0000
Gerrit-HasComments: No
[kudu-CR] [docker] Add an initial docker build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12173
to look at the new patch set (#8).
Change subject: [docker] Add an initial docker build
......................................................................
[docker] Add an initial docker build
This patch adds an initial Docker file which defines a
multistage build that supports creating various Docker
images for Apache Kudu development.
This work is experimental and the choices and structure
may change with follow on commits.
Some things these images could be used for include:
- demos/examples
- pre-built binaries including thirdparty
- multi-os testing
- multi-os tools like build_mini_cluster_binaries.sh
- external integration testing
Some of the open tasks for the future are:
- Handle all build types.
- Optimize image sizes.
- Add kudu user to runtime images.
- Add an upload script with good tagging.
- Add healthchecks.
Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
---
A .dockerignore
A docker/Dockerfile
A docker/Dockerfile-files
A docker/README.adoc
A docker/bootstrap-env.sh
A docker/docker-build.sh
A docker/kudu-entrypoint.sh
M thirdparty/build-definitions.sh
8 files changed, 817 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/12173/8
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 8
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [docker] Add an initial docker build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 7:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/12173/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12173/7//COMMIT_MSG@10
PS7, Line 10: various Docker
: images
> Does it make sense to add more specifics about the currently supported plat
It's in the code and "supported" isn't really a thing yet anyway given this is experimental work.
http://gerrit.cloudera.org:8080/#/c/12173/7//COMMIT_MSG@14
PS7, Line 14: wih
> with?
Done
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile
File docker/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@31
PS7, Line 31: TODO
> We don't have a style guide for Dockerfiles, but maybe follow the same styl
I actually really dislike the TODO(user) model. Given this is an open source project and no individual person is responsible for the code I don't think a name should be tied to a todo. It's also really easy to see who added it via git blame if that is the need.
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@31
PS7, Line 31: seperate
> separate
Done
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@51
PS7, Line 51:
> nit: maybe, add an extra indent for this line since it's a continuation of
Done
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@63
PS7, Line 63: it's
> its
Done
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@79
PS7, Line 79: /*
> nit: drop ? Or you want to keep subdirs like .x ?
Not sure what you mean. This isn't leaving any subdirectories other than the hadoop, hive, and sentry ones.
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@84
PS7, Line 84: /*
> ditto
See above.
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@116
PS7, Line 116: ARG PARALLEL=4
> Maybe, just rely on GNU make and run as many parallel tasks as it allows by
See related comment below.
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@138
PS7, Line 138: -j${PARALLEL}
> As commented above, maybe just leave '-j' and run as many parallel tasks as
Lets experiment with this in future patches.
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 25 Jan 2019 20:42:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 3:
(11 comments)
Just skimmed through. It's great that we are about to have that dockerized stuff soon!
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/bootstrap-env.sh
File docker/bootstrap-env.sh:
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/bootstrap-env.sh@30
PS3, Line 30: if [[ -n $(which yum) ]];
why not just
if $(which yum); then
...
fi
?
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/bootstrap-env.sh@58
PS3, Line 58: \
drop
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/bootstrap-env.sh@36
PS3, Line 36: autoconf \
: automake \
: cyrus-sasl-devel \
: cyrus-sasl-gssapi \
: cyrus-sasl-plain \
: flex \
: gcc \
: gcc-c++ \
: gdb \
: git \
: java-1.8.0-openjdk-devel \
: krb5-server \
: krb5-workstation \
: libtool \
: make \
: openssl-devel \
: patch \
: pkgconfig \
: redhat-lsb-core \
: rsync \
: unzip \
: vim-common \
: whic
nit here and below: add an extra indent for the continuation of the command line
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/kudu-entrypoint.sh
File docker/kudu-entrypoint.sh:
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/kudu-entrypoint.sh@22
PS3, Line 22: KUDU_FLAGS
Do those come from the environment? Or that's about modifying this particular script and populating KUDU_FLAGS with something? If the latter, consider introducing the KUDU_FLAGS var with default value "" (i.e. empty).
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/kudu-entrypoint.sh@27
PS3, Line 27: resolved
... reached by their DNS name ...
This also verifies that network interfaces of master hosts are up and running. BTW, is it a requirement to have master hosts up and running? If not, why to use ping instead of DNS-related utils?
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/kudu-entrypoint.sh@51
PS3, Line 51: /var/lib/kudu/master
Turn into a variable to re-use elsewhere?
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/kudu-entrypoint.sh@52
PS3, Line 52: $KUDU_FLAGS
KUDU_FLAGS is common for both kudu-master and kudu-tservers. Does it make sense to separate those?
Also, if KUDU_FLAGS are here for customization, maybe make them last so it would be possible to override some options specified by this script?
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/kudu-entrypoint.sh@55
PS3, Line 55: /opt/kudu/www
Is it guaranteed to exist? Or that doesn't matter?
Also, consider turning it into a variable to re-use elsewhere in the script.
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/kudu-entrypoint.sh@62
PS3, Line 62: /var/lib/kudu/tserver
Turn into a variable to re-use elsewhere?
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/kudu-entrypoint.sh@66
PS3, Line 66: /opt/kudu/www
Is it guaranteed to exist? Or that doesn't matter?
http://gerrit.cloudera.org:8080/#/c/12173/3/thirdparty/build-thirdparty.sh
File thirdparty/build-thirdparty.sh:
http://gerrit.cloudera.org:8080/#/c/12173/3/thirdparty/build-thirdparty.sh@252
PS3, Line 252: rm -rf $PREFIX/opt/hadoop
nit: I'm not sure removing data automatically is a good thing to do. Maybe, just move the old dir X into X.$(date)?
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Jan 2019 18:16:25 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 7: Code-Review+1
(14 comments)
SGTM, just a few nits
http://gerrit.cloudera.org:8080/#/c/12173/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12173/7//COMMIT_MSG@10
PS7, Line 10: various Docker
: images
Does it make sense to add more specifics about the currently supported platforms in here?
http://gerrit.cloudera.org:8080/#/c/12173/7//COMMIT_MSG@14
PS7, Line 14: wih
with?
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile
File docker/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@31
PS7, Line 31: seperate
separate
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@31
PS7, Line 31: TODO
We don't have a style guide for Dockerfiles, but maybe follow the same style for TODO that we have in C++ source files? I.e., TODO --> TODO(ghenke)
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@51
PS7, Line 51:
nit: maybe, add an extra indent for this line since it's a continuation of a sub-element in the former one.
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@63
PS7, Line 63: it's
its
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@79
PS7, Line 79: /*
nit: drop ? Or you want to keep subdirs like .x ?
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@84
PS7, Line 84: /*
ditto
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@114
PS7, Line 114: TODO
nit: TODO(ghenke)
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@116
PS7, Line 116: ARG PARALLEL=4
Maybe, just rely on GNU make and run as many parallel tasks as it allows by dependency tree?
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@138
PS7, Line 138: -j${PARALLEL}
As commented above, maybe just leave '-j' and run as many parallel tasks as the target dependency tree allows?
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@250
PS7, Line 250: TODO
nit: TODO(ghenke)
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@288
PS7, Line 288: TODO
nit: TODO(ghenke)
http://gerrit.cloudera.org:8080/#/c/12173/6/docker/bootstrap-env.sh
File docker/bootstrap-env.sh:
http://gerrit.cloudera.org:8080/#/c/12173/6/docker/bootstrap-env.sh@165
PS6, Line 165: "Unsupported OS"
> There are no official SLES images in docker hub. There is OpenSUSE that we
SGTM
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 25 Jan 2019 20:18:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile
File docker/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@79
PS7, Line 79: /*
> To match that '-maxdepth 0' with removed globing, that should be replaced w
Given "hidden" directories starting with '.' actually never exist, are you okay with keeping it as is?
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 25 Jan 2019 22:37:44 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/README.adoc
File docker/README.adoc:
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/README.adoc@72
PS3, Line 72:
: === kudu-master
: A runtime image with kudu-master as the ENTRYPOINT.
: Uses the kudu-runtime image as a base.
:
: === kudu-tserver
: A runtime image with kudu-tserver as the ENTRYPOINT.
: Uses the kudu-runtime image as a base.
You might not have run into this if your docker version is new, but it might be worth making note of KUDU-2000 in case users try to deploy using older versions of docker (or maybe just note the version of docker you tested with).
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Jan 2019 19:56:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP: Add an intial docker integration
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: WIP: Add an intial docker integration
......................................................................
Patch Set 1:
(9 comments)
Neat stuff!
http://gerrit.cloudera.org:8080/#/c/12173/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12173/1//COMMIT_MSG@20
PS1, Line 20: - Add kudu-master and kudu-tserver images with ENTRYPOINT’s
Is this an incomplete thought or is the apostrophe in ENTRYPOINT unnecessary?
http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile
File docker/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@1
PS1, Line 1: #
Nit: remove empty line?
http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@22
PS1, Line 22: edditing
editing
http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@27
PS1, Line 27:
Nit: two spaces here
http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@35
PS1, Line 35: FROM centos:7.6.1810 as kudu-base
Curious why you chose to start with CentOS 7.6 and something like 6.6 (which would be the oldest we support). Maybe document that decision in the commit message?
http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@50
PS1, Line 50: gdb git \
Nit: looks like you're listing one dependency per line except here and in a few places below. Could you fix that?
http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@88
PS1, Line 88: COPY ./thirdparty thirdparty
Does COPY operate recursively? Will it grab thirdparty/patches, for example?
http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@100
PS1, Line 100: ARG PARALLEL=4
Is it safe to run `nproc` here to derive the parallelism?
http://gerrit.cloudera.org:8080/#/c/12173/1/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:
http://gerrit.cloudera.org:8080/#/c/12173/1/thirdparty/build-definitions.sh@746
PS1, Line 746: mkdir -p $TP_DIR/../www/
Curious why this is needed: the 'www' subdirectory is checked into source control (well, a bunch of files in it are checked in, but you get the idea).
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Jan 2019 06:07:35 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/bootstrap-env.sh
File docker/bootstrap-env.sh:
http://gerrit.cloudera.org:8080/#/c/12173/3/docker/bootstrap-env.sh@58
PS3, Line 58: \
> Why drop? Not sure what this means?
That meant the ending backslash is not needed for the very last line of the command (at least I cannot see other command lines in this file have such a thing)
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Jan 2019 23:15:08 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 8: Verified+1 Code-Review+1
I'll let you guys decide on the remaining comments, I'm good either way.
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 8
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 25 Jan 2019 21:33:00 +0000
Gerrit-HasComments: No
[kudu-CR] [docker] Add an initial docker build
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 2:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/12173/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12173/2//COMMIT_MSG@20
PS2, Line 20: muti
multi
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile
File docker/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile@71
PS2, Line 71: # TODO: Remove build dir too?
In theory you could remove thirdparty/build too, except that the clang-toolchain symlink points to llvm-6.0.0 within it. Off the top of my head I can't remember why that's the case (i.e. why it doesn't point to somewhere in thirdparty/installed).
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile@110
PS2, Line 110: COPY ./docs/support docs/support
Can we skip this? Not seeing where we build docs below.
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile-files
File docker/Dockerfile-files:
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile-files@18
PS2, Line 18: # This docker file is a minimal build usefull for debugging
useful
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/kudu-entrypoint.sh
File docker/kudu-entrypoint.sh:
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/kudu-entrypoint.sh@49
PS2, Line 49: KUDU_OPTS="$KUDU_FLAGS \
Is the idea that KUDU_FLAGS can be set by users to do more configuration? Maybe doc that at the top of the file?
http://gerrit.cloudera.org:8080/#/c/12173/2/docker/kudu-entrypoint.sh@52
PS2, Line 52: --fs_data_dirs=/var/lib/kudu/master \
You can omit this if the intent is to use the same value as --fs_wal_dir. Below too.
http://gerrit.cloudera.org:8080/#/c/12173/2/thirdparty/build-thirdparty.sh
File thirdparty/build-thirdparty.sh:
http://gerrit.cloudera.org:8080/#/c/12173/2/thirdparty/build-thirdparty.sh@248
PS2, Line 248: # Install Hadoop, Hive, and Sentry by copying their source directories into $PREFIX/opt.
: # Copying allows the source directory to be removed once thirdparty is built.
How much additional disk space will this consume?
Also, will the migration be smooth for existing thirdparty trees where these are symlinks? Looking at the build results for this patch, looks like the migration doesn't work at all. Maybe we need to rm -rf the existing symlinks first? I wonder if we should use rsync to manage this instead, as it'll do a better job of synchronizing the source and destination.
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 22 Jan 2019 21:44:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12173/6/docker/bootstrap-env.sh
File docker/bootstrap-env.sh:
http://gerrit.cloudera.org:8080/#/c/12173/6/docker/bootstrap-env.sh@165
PS6, Line 165: "Unsupported OS"
> What about SLES? We don't want to support it at all or it's just coming la
There are no official SLES images in docker hub. There is OpenSUSE that we could build on, but I didn't want to put in the effort right now. We can always add it later.
--
To view, visit http://gerrit.cloudera.org:8080/12173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42
Gerrit-Change-Number: 12173
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 24 Jan 2019 00:41:29 +0000
Gerrit-HasComments: Yes