nodejs-express: new template #698
Conversation
|
@ianpartridge PTAL |
|
@neeraj-laad Not sure on your prefs -- should this start in experimental so it can be promoted later to incubator? |
|
ping @neeraj-laad : #698 (comment) |
|
Strange, node-rdkafka failed to build after master was merged into this branch. I'll look into it. |
|
@sam-github just curious to understand why is this stack inheriting from |
|
Regarding your earlier comment on experimental vs incubator. Appsody does not mandate that all stacks start as experimental. As long as you are willing to maintain and try to get it to stable. However, if we are not 100% sure if this is a good idea or taking shortcuts for some behaviour, then we can put this in experimental too. |
I'll see if I can slim the stack down via inheritance, but I'm not sure it matters. There don't look to be any layers that are fruitfully inherited. Dockerfiles still have to contain the same contents, the /package gets overwritten and has different contents, the templates are different, the READMEs are self-contained. If there was some way of making the lower-stack layers contain things that inherited stacks could just use, inheritance would be more meaningful, but either because of the nature of node.js apps, or just the nature of stacks that they have a top-level app contained in them that then delegates some things to the template, I don't see a lot of inheriting going on. Even nodejs-express contains, AFAICT, basically just a verbatim copy of much of the contents of nodejs, even though it inherits from it. But maybe the stacks weren't written with enough attention towards inheritance. I'll see. |
|
I moved it to experimental because we don't know if we can commit to node-librdkafka for the longer term, even though it is best choice ATM. I copied the recent changes from the other nodejs stacks to not update system or npm packages. I'd prefer to leave the inheritance change, if it ends up being useful, to happen as part of a greater cleanup I'm working on. |
|
OK, passing, PTAL @neeraj-laad |
| app.use('/live', health.LivenessEndpoint(healthcheck)); | ||
| app.use('/ready', health.ReadinessEndpoint(healthcheck)); | ||
| app.use('/health', health.HealthEndpoint(healthcheck)); | ||
| app.use('/metrics', require('appmetrics-prometheus').endpoint()); |
experimental/nodejs-kafka/README.md
Outdated
|
|
||
| 1. A message may be posted into the default skeleton app. This could be done with `curl`: | ||
| ```bash | ||
| curl -v -X POST -H "content-type: application/json" \ |
There was a problem hiding this comment.
Running this command at first succeeds for me although eventually it looks like connection to the producer/consumer is lost:
[Container] {"level":50,"time":1584971907527,"pid":45,"hostname":"f4395b2e3ac3","msg":"error from consumer: Error: broker transport failure","v":1}
[Container] {"level":50,"time":1584971907527,"pid":45,"hostname":"f4395b2e3ac3","msg":"error from consumer: Error: all broker connections are down","v":1}
which then results in failures running this.
Also nit: You could cut out the use of the -X flag. Not a big deal but I get this note when running the command:
Note: Unnecessary use of -X or --request, POST is already inferred.
There was a problem hiding this comment.
I see this problem too. Are you running the https://github.com/lensesio/fast-data-dev image that Sam suggested? I also tried using a docker-compose.yaml for Kafka that I had lying around, but that has the same problem.
|
Can I suggest we change the README away from the Lenses.io image, and use Docker Compose running Strimzi instead - https://github.com/scholzj/strimzi-compose-up version: '2'
services:
zookeeper:
image: strimzi/kafka:0.15.0-kafka-2.3.1
command: [
"sh", "-c",
"bin/zookeeper-server-start.sh config/zookeeper.properties"
]
ports:
- "2181:2181"
environment:
LOG_DIR: /tmp/logs
kafka:
image: strimzi/kafka:0.15.0-kafka-2.3.1
command: [
"sh", "-c",
"bin/kafka-server-start.sh config/server.properties --override listeners=$${KAFKA_LISTENERS} --override advertised.listeners=$${KAFKA_ADVERTISED_LISTENERS} --override zookeeper.connect=$${KAFKA_ZOOKEEPER_CONNECT}"
]
depends_on:
- zookeeper
ports:
- "9092:9092"
environment:
LOG_DIR: "/tmp/logs"
KAFKA_ADVERTISED_LISTENERS: PLAINTEXT://localhost:9092
KAFKA_LISTENERS: PLAINTEXT://0.0.0.0:9092
KAFKA_ZOOKEEPER_CONNECT: zookeeper:2181 |
|
READMEs don't usually describe how to setup the backend services, I think I should remove the note, anyone contemplating using Kafka should already have their favourite way to run it. |
|
I'm still looking at how to fix this, but at this point I've convinced myself its a network config issue, I removed the appsody layer and just ran the scaffolded app.js locally on my host, and it works fine, but doesn't inside appsody run's docker container. cf. https://www.confluent.io/blog/kafka-listeners-explained/, the 9092 port is reachable, the client connects, then drops the connection. I think we hit a variant of "Why can I connect to the broker, but the client still fails?". |
|
@Kamran64 So, there are two issues here, the first of which seems a generic problem, how does someone usually run a container in development with
On Linux, this works, because
The general recommendation here seems to be "use docker-compose", which I did, and it works, but not with The kube approach would be to deploy kafka and zookeeper into kube, create a service for kafka, and use that service name to connect from the appsody nodejs-kafka container (in production) and in dev, do the same, except do a |
|
@neeraj-laad ^--- see above, what is appsody recommendation for a container run with |
There was a problem hiding this comment.
@dalelane As a Kafka expert, could you give the sample code here a look to see if you think its reasonable? The template is loosely based on the event streams sample: https://github.com/ibm-messaging/event-streams-samples/tree/master/kafka-nodejs-console-sample
|
I'm still not a Kafka install expert, but @dalelane of the event team helps maintain https://github.com/strimzi/strimzi-kafka-operator#quickstart, perhaps we should link to it in the README? |
|
https://github.com/appsody/stacks/blob/fc83e6b1722054f9b809337c57da5e41565c4341/incubator/java-spring-boot2/templates/kafka/README.md <--- perhaps this should be aligned with the java kafka stack in its README? And possibly in what it does in the default template? |
|
@ianpartridge I see #742 landed, and they figured out how to use appsody run with kafka, I synced the instructions here with that PR. PTAL |
|
Turns out its become a requirement that all the kafkas be templates, I copied the README from the java-spring-boot2 (basically), and made this a template. The libssl deps can only be added in the stack's dockerfile, and nodejs-express stack (and likely all of the nodejs stacks) doesn't support the user's app depending on compiled addons (permissions problems on the stack user's home directory, /root), which is why rdkafka was added as a dep of the image/project (fixing this bug in user setup is out of scope for this PR). to: @ianpartridge |
There was a problem hiding this comment.
Looks good! Just a few notes inline.
Can we ship a sample docker-compose.yaml for Kafka in the template? Like this:
https://github.com/appsody/stacks/pull/775/files#diff-f8c1fd8a01a259e6ef300c40dce4a300
|
copied in docker-compose file PTAL @ianpartridge |
ianpartridge
added
the
stack/nodejs-express
|
@neeraj-laad Can you land? |
|
Once we bump the stack version, this is good to go |
|
@Kamran64 done |
Fixes: #692
Related: #695
Checklist:
Read the Code of Conduct and Contributing Guidelines.
Followed the commit message guidelines.
Stack adheres to Appsody stack structure.
Modifying an existing stack:
stack.yamlContributing a new stack:
Describe how application dependencies are managed:
Explain how Appsody file watcher is utilized:
Describe other Appsody environment variables defined by the stack image:
Describe any limitations and known issues:
Related Issues: