Add Kubernetes config examples for endpoints + gRPC #666
Conversation
googlebot
added
the
cla: yes
|
@jeffmendoza Can you review? |
| "-p", "80", | ||
| "-P", "9000", | ||
| "-s", "SERVICE_NAME", | ||
| "-v", "SERVICE_VERSION", |
There was a problem hiding this comment.
Now we should call it SERVICE_CONFIG_ID
There was a problem hiding this comment.
fyi, the public doc still says "SERVICE_VERSION": https://cloud-dot-devsite.googleplex.com/endpoints/docs/quickstart-container-engine
@jeffmendoza
how would you recommend? we need to settled down one and be consistency
| spec: | ||
| containers: | ||
| - name: esp | ||
| image: b.gcr.io/endpoints/endpoints-runtime:latest |
There was a problem hiding this comment.
It's K8s best practice to avoid the use of :latest and specify a version.
There was a problem hiding this comment.
that is a good question. As discussed within the team, we are planning to use :latest for samples, so we don't have to keep updating versions when we have new release, but adding a link to release page here and also comment that:
- :latest is for sample/testing only.
- For production, we recommend a stable version from the release page, i.e. b.gcr.io/endpoints/endpoints-runtime:1.0 for GA version
How do you think?
There was a problem hiding this comment.
I think we should follow best practices where we can, even if it means more maintenance. http://kubernetes.io/docs/user-guide/config-best-practices/ People often take samples as-is without reading docs or notices. We shouldn't provide samples for testing only, when it is simple enough to have the actual version without any caveats.
That said, for docker images it is typical to have hierarchical tags, sort of like semantic versioning. ESP should have a major version tag, such as :1 that points to the latest point release, e.g. :1.2, and is updated with new point releases. It would be fine to have a higher-level major version tag here.
There was a problem hiding this comment.
yes, ESP is planning to have :1 pointing to the latest major.minor version, and :1.x for specific major.minor stable release.
How about we use major.minor, 1.0 here? since 1.1 will introduce new features and I am not sure if users wants to switch over by default.
and in this case, should we tag :1.0 for the python grpc backend docker as well, for consistency
There was a problem hiding this comment.
I take back what I said, :1 looks good to me, move to :2 will take a LONG time. we may upgrade from :1.0 to :1.1 in a quarter.
similarly, we can tag the backend as :1 too.
Let me know if you agree?
There was a problem hiding this comment.
Cool.
I added the 1.0.0 tag for the python backend in the container registry. Will update this PR in a minute.
| - containerPort: 80 | ||
| - containerPort: 9000 | ||
| - name: bookstore | ||
| image: gcr.io/endpointsv2/python-grpc-bookstore-server:latest |
There was a problem hiding this comment.
how about we make that version consistent with the grpc python version we used to build this docker image? :1.0.1
* Use fixed docker image versions instead of :latest
|
@jeffmendoza I've changed this PR to use fixed version number. PTAL. |
|
@jonparrott can you merge this PR if you don't have extra comment? I don't have write access to this repo so can't merge it myself. |
|
cc @ryanmats Jon is sick today |
| "-P", "9000", | ||
| "-s", "SERVICE_NAME", | ||
| "-v", "SERVICE_CONFIG_ID", | ||
| "-a", "grpc://localhost:8000" |
* Remote port 80 and only expose the gRPC ports
| - name: esp | ||
| image: b.gcr.io/endpoints/endpoints-runtime:1 | ||
| args: [ | ||
| "-p", "80", |
There was a problem hiding this comment.
This is not needed as well.
There was a problem hiding this comment.
You probably want two versions of this file, one for GRPC passthrough and one for GRPC with transcoding. I'd also want another one without ESP.
There was a problem hiding this comment.
I'd argue that should be added in a followup PR
No description provided.