palantir / sls-packaging Goto Github PK

Disclaimer: I'm on 0.9.0, but I don't see any relevant changes in recent commits.

I'm running a java application on Windows. This application uses spring shell and jline, which means that it loads a .dll file to the temporary directory upon startup. This consistently fails for me due to the default JVM_OPT of -Djava.io.tmpdir=var/tmp/data - it doesn't seem to point to the correct directory on Windows. I've haven't been able to get any relative directory to work in this setting, but using a full path did work (e.g. C:...\var\tmp\data).

If the top-level (daemonized) service starts one or more long-running child processes, it has to ensure that those child processes get killed when the parent process is killed. This is somewhat painful to do in Java because it involves setting a shutdown hook, and it's repetitive work that's easy to get wrong. If done incorrectly, the child processes become zombies and have to be reaped manually by other means; they could take up system resources such as memory and ports.

Instead, if init.sh were to record the process group ID and kill the process group, we'd have a clean solution that works without additional work from the app.

Tagging @mingyukim, @tomkozlowski and @gdearment for comments.

init.sh stop has sleep 4 right now; instead could loop for awhile checking

Or just var/conf? My use case is that I need my $SERVICE_HOME/var/conf/logback.xml to exist on the classpath in order for logback to pick it up properly.

Healthcheck logs by default get overwritten on each run. We cannot reliably debug healthcheck alerts from x hours ago since the state is lost.
Can we please default to append + archive instead over-writing the file?

Saw an internal product which forcibly deletes var/data before doing any docker tasks; this removed the tmpdir that gjd created. Gjd should consider creating the dir by some other mechanism, because the current method is liable to being tampered with and then leading to confusing runtime bugs.

I'd like to run a custom dropwizard command (migrate) to do migrations, that needs the JVM trustStore system property to be set.

Since I can't use init.sh to run migrate, I tried to use the [service-name] bash start script, however, this did not pick up the JVM arguments I'd set in launcher-custom.yml.

  launcherCustom:
    configType: java
    configVersion: 1
    jvmOpts:
      - '-Djavax.net.ssl.trustStore=XXX'

Is this a reasonable approach to take or am I going about this the wrong way?

manifest.yml is set to executable. Is there a reason for this line? https://github.com/palantir/gradle-java-distribution/blob/develop/src/main/groovy/com/palantir/gradle/javadist/JavaDistributionPlugin.groovy#L95

https://github.com/palantir/gradle-java-distribution/blob/53bceca9dea953e821ebea908c2e868db859289b/src/main/resources/init.sh#L140

Would it be useful if this printed more information to STDOUT?

https://circleci.com/gh/palantir/gradle-java-distribution/300?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

Given that this project bundles that one, making that well known would be helpful.

https://github.com/palantir/go-java-launcher

Per the spec here: https://refspecs.linuxbase.org/LSB_3.0.0/LSB-PDA/LSB-PDA/iniscrptact.html

...In addition to straightforward success, the following situations are also to be considered successful:

• running start on a service already running

PR incoming

Can we move config.sh to var? It seems like a better place for deployment specific configurations to stay consistent. If it's cool I can look at making a PR.

e.g. I should be able to run /opt/some/directories//service/bin/init.sh rather than having to cd into /opt/some/directories/ and then service/bin/init.sh

cc @schlosna

The JVM defaults are to use the Parallel Old GC collector. Probably most services using g-j-d should be using either the CMS + Parallel New collector or the G1 collector.

Should we make an opinionated decision in g-j-d about a different default GC collector than the JVM defaults?

At the moment, the distTar task always uses the runtime configuration: https://github.com/palantir/gradle-java-distribution/blob/b5d2c60a3e9e59fa382a793c834eaf8dd23b5e54/src/main/groovy/com/palantir/gradle/javadist/DistTarTask.groovy#L48

To facilitate testing plugin points, I've added an extra eteDrivers configuration which has a couple of extra drivers. It would be great if the gradle-java-distribution plugin would allow me to set up both a normal distTar task and also an eteDistTar task.

It would be helpful for our users is init.sh reported "Running (PID xxxxx)" instead of the current "Ok".

Furthermore, it would be a good idea to include some sort of logic to ensure the server actually started if at all possible (eg checking to see if the pid is still live after a few seconds...)

I've seen several issues in the field whose root cause was people running init.sh start by hand instead of via lifecycle management tools. It would be nice to add an optional flag to the distribution block that would cause a message to be printed to stderr, so that when people do run the init script by hand they get immediate feedback telling them it may cause problems.

Something like stdErrOnStart, which takes the full message you want printed, maybe? I'm not sure if this is too specific.

Probably best to have on by default in prod

Concerns:

• what directory to write them to
• log rotation
• what exact flags to enable by default

We have a service which generates some 100 or so files automatically which are included in the output distribution.

At the moment, this task dictates that we must store these generated files in projectDir/service.

It would be nice if we could instead supply a path to an alternate location to pull files from, like buildDir/work. Otherwise, we're cluttering our git repo with auto-generated files (or at the very least leaving behind a dirty git repo).

See https://github.com/nmalaguti/repro-distribution-flexversion for a repro. Running ./gradlew distTar results in:

* What went wrong:
Execution failed for task ':createManifest'.
> No signature of method: java.lang.String.replaceAll() is applicable for argument types: (java.lang.String, com.palantir.gradle.versions.flexversioning.FlexVersion) values: [@serviceVersion@, master-1-g2c6266869ebb]
  Possible solutions: replaceAll(java.lang.String, java.lang.String), replaceAll(java.util.regex.Pattern, groovy.lang.Closure), replaceAll(java.util.regex.Pattern, java.lang.CharSequence), replaceAll(java.lang.CharSequence, java.lang.CharSequence), replaceAll(java.lang.CharSequence, groovy.lang.Closure), replace(char, char)

Since Gradle will convert the project.version object to a string when it needs to, I would expect the plugin to do the same.

layoutVersion indicates the version of the directory&file layout

Some products like to package a JRE to reduce their dependency on the environment. There is currently not a good way to set JAVA_HOME to point at the packaged one from within init.sh or the start scripts. I'm not totally convinced products should do this as it makes your distro larger and platform-specific (generally linux only), but it seems to be a thing people want to do. There would need to be a good way for users to override it too, unless the answer is to publish two versions of everything.

While trying to run our ETE tests, if our commit hash changes, the launcher-static.yml isn't updated. This causes our ETE tests to fail because the tgz has the new jars but the launcher-static.yml is referencing the old ones.

@hsaraogi @jeremyk-91

Is there a way to configure the classpath of the application script to include a specified plugins directory? I'd like to add var/plugins/* to the end of the classpath.

This can be handled by excluding manifest.yaml when in the from("${project.projectDir}/deployment") { closure.

For debugging purposes it can be helpful to have minutely jstacks written out to disk as a poor-man's version of assembling a flamegraph

• Generate service/bin/config.sh as part of script generation.
• Add javaHome config option and emit JAVA_HOME="<javaHome>" into config.sh as part of generation

Consumers should be able to specify that they want a certain major version of Java. If run with a lower version, the service script should die with an informative message (rather than falling through to Java, which will usually produce a mysterious "incompatible class format 52" message down the line).

• need to bump manifest format version when making this change

init.sh check tries to grep service name in the command for the PID. If the service name is different than project name, this check might fail with a generic failure message "process dead but PID exists".

Three options I can think of to make this better:
(1) A better way to check command for the PID (not sure if there's another easy way)
(2) Enforce service name to be same as project name
(3) At minimum, fail with a better message, like "Could not find service name in command for PID"

This has been an annoying pain point for a very long time, and it would be great to finally fix this.

Both the var/log and var/run directories may contain .log or .pid files that were generated when developing locally. They shouldn't be included in the resulting tarball.

Something like

  launcherCustom:
    configType: java
    configVersion: 1
    jvmOpts:
      - '-XX:+PrintGCDateStamps'
      - '-XX:+PrintGCDetails'
      - '-XX:-TraceClassUnloading'
      - '-XX:+UseGCLogFileRotation'
      - '-XX:GCLogFileSize=10M'
      - '-XX:NumberOfGCLogFiles=10'
      - '-Xloggc:var/log/{{product_name}}.gc.log'
      - '-verbose:gc'

The DistTar task will include source jars (files off the form *-sources.jar). This is dangerous if you are pulling in source jars, as you might accidentally be distributing proprietary source code.

This config:

configType: java
configVersion: 1
jvmOpts: -XX:-PerfDisableSharedMem

Produces this panic:

panic: yaml: unmarshal errors:
  line 12: cannot unmarshal !!str `-XX:-Pe...` into []string

goroutine 1 [running]:
panic(0x555de0, 0xc8200d9860)
        /usr/local/go/src/runtime/panic.go:481 +0x3e6
github.com/palantir/go-java-launcher/launchlib.ParseCustomConfig(0xc820093500, 0x156, 0x356, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
        /home/ubuntu/.go_workspace/src/github.com/palantir/go-java-launcher/launchlib/config.go:57 +0xf8
main.LaunchWithConfig(0x7fffd6e37e2e, 0x1f, 0x7fffd6e37e4e, 0x1c)
        /home/ubuntu/.go_workspace/src/github.com/palantir/go-java-launcher/go-java-launcher/main.go:36 +0x3d6
main.main()
        /home/ubuntu/.go_workspace/src/github.com/palantir/go-java-launcher/go-java-launcher/main.go:56 +0xf6

I would expect to instead get a helpful error message here about the correct format for the jvmOpts parameter needing to be a string array rather than just a string.

Corrected config is (from memory):

configType: java
configVersion: 1
jvmOpts:
 - "-XX:-PerfDisableSharedMem"

Some Gradle plugins depend, for example, on the distTar task output filename to resolve against artifacts or publish locations. (For example, pt-almanac.) Since gradle-java-distribution adds an afterEvaluate to configure the output name, any output filenames fetched during normal project evaluation are not reliable, and will cause those plugins to break in non-obvious ways at runtime (see HIGHLINE-1091).

The workaround, as it exists currently, is to move the publication resolution strategy (or the entire almanacPublish task) to an afterEvaluate, but I find this difficult to maintain for the same reason _.defer isn't desirable in JS code. If, for example, the Almanac task is placed in a shared Gradle script, depending on it or modifying its configuration would probably require a nested afterEvaluate block.

I'd suggest refactoring the extension instead to mutate variables/properties synchronously, so that after the extension is reconfigured, the project is in a consistent/complete state and ready for further configuration. As an even stronger notion, the extension can be a call-me-once method, such that all necessary configuration is collected from the closure, and the run/dist tasks are created in immutable-state fashion from the one-time configuration closure.

Especially important because Java convention for separating http.nonProxyHosts parameters is with pipes.

Setting JAVA_OPTS to this would cause you very bad times: -Dhttp.nonProxyHosts=127.0.0.1|sudo\ rm\ -rf\ /opt

I think https://github.com/palantir/gradle-java-distribution/blob/8242c733c6f0ea5361bf9d6a4c59195d2073806e/gradlew#L161 needs to quote those parameters with double-quotes

Currently this line mentions setting Java options using the JVM_OPTS environment variable, but the artifacts built using the plugin look for a variable named JAVA_OPTS (see this line).

// + @markelliot + @bmoylan for SA + thoughts

i'd like to refer to projects that produce a java distribution tarball via Gradle dependencies. specifically, i'd like to do something like this:

dependencies {
    skylab project(path: ':product', configuration: 'sls')
}

this would give a clean way for a project to depend on the resulting sls artifact produced by this plugin.

it would involve adding something like this to the plugin:

configurations {
    sls
}

artifacts {
    sls distTar
}

i believe this would be non-disruptive to existing projects.

http://www.evanjones.ca/jvm-mmap-pause.html

https://docs.gradle.org/2.14.1-rc-1/release-notes

My project has multiple configurations (that extend from the runtime configuration) that allow the project to be built with differently versioned jars. I would like to be able to use distTar to build a tgz for each of these configurations, but currently, the plugin only works for runtime. Currently I have a project-distribution subproject that iterates over these configurations and exposes a dist task for each (distA, distB.. etc). If I could specify which configuration to use with this plugin, then I could instead make sub-projects for each of the different configurations and apply the plugin to a different configuration on each one (project-distribution-a:distTar, project-distribution-b:distTar). That's somewhat more overhead but it's the only way that I can think of that would make this work.