Review on 10/11's draft

[evelinec]

What you'll learn

• [x] Kubernetes polls the endpoint as specified by the liveness and readiness probes. => it feels like there's something missing for this sentence. ie, Kubernetes polls the endpoint as specified by the liveness and readiness probes (to do something..)?

• [x] It may be good to add a link to the health guide, if user wants to learn the basics of MP health.

• [x] It feels like you may want to describe the microservice(s) that user will be working with in this section, as the ping microservice is used in a heading later on.

Prereqs

• [x] After following one of the sets of instructions, ensure that Kubernetes (not Swarm) is selected as the orchestrator. => it may be clearer to add that, this step is to done in the Docker Preferences. ie, After following one of the sets of instructions, ensure that Kubernetes (not Swarm) is selected as the orchestrato in Docker Preferences.

Add health check to ping microservice

• [x] Should add a line saying, "navigate to the start directory to begin"?

• [x] There are two applications that you will work with — name and ping. => applications? or microservices, or services?

• [x] The ping microservice is used in this section's heading, but it's really the first time being introduced in this guide. Either update to heading to not pointing out the ping microservice, or add a quick intro of the microservices in the "what you'll learn" section.

• [x] The name microservice already has a health check implemented, so you will implement a health check for ping. => why is the health check implemented for the name service already? Also perhaps to mention about this at the end of this section instead of the first paragraph. It sounds like something is done for user already then what's the point of learning to implement the health check for the ping service?

• [x] isNameServiceReachable method => the name-service hostname is hard coded in this method. Perhaps to use the HOSTNAME variable? Also, for the http://name-service:9080/api url in this method, can you get the port from runtime? basically, we should not hard code the port number.

Configure readiness and liveness probes

• [x] I think it may be less error-prompt to have user created the entire kubernetes.yaml file instead of updating part of it, to get around with the indentation etc.

Deploy name and ping microservices

• [x] Again, not sure the name and ping service names need to be in the heading.

• [x] When the pods are in the ready state, navigate to http://[hostname]:31000/api/name and observe a response similar to Hello! I’m container name-deployment-5f868854bf-2rhdq. => A clear indication of what the [hostname] is, seems needed here to help with usability.

• [x] This scenario can occur when the container has started, but Open Liberty hasn’t fully initialized. => instead of "Open Liberty", perhaps say "the application server" hasn't fully initialized? what do you think?

• [x] I was wondering why there are 2 name pods and 1 ping pods. May be helpful to add a quick note about that, even if it's just for this guide's purpose. ie, You will see two name pods and one ping pod that are needed for this guide.

Trigger restart in name microservice and Trigger restart in name microservice and observe effects on ping microservice

• [x] Kind of think these two sections can be combined, and "obverving the effects on the ping microservices" can be a sub-section. The reason being, these two sections are essentially similar. One other reason is to get rid off mentioning the service names in the heading. It's not that things aren't clear as they are now, just a bit of repetitiveness, and needing to mention the service names in the heading.

Testing the microservices

• [x] So the tests aren't really testing the health checks, just checking the two microservices are functioning accordingly, right? Then, this section is not useful/meaningful to this guide.

Related guides

• [x] I think this guide would be also related to the kube-intro guide, but not the kube-config guide? Not sure about Istio neither.

General

• [x] Most of the section headings do not start with a gerund. They should, as described by our styles guideline.

• [x] It's a very nice guide!

[proubatsis]

@evelinec

why is the health check implemented for the name service already?

Because we provide them an endpoint that allows them to make the service unhealthy to test the probes later in the guide, it wouldn't really be a valuable experience for the user to create this health check. The ping health check is considerably different then the one for name.

Perhaps to use the HOSTNAME variable? Also, for the http://name-service:9080/api url in this method, can you get the port from runtime?

We can't use the HOSTNAME environment variable in this case, but I can configure it elsewhere so it is not hardcoded in the file.

A clear indication of what the [hostname] is, seems needed here to help with usability.

~Currently working out what to do with this for the config guide, so we'll update this once we sort that out.~

The kube intro guide does this already, we'll do the same thing here.

So the tests aren't really testing the health checks, just checking the two microservices are functioning accordingly, right? Then, this section is not useful/meaningful to this guide.

I'll see what can be done with testing the health checks and probes first before we decide to remove the section.

[proubatsis]

@evelinec

6

https://github.com/OpenLiberty/guides-common/pull/196

[evelinec]

Reviewed all the changes, they look good. Approve for merging, then, refresh the test site, andI'll review again. Thanks.