-
-
Notifications
You must be signed in to change notification settings - Fork 9
Conversation
imagePullPolicy: IfNotPresent | ||
env: | ||
- name: "JAVA_OPTS" | ||
value: "-Dplay.http.secret.key=a-very-strong-key-for-production -Dplay.filters.hosts.allowed.0=myservice.example.org -Dplay.server.pidfile.path=/dev/null" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pidfile disabled because https://github.com/lightbend/tooling-team/issues/238
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disabling the use of the pidFile is only needed on OpenShift ATM (until file permissions are configurable).
Still, using pidFile in an ephemeral container doesn't make much sense so I think we should suggest disabling it in all deployment targets (vanilla k8s, openshift, ...). WDYT @jroper @TimMoore ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it makes sense to disable. Also, I'd prefer that we create a separate configuration file for prod, and put all the prod specific configuration in there, including PID file. Then the only command line argument that needs to be passed here is the configuration file name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I like that approach, It'll make the docs a lot cleaner.
I'll consolidate the settings into prod-application.conf
on a separate PR once both this and #38 are merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
raised #41
d3bf75b
to
fe9c90e
Compare
@@ -31,7 +20,6 @@ | |||
<logger name="com.gargoylesoftware.htmlunit.javascript" level="OFF" /> | |||
|
|||
<root level="INFO"> | |||
<appender-ref ref="ASYNCFILE" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed the FILE
appenders because the container isn't built with proper permissions for OpenShift at the moment.
restartPolicy: Always | ||
containers: | ||
- name: "play-scala-grpc-example" | ||
image: "docker-registry-default.centralpark.lightbend.com/play-scala-grpc-example/play-scala-grpc-example:1.0-SNAPSHOT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The image coordinates are the only relevant difference in openshift.yaml
when compared to kubernetes.yaml
(other than routes vs ingress).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like these coordinates to not be centralpark-specific.
protocol: TCP | ||
selector: | ||
appName: "play-scala-grpc-example" | ||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Deployment
and Service
are (almost) identical to kubernetes.yml
ingress: | ||
- conditions: | ||
host: myservice.example.org | ||
routerName: router |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This YAML is a stripped down version of the YAML OpenShift generates when creating a Route
from the GUI. I've removed a few fields that I thought irrelevant. It's possible this YAML can be reduced a bit more. I don't think we should block this PR on that potential improvement.
docker login -p $TOKEN -u unused $DOCKER_REGISTRY_SERVER | ||
docker tag $IMAGE:$TAG $DOCKER_REGISTRY/$IMAGE:$TAG | ||
docker push $DOCKER_REGISTRY/$IMAGE:$TAG | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This script publishes the image locally and then pushes it to centralpark. I'd like to publish it directly into centralpark.
Note: I followed the instruction in https://docs.google.com/document/d/1LmNoH3k37OjF98dkrhIXyuJtTOc-GiPduHfujJsiTf0/edit#
restartPolicy: Always | ||
containers: | ||
- name: "play-scala-grpc-example" | ||
image: "docker-registry-default.centralpark.lightbend.com/play-scala-grpc-example/play-scala-grpc-example:1.0-SNAPSHOT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like these coordinates to not be centralpark-specific.
kubernetes v1.10.0+b81c8f8 | ||
features: Basic-Auth | ||
|
||
Server https://centralpark.lightbend.com:443 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should replace each mention to centralpark.lightbend.com by
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. I think I'd rather get this PR merged ASAP and I'll do the suggested fix in #42 . Sounds good?
Must be rebased on top of
master
when #38 is merged.