FLUME-3415 - Provide Docker image for Flume #351

tmgstevens · 2022-02-08T17:58:22Z

Draft docker build and integration tests.

This reverts commit b61c4c5. Conflicts: flume-ng-dist/Dockerfile

busbey · 2022-03-07T21:29:19Z

is there a jira that describes the goals of this PR?

flume-ng-dist/Dockerfile

…dency" This reverts commit 67664ab.

tmgstevens · 2022-04-09T09:59:16Z

Reverting the now duplicated HBase client changes

Add in github secrets and push to dockerhub

busbey · 2022-10-31T15:06:27Z

.github/workflows/build.yml

+      - name: Deploy Docker (Linux)
+        if: runner.os == 'Linux' && github.ref == 'refs/heads/trunk'
+        timeout-minutes: 90
+        shell: bash
+        run: |
+          ./mvnw deploy -Ddockerfile.username=$${{ secrets.DOCKERHUB_USER }} \
+           -Ddockerfile.password=... $${{ secrets.DOCKERHUB_TOKEN }}
+


We shouldn't be publishing snapshot builds of trunk to DockerHub. Can we make sure we're only publishing released artifacts?

busbey · 2022-10-31T15:08:34Z

flume-ng-dist/Dockerfile

+LABEL org.opencontainers.image.authors="${MAINTAINER}"
+LABEL site="https://flume.apache.org"
+
+COPY target/apache-flume-${ASSEMBLY_VERSION}-bin/apache-flume-${ASSEMBLY_VERSION}-bin/ /etc/flume/


we shouldn't be putting executables into /etc. Since we're making a flume user can we put the executables into that user's home directory?

busbey · 2022-10-31T15:12:12Z

flume-ng-dist/pom.xml

+        <groupId>com.spotify</groupId>
+        <artifactId>dockerfile-maven-plugin</artifactId>
+        <version>${dockerfile.version}</version>
+        <!-- Wire up to the default build phases -->


this plugin is in "public archived" status. we should treat it as EOL and should not rely on it.

it's over here:
https://github.com/spotify/dockerfile-maven

busbey · 2022-10-31T15:17:54Z

flume-ng-tests/src/test/java/org/apache/flume/test/agent/TestFileChannelDocker.java

+
+  @After
+  public void tearDown() throws Exception {
+    DockerInstall.getInstance().stopAgent();
+    for (File tempResource : tempResources) {
+      tempResource.delete();
+    }
+    agentProps = null;
+  }
+
+  /**
+   * File channel in/out test. Verifies that all events inserted into the
+   * file channel are received by the sink in order.
+   * <p>
+   * The EXEC source creates 100 events where the event bodies have
+   * sequential numbers. The source puts those events into the file channel,
+   * and the FILE_ROLL The sink is expected to take all 100 events in FIFO
+   * order.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testInOut() throws Exception {
+    LOGGER.debug("testInOut() started.");
+
+
+    DockerInstall.getInstance().startAgent("a1", agentProps, tempResources);


there's a code smell here due to the agent start being in the test while the agent stop is in an after block. either start and stop should both be in the test, or the start should be in a before.

busbey · 2022-10-31T16:07:52Z

flume-ng-tests/src/test/java/org/apache/flume/test/agent/TestFileChannelDocker.java

+  @Before
+  public void setUp() throws Exception {
+    /* Create 3 temp dirs, each used as value within agentProps */


you should add a JUnit Assert.assumeTrue to skip the test cases when Docker isn't available on the system.

busbey · 2022-10-31T16:09:02Z

flume-ng-tests/src/test/java/org/apache/flume/test/util/DockerInstall.java

+    ImmutableList.Builder<String> builder = new ImmutableList.Builder<>();
+    builder.add("/bin/sh");
+    builder.add("-c");
+    builder.add("'docker kill " +  containerId + "'");


all of the cases where docker is invoked here should be an environment variable or some such that defaults to "docker" and allows whoever is running the test to specify a fully qualified path to the docker executable.

rgoers · 2022-10-31T22:13:41Z

Now that I look at this again I don't think I really understand how it helps.

It packages all the artifacts whether they are required or not.
Unless I am mistaken, it is getting the distribution tar and using the configuration located within that. I fail to see how useful that will be as I would expect most users would have a custom configuration.

To be honest, I added Spring Boot support to simplify a LOT of this. Spring Boot packages up just the dependencies you need, uses the standard application.yml to configure Flume, and allows all the wiring of Sinks, Sources, and Channels to be performed in standard Spring Java config source files. Once packaged with the Spring Boot Maven plugin startup consists of either

java -Dspring.profiles.active={envName} -jar my-flume.jar
2../my-flume.jar -Dspring.profiles.active={envName}

busbey · 2022-11-01T16:49:21Z

Container images make deployment to a variety of environments much, much simpler. So I definitely agree with Tristan that we need something like this. For example, it would make it about an order of magnitude easier for me to evaluate release candidates.

Additionally, the project having an opinion on what a reasonable starting point for a container image looks like will make conversation I have about deployment in modern production environments easier.

I think starting from "everything goes in the bucket" to match our historic deployment shape is fine. I agree that eventually we need to get to a more modular approach that provides easy examples for folks building just the parts they need.

There's some tooling around for easier container image building based on spring boot applications, e.g. Jib. Maybe we should take an approach that leverages that?

rgoers · 2022-11-01T23:47:48Z

I certainly don't have an issue against providing a Docker image. I just am unclear as to how it will be useful, but I suppose if it is just used as a starting point it could work out. But the approach you mention sounds much better in the long run. I just don't want to have to support that a) I won't have any use for, and b) we will have to keep compatibility with even though we want to do something much better.

To be clear, I use the FileChannel, which pretty much requires fast disk (i.e. SSDs or equivalent) to perform well. It also requires a dedicated "disk" so that data isn't lost on restart. This doesn't really work well with Docker/Kubernetes so we don't use it for Flume.

tmgstevens · 2022-11-02T11:02:02Z

So a couple of points in here:

It packages all the artifacts whether they are required or not.

True - packaging is something that I think we need to think about going forwards. For people who want a lightweight deployment, should we offer different profiles, the flipside being that if you're combining two components from different modules (e.g. syslog and kafka, HTTP and HDFS etc) then actually do you ever get the benefit of the modularity, or does it just ramp up your complexity (complexity = adoption blocker in my mind).

Unless I am mistaken, it is getting the distribution tar and using the configuration located within that. I fail to see how useful that will be as I would expect most users would have a custom configuration.

It does bundle the default conf directory, but it is anticipated that a user would re-map that or pass config in via environment variables (which could then include secrets). Both designed to work in docker and kubernetes. There's an example of doing that here: https://github.com/apache/flume/blob/d2bd7812dbacd86459726c0fd3dc774272ce0222/flume-ng-tests/src/test/java/org/apache/flume/test/util/DockerInstall.java#L137-L153

I think starting from "everything goes in the bucket" to match our historic deployment shape is fine. I agree that eventually we need to get to a more modular approach that provides easy examples for folks building just the parts they need.

+1

There's some tooling around for easier container image building based on spring boot applications, e.g. Jib. Maybe we should take an approach that leverages that?

Personally I'd rather not re-write the docker deployment right now given that what's there works pretty well. We could look to move away from the spotify plugin to something else, but I don't want to re-architect the whole packaging of Flume at the moment.

To be clear, I use the FileChannel, which pretty much requires fast disk (i.e. SSDs or equivalent) to perform well. It also requires a dedicated "disk" so that data isn't lost on restart. This doesn't really work well with Docker/Kubernetes so we don't use it for Flume.

I actually think this would be fine - you can have persistent disks in Kubernetes assigned to pods, same applies to docker where you can mount a volume. Would I necessarily recommend that you re-write your deployment model to use containers? No. But for example, in @busbey 's world where he might be moving from a previously managed deployment to something that needs additional orchestration, using Docker or Kubernetes and deploying agents across many nodes, this could make things much easier to manage and maintain.

Tristan Stevens and others added 17 commits February 7, 2022 12:07

Draft docker

b61c4c5

Add docker build to flume-ng-dist as part of package phase

31d99d3

De-dupe command line arguments

39042bd

Re-add in dockerfile.version property

fdc31cf

Re-add in dockerfile.version property

d749050

Revert "Draft docker"

1f19ec5

This reverts commit b61c4c5. Conflicts: flume-ng-dist/Dockerfile

Add check to make sure container starter okay.

81773b8

Checkstyle fixes

5e8fc3b

Docker fixes for Ubuntu

ac262b2

Setup docker on Macos

87dc92a

Update build

5f8f981

Update build

877337a

Remove macos

caa2745

Add profile to skip docker on Macos builds if required

ccbbda7

Merge branch 'trunk' into docker

bf2b41b

Add profile to skip docker on Macos builds if required

d733b3f

Check if process is still running

d2bd781

busbey reviewed Mar 7, 2022

View reviewed changes

flume-ng-dist/Dockerfile Show resolved Hide resolved

Tristan Stevens added 2 commits March 8, 2022 16:10

Switch to use label for maintainer

fdb3a76

per HBASE-22318 switch HBase to use unbounded glassfish dependency

67664ab

tmgstevens changed the title ~~Docker~~ FLUME-3415 - Provide Docker image for Flume Mar 28, 2022

Revert "per HBASE-22318 switch HBase to use unbounded glassfish depen…

28370c2

…dency" This reverts commit 67664ab.

tmgstevens and others added 3 commits May 11, 2022 19:21

Update build.yml

7571738

Add in github secrets and push to dockerhub

Merge from trunk

6a37230

Merge branch 'trunk' into docker

e024d5f

busbey reviewed Oct 31, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FLUME-3415 - Provide Docker image for Flume #351

FLUME-3415 - Provide Docker image for Flume #351

tmgstevens commented Feb 8, 2022

busbey commented Mar 7, 2022

tmgstevens commented Apr 9, 2022

busbey Oct 31, 2022

busbey Oct 31, 2022

busbey Oct 31, 2022

busbey Oct 31, 2022

busbey Oct 31, 2022

busbey Oct 31, 2022

busbey Oct 31, 2022

rgoers commented Oct 31, 2022

busbey commented Nov 1, 2022

rgoers commented Nov 1, 2022

tmgstevens commented Nov 2, 2022

FLUME-3415 - Provide Docker image for Flume #351

Are you sure you want to change the base?

FLUME-3415 - Provide Docker image for Flume #351

Conversation

tmgstevens commented Feb 8, 2022

busbey commented Mar 7, 2022

tmgstevens commented Apr 9, 2022

busbey Oct 31, 2022

Choose a reason for hiding this comment

busbey Oct 31, 2022

Choose a reason for hiding this comment

busbey Oct 31, 2022

Choose a reason for hiding this comment

busbey Oct 31, 2022

Choose a reason for hiding this comment

busbey Oct 31, 2022

Choose a reason for hiding this comment

busbey Oct 31, 2022

Choose a reason for hiding this comment

busbey Oct 31, 2022

Choose a reason for hiding this comment

rgoers commented Oct 31, 2022

busbey commented Nov 1, 2022

rgoers commented Nov 1, 2022

tmgstevens commented Nov 2, 2022