for discussion only, first hack attempt at a patch by bhupendra

haven't done much packaging but looking at this patch Konstantin Boudnik or Olaf Flebbe ... any obvious mistakes come to mind?

This is being done by a group of graduate students in a class I'm guest lecturing, so go easy on em

Hi,

This looks good to me, congrats! Though I could not test it right now.

Only 3 nitty picky things, and an obvious larger one:

• The patch should be best relative to master branch not 1.1.0.
• The export {{+export SPARK_HOME=\$ {SPARK_HOME:-/usr/lib/spark}

  }} does not make sense here, since the comand above is an exec

• I like the subsequent tries of patches numbered (for instance BIGTOP-2345.1.patch ... BIGTOP-2345.2.patch). Its easier to check what's changed if I can download them without sorting them by time.

And the obvious one (Don't know whether that was in your focus.) We need the debian packaging as well. Hope your install _fink.sh is flexible enough for that kind of install layout.

And the big question (I don't know flink at all), if I look at the Apache Flink top page:
I see server side components. (YARN for instance). Doesn't flink have services to start or at least things to copy to HDFS?

> Doesn't flink have services to start or at least things to copy to HDFS?

Flink does not require HDFS to run, if that's answering your question.

Additionally, We aren't packaging flink as a Yarn app but as standalone, following the model for spark

+1 that makes sense for bigtop in my opinion!
In that case, Flink is really independent of the file systems. You can easily use Flink with Google cloud storage ("gs:///") or Amazon S3.

Hi,
thanks for the update. surely looking into the issues. have resolved issues reported by Olaf Flebbe. Will update the patch as soon as possible. Working fine now for majority of the build process. Still minor script changes remaining.

Hi Bhupendra! What is the current state of the patch? Did you make any further changes? I would like to resolve any pending issues so we can open a pull request.

Here is the pending pull request: https://github.com/apache/bigtop/pull/93/

See for for instance {{ https://ci.bigtop.apache.org/job/Bigtop-pullrequest93/BUILD_ENVIRONMENTS=centos-7,label=docker-slave/4/console}}

The patch does not apply cleanly

Thank you for looking into this Olaf.
I've started reworking the pull request today to make it work. I hope I'm finished with the rework tomorrow.

Great. The compile job should run every two hours. But I am not sure you can rebase the pull request #93

And BTW I can send you the command lines for you to test building within the bigtop environment. All you need is a running docker .

GitHub user rmetzger opened a pull request:

https://github.com/apache/bigtop/pull/101

BIGTOP-2345 Add deb and rpm packaging for Apache Flink

This PR is based on the work done in https://github.com/apache/bigtop/pull/93.
I've picked only the parts regarding the packaging (ignoring the puppet deployment).

The changes have been tested on a Debian8 virtual machine and Fedora 23.
Packaging, installation, the init scripts, user creation, file removal, logging seem to work now.

I'm looking forward to any comments regarding the proposed changes.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/rmetzger/bigtop bigtop2345-pr

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/bigtop/pull/101.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #101

commit f076772ba20ad5a9590c479987e95292e930a752
Author: bhupendrasingh <bhupendrasingh.2@gmail.com>
Date: 2016-02-20T21:47:27Z

BIGTOP-2345 Initial prototyping for Flink packaging

commit 1e67d973bf36644e3e13fb5a287b36b72ab2d120
Author: Robert Metzger <rmetzger@apache.org>
Date: 2016-04-20T15:59:54Z

BIGTOP-2345 Refined Flink packaging

I can not change other people's pull requests.
I've opened a new one for this issue.

If its not much overhead for you, it would be great if you could paste me the command lines for testing my stuff.
(I have docker, but I never got around trying it really out )
I've manually tested my changes, but I guess some automated "official" tests can not hurt.

Github user mxm commented on a diff in the pull request:

https://github.com/apache/bigtop/pull/101#discussion_r60749429

— Diff: bigtop-packages/src/rpm/flink/SPECS/flink.spec —
@@ -0,0 +1,143 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+%define flink_name flink
+%define lib_flink /usr/lib/%

+%define config_flink %

/conf
+%define man_dir %{_mandir}
+%define flink_services jobmanager taskmanager
+%define var_run_flink /var/run/%

+
+
+%if %

0
+%define doc_flink %{_docdir}/%

-%

-bin/%

-%

+Release: %

+Summary: Apache Flink is an open source platform for distributed stream and batch data processing.
+License: ASL 2.0
+URL: http://flink.apache.org/
+Group: Development/Libraries
+Buildroot: %{_topdir}/INSTALL/%

-%

+
+%build
+bash $RPM_SOURCE_DIR/do-component-build
+
+
+
+# Init.d scripts
+%__install -d -m 0755 $RPM_BUILD_ROOT/%

/%

-$

status > /dev/null 2>&1
+ if [ $? -eq 0 ]; then
+ /sbin/service %

-conf %

.dist
+
+%dir %{_sysconfdir}/%

-master
+%config(noreplace) %

/%

-taskmanager
```

Github user mxm commented on the pull request:

https://github.com/apache/bigtop/pull/101#issuecomment-213452902

I'm getting the following error message when building using `gradle flink-rpm`:

>RPM build errors:
File not found: /home/max/Desktop/bigtop/build/flink/rpm/BUILDROOT/flink-1.0.0-1.fc23.x86_64/etc/rc.d/init.d/flink-master
File not found: /home/max/Desktop/bigtop/build/flink/rpm/BUILDROOT/flink-1.0.0-1.fc23.x86_64/etc/rc.d/init.d/flink-worker
File not found: /home/max/Desktop/bigtop/build/flink/rpm/BUILDROOT/flink-1.0.0-1.fc23.x86_64/usr/share/doc/flink-1.0.0

You can follow https://ci.bigtop.apache.org/job/Bigtop-pullrequest101/ now.

Thank you. I'm not a Jenkins expert, but I don't think that this error is caused by my pull request:
https://ci.bigtop.apache.org/job/Bigtop-pullrequest101/BUILD_ENVIRONMENTS=fedora-20,label=docker-slave/lastBuild/consoleFull
Can you look into it?

Aehm, sorry. That's not an jenkins issue, that's me. Fixed and running again.

Github user oflebbe commented on the pull request:

https://github.com/apache/bigtop/pull/101#issuecomment-214708194

Hi the build looks very promising! You should not package /var/run, since this is only a temporary filesystem. You should create it with your init script (This will fix one of the lintian errors for debian/ubuntu).

Github user rmetzger commented on the pull request:

https://github.com/apache/bigtop/pull/101#issuecomment-214720306

Thank you for the feedback!
I'm trying to address them soon.

Github user rmetzger commented on the pull request:

https://github.com/apache/bigtop/pull/101#issuecomment-214785025

Thank you for the review @mxm!

Regarding the lsb dependency: I saw that the Hadoop spec for example has the following entry:
```
Requires: sh-utils, /lib/lsb/init-functions
```
I guess we can add that as well.

> I wonder, should we rename the service to flink-master and flink-worker? Or flink-jobmanager and flink-taskmanager? Simply TaskManager and JobManager could confuse people.

I'm definitively against `flink-master`, `flink-worker`, because that's not an "official" terminology (at least in the Flink code we call them job- and taskmanager).
I also thought about `flink-jobmanager` and `flink-taskmanager`. Any other thoughts on this?

> Another thing, the logs are named "flink-flink-jobmanager-0-localhost.localdomain.log". Probably could remove the flink-flink prefix.

The naming schema for logs is `flink-<username><component><instance>-<host>.<log|out>`. By starting the flink services with the `flink` user, we end up with that ugliness.
I'll look into it and try to see if I can change the behavior easily (the problem is that I need to create patches if I want to change anything on the flink source, and I want to avoid patching too much for bigtop).

@oflebbe: I'll check the guide again and adopt the patch integration accordingly.

Github user rmetzger commented on the pull request:

https://github.com/apache/bigtop/pull/101#issuecomment-214794072

@oflebbe quick question:
The spec file contains already the following section:

```spec
%if %{?suse_version:1}0

1. Required for init scripts
   Requires: insserv
   %global initd_dir %{_sysconfdir}/rc.d

%else

1. Required for init scripts
   Requires: /lib/lsb/init-functions
   %global initd_dir %{_sysconfdir}/rc.d/init.d
   %endif
   ```

I assume the if condition is true for suse, so the else branch applied for Max' build, requiring the init-functions?

Github user rmetzger commented on the pull request:

https://github.com/apache/bigtop/pull/101#issuecomment-214798709

Another follow up Q:

> You should not package /var/run, since this is only a temporary filesystem. You should create it with your init script (This will fix one of the lintian errors for debian/ubuntu).

Okay, makes sense. I was looking a bit into other scripts and I saw that others are setting the `$WORKING_DIR` variable.
I wonder why `/bigtop-packages/src/templates/init.d.tmpl` is not taking care of the creation and deletion of the `$WORKING_DIR` for that?

Github user mxm commented on the pull request:

https://github.com/apache/bigtop/pull/101#issuecomment-214802211

>I'm definitively against flink-master, flink-worker, because that's not an "official" terminology (at least in the Flink code we call them job- and taskmanager).

Then let's call them `flink-jobmanager` and `flink-taskmanager`. The `flink-` prefix will help people to discover the service names.

Github user rmetzger commented on the pull request:

https://github.com/apache/bigtop/pull/101#issuecomment-215009947

Okay, I'll rename the services

Github user rmetzger commented on the pull request:

https://github.com/apache/bigtop/pull/101#issuecomment-215436502

I addressed all remaining issues with the pull request.

Please let me know if there's anything else blocking this from being merged.

This looks good for a first pass , lets merge.! Konstantin Boudnik YoungWoo Kim Evans Ye Olaf Flebbe

can we merge https://github.com/apache/bigtop/pull/101.patch

A few more...

• first of

  git apply 101.patch
  101.patch:58: trailing whitespace.
  enable_local_repo: false 
  101.patch:91: trailing whitespace.
  mvn install $FLINK_BUILD_OPTS -Drat.skip=true -DskipTests -Dhadoop.version=$HADOOP_VERSION "$@" 
  101.patch:153: trailing whitespace.
     
  101.patch:443: trailing whitespace.
   
  101.patch:444: trailing whitespace.
   - `recovery.job.delay`: (Default 'akka.ask.timeout') Defines the delay before persisted jobs are recovered in case of a recovery situation. 
  warning: squelched 20 whitespace errors
  warning: 25 lines add whitespace errors.
  

• bigtop-deploy/vm/vagrant-puppet-vm/vagrantconfig.yaml has irrelevant changes

RAT report seems to be fine. So let's fix these white-space issues and merge unless there are more comments from others.

Thank you for the review Konstantin Boudnik.

I fixed the whitespace errors (btw you can fix them yourself with git apply --whitespace=fix 101.patch) and removed the irrelevant changes.

I am pretty sure I can, but then my version will be different from yours and I would need to create a PR, which I don't want to

Github user mbalassi commented on the pull request:

https://github.com/apache/bigtop/pull/101#issuecomment-217156713

Hey guys, any update on this? I am looking forward to having the Flink integration in.

Olaf Flebbe, any other comment from your side?

I have committed this and pushed to the master. Attaching the latest patch for the reference as who know where the GH PRs will be in the future.

Thanks everybody who helped to get this patch ready!

Github user c0s commented on the pull request:

https://github.com/apache/bigtop/pull/101#issuecomment-217743938

I have committed this to the master. Feel free to close the PR and thanks for your contribution!

Awesome news! Can't wait to deploy Flink as part of my test Bigtop cluster. Btw, any reason it shouldn't be added here: https://ci.bigtop.apache.org/view/Packages/job/Bigtop-trunk-packages/ ?

No reason, just no one has done it yet

Github user rmetzger commented on the pull request:

https://github.com/apache/bigtop/pull/101#issuecomment-217852730

Thanks a lot for merging.
I'm closing the PR now.

Github user rmetzger closed the pull request at:

https://github.com/apache/bigtop/pull/101

BTW: I added flink to the bigtop-packages-trunk ci job.