Giter Club home page Giter Club logo

Comments (8)

Kubik42 avatar Kubik42 commented on July 23, 2024 1

@evelinec addressed in 37d0d91

from guide-kubernetes-intro.

Kubik42 avatar Kubik42 commented on July 23, 2024

It feels like explanation is needed for "Pods" and "Ingress controller" as of what they are. Looks like "Pod" is explained in the later section.

I didn't feel like they needing explaining in the introductory section since all I'm doing there is just briefly describing what the purpose of the guide is. I added a brief description for ingress controllers into the "Architecture" section. Pods are also explained in that section.

Can be helpful to include a link to read more about "Ingress".

I added a link to the Kubernete's concepts page to the end of the "Architecture" section if users find themselves wanting to know more about various Kubernetes resources..

The link in this step has an extra \ on the test site: kubernetes-helm, a package manager for Kubernetes called Helm. For installation instructions, see https://docs.helm.sh/using_helm/\#installing-helm.

This was fixed already, the site just wasnt updated.

...then check the Maven build log for any potential errors and make sure that your Docker CLI is configured to use Minikube’s Docker daemon and not your host’s as described in [Starting and preparing your cluster for Deployment]. => the link to the Starting and preparing your cluster for Deployment section is enclosed within the []. Is the [] really needed?

I actually don't have [] in my adoc, they are generated by the website.

Running the install a chart release commands gave error: Error: Chart incompatible with Tiller v2.10.0-rc.1

This is probably because the chart is outdated. My guess is that you have a newer Helm version than the chart currently supports.

If you need to use additional parameters or if you would like more information on the existing parameters, visit the official IBM chart GitHub repository. => I think "IBM chart GitHub repository" is to be read together, right? ie, the link to be applied to them together, instead of just the "GitHub repository". In general, to make this clearer, ie, perhaps "visit the official IBM Helm Chart in the GitHub repository"? I also think the link should be added to "IBM Helm Chart" as in "visit the official IBM Helm Chart in the GitHub repository".

Changed to "IBM Helm chart repository"

See the official Open Liberty Helm chart GitHub repository for more information on this parameter. => here you refer to the same repository again but with different name.

Formatted the same as the other hyperlink.

Working on Mac, when running the helm install command, there's a pop to accept the incoming network connections as a security and privacy preference. Not sure whether this should be noted.

I dont think so. This is the setup side of the guide, which is entirely up to the user.

Use of bold words in the guide. Can leave for ID to comment on it.

I think its fine because "unique" and "non-unique" are important and need to stand out more.

Different styles in inserting commands and outputs, ie, unix and windows commands together in one box; command and output in one box; command only and output only.

For command and output in the same box, I find this much nicer than separating them. Perhaps we can alter the copy button on the website to copy only the command when this kind of formatting is used. Otherwise it just feels distracting having listing blocks all over the place, especially when they could have been combined.

Same argument for putting windows and unix commands in the same listing block. Its going to look too cluttered when there are listing blocks all over the place. In this case especially, if the commands are separated into two blocks, then the users might think that they both need to be executed. Putting them in one block makes it more obvious that its either or.

Duration of guide (without optional section) seems long

Kubernetes is full of things and hence is not a short topic. I feel that I've gone into a good amount of detail that would be useful to anyone starting with Kubernetes. Some things I've added in definitely helped me along the way.

from guide-kubernetes-intro.

evelinec avatar evelinec commented on July 23, 2024

Didn't see changes or comment for the following points:

  • I think the "kubectl" command isn't needed. If you really think it's needed, it needs to be backticked? Here: Explore how to deploy microservices to Kubernetes and manage them with the kubectl Kubernetes CLI.

  • Regarding the [] for a link to refer back to a section, looks like it's the way the reference is working. You have "<<>>" in your adoc, and that gets rendered into [] on github and the website. Btw, when clicking this link on the test site, it doesn't seem to do anything.

  • VirtualBox is required when running minikube on Mac. I'm referring to this sentence, "If you’re on Windows and you run into issues starting Minikube in VirtualBox, try using Hyper-V instead since Hyper-V is already required to run Docker on Windows." It sounds like only Windows requires VirtualBox. I didn't have VirtualBox setup as part of the prereqs, cus it isn't clear it is needed. Util I get to this point of the guide, where the "minikube start" command failed. The sentence in quote above is the first thing after this command, but it only refers to Windows. I think it'd be good to list VirtualBox as a requirement in the prereqs section.

  • Working on Mac, when running the helm install command, there's a pop to accept the incoming network connections as a security and privacy preference. Not sure whether this should be noted.
    => actually this pop up shows up when running the helm chart release installation commands, ie, helm install --name ol-name --set image.pullPolicy=IfNotPresent --set image.repository=name --set image.tag=1.0-SNAPSHOT --set ssl.enabled=false --set service.port=9080 --set service.targetPort=9080 --set ingress.enabled=true --set ingress.secureBackends=false --set ingress.rewriteTarget=/api/name --set ingress.path=/name ibm-charts/ibm-open-liberty. This doesn't seem like setup side of the things..

The rest of the fixes are all good. Tested on openlibertydev site. Thank you.

from guide-kubernetes-intro.

evelinec avatar evelinec commented on July 23, 2024

Also to add, I changed my Helm version from "v2.10.0-rc.1" to "v2.9.1" then I was able to install the helm chart release successfully, re "Running the install a chart release commands gave error: Error: Chart incompatible with Tiller v2.10.0-rc.1."

from guide-kubernetes-intro.

Kubik42 avatar Kubik42 commented on July 23, 2024

I think the "kubectl" command isn't needed. If you really think it's needed, it needs to be backticked?

I already removed kubectl from the description. Which one are you referring to?

Regarding the [] for a link to refer back to a section...

I removed the section reference altogether.

VirtualBox is required when running minikube on Mac.

I think its up to the user to setup Minikube. The note about Windows is something Panagiotis ran into, so I think its worth including it.

Working on Mac, when running the helm install command

I dont think it should be noted then. Its part of the setup that the user is responsible for.

You can find the latest changes in e976c86

from guide-kubernetes-intro.

evelinec avatar evelinec commented on July 23, 2024

@Kubik42 Here's the sentence with the "kubectl" command: Explore how to deploy microservices to Kubernetes and manage them with the kubectl Kubernetes CLI.

from guide-kubernetes-intro.

evelinec avatar evelinec commented on July 23, 2024

New issues:

  • There are missing copyright header for the ping service client files

  • The copyright year in the readme.adoc need to be updated to 2018 from 2017

from guide-kubernetes-intro.

evelinec avatar evelinec commented on July 23, 2024

Reviewed changes. Looks good

from guide-kubernetes-intro.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.