Format update #124

OGtrilliams · 2018-05-24T19:17:22Z

made a few grammatical changes, added hyperlinks to CRI github for those who don't know what it is, & reworked formatting a bit.

jodh-intel

Hi @klynnrif - ptal.

jodh-intel · 2018-05-25T10:38:41Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md


 ## Requirements 
 - Kubernetes, kubelet, kubeadm
 - cri-containerd
 - Kata Containers

-For information about the supported version of these components see 
-Kata Containers [versions.yaml](https://github.com/kata-containers/runtime/blob/master/versions.yaml) file.
+Note|


I like the way you've made this stand out, but could you change it to use the standard formatting we use for notes? I'm afraid this hasn't historically been written down, but I've just raised the following to rectify that 😄: #126.

Hi @OGtrilliams - I see your note is still in the original style, not formatted as per the guideline @jodh-intel detailed at https://github.com/kata-containers/documentation/blob/master/Documentation-Requirements.md#notes - could you make that small change please?

jodh-intel · 2018-05-25T10:40:44Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md

@@ -1,15 +1,18 @@
 # How to use Kata Containers and CRI (containerd plugin) with Kubernetes

 This document describes how to set up a single-machine Kubernetes cluster. 
-The Kubernetes cluster will use the CRI containerd plugin and Kata Containers to launch untrusted workloads.
+The Kubernetes cluster will use the [CRI containerd plugin](https://github.com/containerd/cri) and [Kata Containers](https://katacontainers.io) to launch untrusted workloads.


We've tended to specify https://github.com/kata-containers as the project homepage for docs on github. Note that that URL does contain a link to https://katacontainers.io but the former is going to be quicker to navigate to find the code I think.

jodh-intel · 2018-05-25T10:41:38Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md


 #### Define the Kata runtime as `untrusted_workload_runtime` 

 Configure the Kata runtime for untrusted workload with the [config option](https://github.com/containerd/cri/blob/v1.0.0-rc.0/docs/config.md)
 `plugins.cri.containerd.untrusted_workload_runtime`.

 Unless configured otherwise, the default runtime is set to `runc`.
+
+- Configure containerd to use Kata as `untrusted_workload_runtime`


Maybe add a colon at the end of this sentence?

jodh-intel · 2018-05-25T10:50:37Z

Hi @OGtrilliams - thanks for raising!

Our process requires that commits conform to a certain format. Travis is failing as a result and referencing this doc which explains the format of commits our CI tooling checks for:

https://github.com/kata-containers/community/blob/master/CONTRIBUTING.md#patch-format

Basically, your commit needs to add:

A signed-off-by comment.
A "subsystem" tag in the summary line.
A "Fixes" comment referencing a github issue the PR is fixing (I've raised Improve formatting in cri containerd howto #127 for you to use).

Hence, if you squash your commits into a single commit, you could change your commit to something like the following (obviously change to add your name+email address) to keep the CI happy:

docs: Improve formatting in CRI Containerd howto

Updated the CRI Containerd howto guide to improve the formatting.

Fixes #127.

Signed-off-by: Your Name <your@email.address>

klynnrif

Scrubbed for grammar, flow, and structure. A few changes. Thanks!

klynnrif · 2018-05-25T15:22:21Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md

- Default runtime: A runtime that is used by default to run workloads.
- Untrusted workload runtime: A runtime that will be used run untrusted workloads.
+- **Default runtime:** A runtime that is used by default to run workloads.
+- **Untrusted workload runtime:** A runtime that will be used run untrusted workloads.


Suggested rewrite to stay active (and adding in a missing "to": Untrusted workload runtime: A runtime that is used to run untrusted workloads.

klynnrif · 2018-05-25T15:23:26Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md

@@ -71,23 +78,29 @@ EOT

 ### Configure Kubelet to use containerd

-In order to allow kubelet use containerd (using CRI interface) configure the service to
-point to containerd socket.
+In order to allow kubelet use containerd (using CRI interface), configure the service to


In order to allow kubelet, use containerd (using CRI interface) and configure the service to

klynnrif · 2018-05-25T15:27:01Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md

 $ sudo systemctl daemon-reload
 ```

 ### Optional: Configure proxy

-If you are behind a proxy this script will configure your proxy for docker
-kubelet and containerd.
+If you are behind a proxy, use this script to configure your proxy for docker,


If you are behind a proxy, use the following script to configure your proxy for docker,

klynnrif · 2018-05-25T15:27:40Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md

-If you are behind a proxy this script will configure your proxy for docker
-kubelet and containerd.
+If you are behind a proxy, use this script to configure your proxy for docker,
+kubelet, and containerd.


I suggest using a colon at the end of this sentence rather than a period.

klynnrif · 2018-05-25T15:29:18Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md

-If a pod has the `io.kubernetes.cri.untrusted-workload annotation` set as
-`"true"`, the CRI plugin will run the pod with the Kata Containers runtime.
+If a pod has the `io.kubernetes.cri.untrusted-workload` annotation set to
+`"true"`, the CRI plugin will run the pod with the [Kata Containers runtime](https://github.com/kata-containers/runtime).


Suggested rewrite to stay active: "true", the CRI plugin runs the pod with the Kata Containers runtime.

OGtrilliams · 2018-05-30T16:33:49Z

Thanks everyone! I'll go back & fix everything you all suggested - but I'll just let you know ahead of time that I'm terrible at rebasing so... this might take a while. 😺

jodh-intel · 2018-05-30T16:42:53Z

Thanks @OGtrilliams. No problem. I think all you'll need to do is:

Update the doc with the review comments.
Create a new commit (git commit -asm latest).
git rebase -i master
Change the bottom two pick lines to squash, save and quit the editor.
git push -f.

grahamwhaley · 2018-06-11T11:01:07Z

gentle ping to @OGtrilliams - just wondering if you'll get to update this PR at some point?

OGtrilliams · 2018-06-12T21:07:24Z

@grahamwhaley doing The Things right now!

jodh-intel · 2018-06-13T08:20:18Z

Hi @OGtrilliams - we're getting close but the CI checks are failing. What you need to do is change the format of your commit slightly to match the doc the Travis log is pointing you at (https://github.com/kata-containers/community/blob/master/CONTRIBUTING.md#patch-format).

Something like this should do it:

howto: k8s grammar and format changes

Went through and made a few grammar and format updates to make how-to guide
easier to follow, as well as adddirect links to project Github pages where applicable.

Fixes: #127

Signed-off-by: trilliams <tribecca@tribecc.us>

You need to do the rebase I mentioned above as well to squash the two commits into one as they are a single logical unit.

jodh-intel · 2018-06-21T09:59:18Z

Hi @OGtrilliams - do you still plan to fixup the commit as mentioned above? We'd really like your contribution so feel free to join us on irc/slack if you want any help with this: https://github.com/kata-containers/community/blob/master/CONTRIBUTING.md#contact.

OGtrilliams · 2018-06-21T18:02:03Z

I think it's fixed @jodh-intel - going through checks now.

intelkevinputnam · 2018-07-23T17:46:20Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md

-If a pod has the `io.kubernetes.cri.untrusted-workload annotation` set as
-`"true"`, the CRI plugin will run the pod with the Kata Containers runtime.
+If a pod has the `io.kubernetes.cri.untrusted-workload` annotation set to
+`"true"`, the CRI plugin runs the pod with the [Kata Containers runtime](https://github.com/kata-containers/runtime).


Please link directly to the readme.md for the runtime.

intelkevinputnam · 2018-07-23T17:46:21Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md


 ```bash
-# allow master node run pods
+# allow master node to run pods
 $ sudo -E kubectl taint nodes --all node-role.kubernetes.io/master-
 ```


 ### Create a unstrusted pod using Kata Containers


Create an untrusted ...

intelkevinputnam · 2018-07-23T17:46:26Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md

- Default runtime: A runtime that is used by default to run workloads.
- Untrusted workload runtime: A runtime that will be used run untrusted workloads.
+- **Default runtime:** A runtime that is used by default to run workloads.
+- **Untrusted workload runtime:** A runtime that will be used to run untrusted workloads.



I think it would be useful to tell the user why they would use the untrusted workload runtime with the CRI plugin. Also, it might be worth explicitly stating that no further configuration is needed to use the CRI plugin with the default runtime if that is the case.

intelkevinputnam · 2018-07-23T17:46:28Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md

@@ -46,20 +46,24 @@ $ command -v kubeadm

 ### Configure containerd to use Kata Containers


I don't understand why this is a sub-heading of Install Kubernetes.

intelkevinputnam · 2018-07-23T17:46:29Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md

@@ -46,20 +46,24 @@ $ command -v kubeadm



The link on line 39 goes to 404. Also, why are the code blocks commented out?

intelkevinputnam · 2018-07-23T17:46:33Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md

-This document describes how to set up a single-machine Kubernetes cluster. 
-The Kubernetes cluster will use the CRI containerd plugin and Kata Containers to launch untrusted workloads.
+This document describes how to set up a single-machine Kubernetes cluster.
+The Kubernetes cluster will use the [CRI containerd plugin](https://github.com/containerd/cri) and [Kata Containers](https://github.com/kata-containers) to launch untrusted workloads.


cluster uses the

I'd recommend linking directly to the readme.md for the CRI plugin link, and Kata containers should link to: katacontainers.io or perhaps no link as we're already in a project hosted on github.com/kata-containers.

egernst · 2018-07-23T22:01:10Z

@OGtrilliams - looks like a few text updates and we should then be good to merge. Can you PTAL? Thanks!

jodh-intel · 2018-07-30T10:28:47Z

Hi @OGtrilliams - please could you update? If you don't have the time, please let us know and we can assist.

Went through and made a few grammar and format updates to make how-to guide easier to follow, as well as add direct links to project Github pages where applicable. Fixes: kata-containers#127 Signed-off-by: trilliams <tribecca@tribecc.us> howto: k8s grammar and format changes Went through and made a few grammar and format updates to make how-to guide easier to follow, as well as add direct links to project Github pages where applicable. Fixes: kata-containers#127 Signed-off-by: trilliams <tribecca@tribecc.us>

grahamwhaley · 2018-08-06T12:53:45Z

/me gently nudges @OGtrilliams as there has been activity on their github account recently, so I know they are not on holiday ;-) - any update/refresh or ETA for this? (an ETA will help us know when to re-visit the PR ;-) ). thx.

OGtrilliams · 2018-08-06T14:57:34Z

@grahamwhaley sorry! I made final updates about a week ago, but didn't comment here to announce it. It should be good for final review.

grahamwhaley

A couple of notes (no pun intended!).
@egernst - a couple were tech queries - were you the originator of some of this? (I've lost track..)

grahamwhaley · 2018-08-06T16:32:45Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md


 ## Requirements 
 - Kubernetes, kubelet, kubeadm
 - cri-containerd
 - Kata Containers

-For information about the supported version of these components see 
-Kata Containers [versions.yaml](https://github.com/kata-containers/runtime/blob/master/versions.yaml) file.
+Note|


Hi @OGtrilliams - I see your note is still in the original style, not formatted as per the guideline @jodh-intel detailed at https://github.com/kata-containers/documentation/blob/master/Documentation-Requirements.md#notes - could you make that small change please?

grahamwhaley · 2018-08-06T16:40:52Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md


-# Prevent docker iptables rules conflict with k8s pod communication
+```bash
 $ sudo iptables -P FORWARD ACCEPT


I'm an iptables numpty - is this basically disabling a bunch of iptables functionality globally? If so, maybe we want to put some sort of note/warning in here to tell users this is a dev/test measure, and for any sort of deployment they might want to do something more 'robust' ?

grahamwhaley · 2018-08-06T16:41:47Z

how-to/how-to-use-k8s-with-cri-containerd-and-kata.md

-# Start cluster using kubeadm
+-  Start cluster using `kubeadm`
+
+```bash
 $ sudo kubeadm init --skip-preflight-checks \


Are we merely skipping pre-flight tests to save some time during development?

intelkevinputnam · 2018-08-06T17:19:16Z

I'd like to see what the output of the verification steps looks like (command -v ...).

Can we add terminators ("." or ":") for sentences in bullets and preceding instructions.

bergwolf · 2018-08-14T08:06:13Z

@OGtrilliams Could you please address @grahamwhaley's comments?

caoruidong · 2018-09-03T12:05:15Z

Ping @OGtrilliams . Any update?

grahamwhaley · 2018-09-18T09:35:09Z

Hi @OGtrilliams - let us know if you can come back and visit this PR or not - if not, we'll put it up for adoption under the assisted workflow case, and try to wrap it up and land it...

jodh-intel · 2018-09-24T10:17:05Z

🔔 🔔 Outatime [*] 😄

Applying the boilerplate text from https://github.com/kata-containers/community/blob/master/CONTRIBUTING.md#assisted-pr-workflow...

Thank you for your contribution but we are unable to merge it until all review feedback has been applied and all CI checks have passed.

Since we have not heard from you for a while, we assume you are unable to progress this PR. As such, we intend to create a new PR based on this one, and resolve the identified issues on the new PR. The new PR will reference this PR for tracking purposes.

This PR will be marked do-not-merge and will be closed when our new PR has been merged. We will credit all authors of this PR on the new PR.

[*] - Note to self - gotta watch that film again soon! :)

This PR is based on kata-containers#124 but has been reworked and updated to take into account review feedback and extra cleanups to bring this howto in line with the latest documentation requirements. Fixes kata-containers#127. Contributions-from: T. Nichole Williams <tribecca@tribecc.us> Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

This PR is based on kata-containers#124 but has been reworked and updated to take into account review feedback and extra cleanups to bring this howto in line with the latest documentation requirements. Fixes kata-containers#127. Signed-off-by: T. Nichole Williams <tribecca@tribecc.us> Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

jodh-intel · 2018-09-24T11:10:34Z

Replaced by #251.

This PR is based on kata-containers#124 but has been reworked and updated to take into account review feedback and extra cleanups to bring this howto in line with the latest documentation requirements. Fixes kata-containers#127. Signed-off-by: T. Nichole Williams <tribecca@tribecc.us> Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

raravena80 · 2018-09-28T16:39:20Z

@jodh-intel should we close this PR? Since it has been replaced by #251?

jodh-intel · 2018-09-28T16:44:19Z

Hi @raravena80 - yep, it will be once #251 lands (see https://github.com/kata-containers/community/blob/master/CONTRIBUTING.md#assisted-pr-workflow)

This PR is based on kata-containers#124 but has been reworked and updated to take into account review feedback and extra cleanups to bring this howto in line with the latest documentation requirements. Fixes kata-containers#127. Signed-off-by: T. Nichole Williams <tribecca@tribecc.us> Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

grahamwhaley · 2018-10-04T12:24:34Z

I've just landed #251 If we know of remaining gaps, let's open Issues to cover them.
Closing this one as superseded by #251 then.

…Proxy builder: support proxy in distros based on yum or dnf

OGtrilliams changed the title ~~Trilliams~~ Format update May 24, 2018

jodh-intel reviewed May 25, 2018

View reviewed changes

klynnrif suggested changes May 25, 2018

View reviewed changes

OGtrilliams force-pushed the trilliams branch from aca0a8b to c22d45e Compare June 12, 2018 21:06

OGtrilliams force-pushed the trilliams branch from c22d45e to 9dcdb63 Compare June 12, 2018 22:09

OGtrilliams force-pushed the trilliams branch from 9dcdb63 to 5700df5 Compare June 13, 2018 14:39

OGtrilliams force-pushed the trilliams branch 2 times, most recently from 9188d02 to f208a33 Compare June 21, 2018 17:42

intelkevinputnam reviewed Jul 23, 2018

View reviewed changes

OGtrilliams force-pushed the trilliams branch 2 times, most recently from 6486053 to 85964d3 Compare July 26, 2018 21:49

OGtrilliams force-pushed the trilliams branch 2 times, most recently from 8c20dc7 to 6008c72 Compare July 30, 2018 16:24

OGtrilliams force-pushed the trilliams branch from 6008c72 to 7a99b70 Compare July 30, 2018 16:37

grahamwhaley reviewed Aug 6, 2018

View reviewed changes

jodh-intel added the do-not-merge label Sep 24, 2018

jodh-intel mentioned this pull request Sep 24, 2018

docs: Clean up k8s with cri-containerd howto #251

Merged

grahamwhaley closed this Oct 4, 2018

devimc pushed a commit to devimc/kata-documentation that referenced this pull request Sep 2, 2019

Merge pull request kata-containers#124 from devimc/dockerfile/support…

ac0c290

…Proxy builder: support proxy in distros based on yum or dnf

		@@ -46,20 +46,24 @@ $ command -v kubeadm

		### Configure containerd to use Kata Containers

Format update #124

Format update #124

Conversation

OGtrilliams commented May 24, 2018

jodh-intel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jodh-intel commented May 25, 2018

klynnrif left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OGtrilliams commented May 30, 2018

jodh-intel commented May 30, 2018

grahamwhaley commented Jun 11, 2018

OGtrilliams commented Jun 12, 2018

jodh-intel commented Jun 13, 2018

jodh-intel commented Jun 21, 2018

OGtrilliams commented Jun 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

egernst commented Jul 23, 2018

jodh-intel commented Jul 30, 2018

grahamwhaley commented Aug 6, 2018

OGtrilliams commented Aug 6, 2018

grahamwhaley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

intelkevinputnam commented Aug 6, 2018

bergwolf commented Aug 14, 2018

caoruidong commented Sep 3, 2018

grahamwhaley commented Sep 18, 2018

jodh-intel commented Sep 24, 2018

jodh-intel commented Sep 24, 2018

raravena80 commented Sep 28, 2018

jodh-intel commented Sep 28, 2018

grahamwhaley commented Oct 4, 2018