Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review feedback #12

Closed
yeekangc opened this issue Aug 1, 2019 · 7 comments
Closed

Review feedback #12

yeekangc opened this issue Aug 1, 2019 · 7 comments
Assignees

Comments

@yeekangc
Copy link
Member

yeekangc commented Aug 1, 2019

  • "For instance, virtual machines, databases, and storage networks are types of resources that resource groups can be formed from." -- I find "can be formed from" odd. Can we rephrase?

  • "Resource groups are dedicated to container registries because the resources in these registries are used by several different hosts and their containers." -- I find the use of "dedicated" here odd and I don't quite understand what we are trying to say here. Can we rephrase too?

  • "Since your application involves a lower usage of storage and image throughput, you are using a Basic SKU, due to its cost-optimized solution." -- Use of "cost-optimized solution" is weird. I will simply say it's cheaper if not cheapest option.

  • "If you’re unfamiliar with Dockerfiles, check out the Using Docker containers to develop microservices guide." -- Should point to the Containerize guide instead.

  • az aks create with service-principal and client-secret: Do we need copyable for the command provided? This is optional, right? Can we call out when or in what case someone will need to do this?

  • "Replace [password] with the registry password that you viewed in the last section by running the az acr credential show -n guideRegistry command." -- I don't think it's the "last section"?

  • "Now that your container images are built, you can deploy them using a Kubernetes resource definition." -- We have done more than building our container images? The cluster is ready too?

  • "It is important to clean up your resources when you are finished with the guide so that you do not incur additional charges for ongoing service." -- What do we mean by "ongoing service"?

  • Should not have the Docker guide as a related guide.

  • Remember to update the front matter of README.adoc where necessary before publishing.

@yeekangc
Copy link
Member Author

yeekangc commented Aug 1, 2019

FYI, @gkwan-ibm.

@NimG98
Copy link
Contributor

NimG98 commented Aug 1, 2019

"For instance, virtual machines, databases, and storage networks are types of resources that resource groups can be formed from." -- I find "can be formed from" odd. Can we rephrase?

Rephrased to:

  • "For instance, resource groups can manage virtual machines, databases, and storage networks together as one collection."

"Since your application involves a lower usage of storage and image throughput, you are using a Basic SKU, due to its cost-optimized solution." -- Use of "cost-optimized solution" is weird. I will simply say it's cheaper if not cheapest option

Rephrased to:

  • "You are using a Basic SKU because it has a cheaper cost and your application involves a lower usage of storage and image throughput."

az aks create with service-principal and client-secret: Do we need copyable for the command provided? This is optional, right? Can we call out when or in what case someone will need to do this?

I rephrased the sentences to explain that this option is included if the user has a pre-existing service principal that they want to use. Also, I changed the green command box to a grey box.

  • "Optionally, a cluster can be created with existing service principals. The az ad sp create-for-rbac command manually creates a service principal and outputs information about it, including an appId and password. In the az aks create cluster creation command, these two values can be passed into the additional arguments, --service-principal and --client-secret:"

    az aks create \
        -g guideGroup \
        -n guideCluster \
        --service-principal [appId] \
        --client-secret [password]
    

"Replace [password] with the registry password that you viewed in the last section by running the az acr credential show -n guideRegistry command." -- I don't think it's the "last section"?

I moved the viewing the registry password command to the same section that the password is needed.

  • "View the password for your Azure container registry:"
    az acr credential show -n guideRegistry --query "passwords[0].value" -o tsv
    
    ...
  • "Replace [password] with the registry password that you viewed with the
    az acr credential show -n guideRegistry command."

"Now that your container images are built, you can deploy them using a Kubernetes resource definition." -- We have done more than building our container images? The cluster is ready too?

Rephrased to:

  • "Now that you have created a cluster and your container images are built, you can deploy the images using a Kubernetes resource definition."
  • "Resource groups are dedicated to container registries because the resources in these registries are used by several different hosts and their containers." -- I find the use of "dedicated" here odd and I don't quite understand what we are trying to say here. Can we rephrase too?
  • Should not have the Docker guide as a related guide.

Removed that sentence.
Removed docker guide from related guides.

@NimG98
Copy link
Contributor

NimG98 commented Aug 1, 2019

Here are the changes: 8864246

@yeekangc
Copy link
Member Author

yeekangc commented Aug 1, 2019

  • Can we call out why a resource group is needed?

  • Consider "You are using a Basic SKU because it is cheaper and the services you will deploy have low storage and throughput requirements."

  • If the idea is that users will work with an existing service credential, then we would call out the command to retrieve the credential as opposed to the command to create a new one? Should rework further.

  • Consider "Now that your container images are built and created a Kubernetes cluster, you can deploy the images using a Kubernetes resource definition."

@NimG98
Copy link
Contributor

NimG98 commented Aug 1, 2019

Is this okay for calling out why a resource group is needed?

A resource group contains resources that are usually bound to a specific application, and dictates that these resources share the same lifecycle. It is needed to control the communication between resources, manage resource access, and easily filter the cost of resources per application. Azure container registries will act as one of these resources within your resource group.

@yeekangc
Copy link
Member Author

yeekangc commented Aug 1, 2019

I will consider something like:

A resource group is an Azure construct to manage a logical collection of resources for your cloud deployments on Azure. For this guide, you will create a new resource group to manage the resources you need for your Kubernetes deployment.

@yeekangc
Copy link
Member Author

yeekangc commented Aug 1, 2019

Also, "To view the credentials of your AKS cluster:" doesn't seem to go with the output that we show following the command. May be we need to update this explanation of the command.

@NimG98 NimG98 closed this as completed Aug 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants