SME Review Checklist

[justineechen]

SME Review

• [ ] Ensure technical accuracy for code
• [ ] Ensure technical accuracy for guide
• [ ] Ensure technical accuracy for overall content

[andrewdes]

SME gave the following feedback:

• [x] you specify t2.small worker nodes, which is just 1 core, 2GB memory. is that a good recommendation for worker node size for running two java microservices? presumably we’re deploying to EKS over minikube because it’s for “production” and is “highly available”
• [ ] similarly the kube yaml has no resource requests or limits in the deployment, which developers should really add since this affects whether workloads get evicted
• [ ] might be also helpful to have a healthcheck in the deployment yaml (and code), partifularly you do a rolling update but i think kube just kills the old container and sends traffic to the new one without seeing if the new container is ready yet
• [ ] it might be useful to review some of the AWS capabilities with EKS for collecting logs and metrics, particularly how openliberty can expose metrics and logs to external tools, particularly cloudwatch
• [ ] you specify NodePort service types and then you need to manually open the security groups. would it be better to use LoadBalancer service type here? it will result in an ELB created for each service and the security groups should be opened automatically. In general if the idea is to allow developers to be able to develop microservices with minimal operations knowledge, having to call an operations guy to step in and open NodePorts on security groups should be discouraged. understandably there may be cost concerns with allowing developers to just create LoadBalancers as well.

[andrewdes]

@gkwan-ibm

similarly the kube yaml has no resource requests or limits in the deployment, which developers should really add since this affects whether workloads get evicted

might be also helpful to have a healthcheck in the deployment yaml (and code), partifularly you do a rolling update but i think kube just kills the old container and sends traffic to the new one without seeing if the new container is ready yet

If we agree to move forward with the above changes, I believe the other cloud guide and perhaps kubernetes guides should make similar changes.

it might be useful to review some of the AWS capabilities with EKS for collecting logs and metrics, particularly how openliberty can expose metrics and logs to external tools, particularly cloudwatch

Perhaps this is outside the scope of this guide which is focused on deploying applications to EKS. Maybe we could have another guide that showcases some of these features? Similar to what we did with the kube-into, kube-config, kube-health guides.

you specify NodePort service types and then you need to manually open the security groups. would it be better to use LoadBalancer service type here? it will result in an ELB created for each service and the security groups should be opened automatically. In general if the idea is to allow developers to be able to develop microservices with minimal operations knowledge, having to call an operations guy to step in and open NodePorts on security groups should be discouraged. understandably there may be cost concerns with allowing developers to just create LoadBalancers as well.

The rest of our Kubernetes guides and cloud guide use NodePorts, is it more important to keep consistent with those guides? Also, as mentioned, LoadBalancers will impose an additional charge for users completing the guide.

[yeekangc]

We should call out these considerations where appropriate in the guide e.g. size of the nodes.

I am open to adding Health Check, integrating it with Kube and having it there OOTB. This is good practice though it won't be the focus of this guide here.

Yes, capabilities for logs and metrics should be a separate guide.

We may need to revisit NodePort in general. Can consider a separate issue for that covering the relevant guides.

[jkwong888]

regarding resource requests, limits and health check, these should really be in the examples, even if you don't discuss them. resource requests and limits tell the platform how to place containers and also become important for autoscaling. health check shouldn't be optional as it's especially important during your rolling update as without one you'll be taking downtime when you roll the deployment over.

for logging, you may want to have the guide show how to stream the container logs using kubectl, even if you don't use cloudwatch. the base container image with the app server probably configures logging to std out which is enough to plug into logging frameworks.

in general I don't think we should be recommending NodePort unless it's in development only scenarios like in minikube. additionally, i think only a user facing service like an API or frontend you expose outside of the cluster would need a LoadBalancer service type; if it's an internal API like a backend microservice, you may be able to get away with running a pod from inside the cluster that calls curl. LoadBalancer typically needs an external cloud provider integration (in AWS case, ELB is created); on other kube platforms it really depends on if the cloud provider integration is present.

there's nothing really in this guide specific to AWS beyond how to create an EKS cluster or use ECR, which might make it easy to re-use some of the materials that are kube agnostic (i.e. the yamls).

[jkwong888]

btw i don't know if this is a nit or not but we mention "docker" and "docker images" a lot, where I think we might want to just call them "containers" and "container images" ... docker is a particular developer toolset to build container images, which technically is not necessary to run a container is kubernetes. it's fine that we use "Docker" and "Dockerfile" to create the image, but once it's built it's a "container image" and when it's running it's a "containers"

[evelinec]

Team have determined to work on the rest of the issue post publishing.