Skip to content

doc: revamp kata containers getting started guide#12144

Merged
borkmann merged 1 commit intomasterfrom
pr/rolinh/doc-kata-revamp
Jun 19, 2020
Merged

doc: revamp kata containers getting started guide#12144
borkmann merged 1 commit intomasterfrom
pr/rolinh/doc-kata-revamp

Conversation

@rolinh
Copy link
Copy Markdown
Member

@rolinh rolinh commented Jun 17, 2020

This commit updates the getting started guide for kata containers in the
following ways:

  • Remove all custom instructions that were likely copied over from
    external sources, namely the official Kata Containers, CRI-O and
    containerd guides. These turned out to be outdated for the most part.
    Instead, this guide now points the reader to the official guides from
    the Kata Containers documentation to setup Kata Containers and a
    Kubernetes cluster.
  • By removing custom instructions and linking to the official Kata
    Containers documentation, this guide is now also more generic in that
    it should work for any platform that supports the Kata Containers
    runtime instead of being specific to Google Compute Engine (GCE).
  • This guide now being generic, rename it, including the file name, to
    just kata instead of kata-gce.
  • Include k8s-install-download-release.rst instead of duplicating the
    instructions.
  • Add a note that this guide has only been validated using instructions
    for GCE.

@rolinh rolinh added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact. needs-backport/1.8 labels Jun 17, 2020
@b3a-dev b3a-dev mentioned this pull request Jun 17, 2020
20 tasks
Comment thread Documentation/gettingstarted/kata.rst Outdated
Comment thread Documentation/gettingstarted/kata.rst Outdated
Comment thread Documentation/gettingstarted/kata.rst Outdated
Comment thread Documentation/gettingstarted/kata.rst Outdated

helm install cilium |CHART_RELEASE| \\
--namespace kube-system \\
--set global.containerRuntime.integration=containerd
Copy link
Copy Markdown
Contributor

@errordeveloper errordeveloper Jun 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if global.containerRuntime.integration is really needed. I used Cilium with containerd and containerd+kata without setting the parameter, and didn't see any issues.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure either but it was part of the documentation and the distinction for CRI-O and containerd was clearly emphasized.

Copy link
Copy Markdown
Contributor

@errordeveloper errordeveloper Jun 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, so actually crio is the only special value. In practice, I believe containerd basically doesn't have special needs.

$ git grep .Values.global.containerRuntime.integration  install/kubernetes 
install/kubernetes/cilium/charts/agent/templates/daemonset.yaml:{{- if not (eq .Values.global.containerRuntime.integration "crio") }}
install/kubernetes/cilium/charts/agent/templates/daemonset.yaml:{{- if not (eq .Values.global.containerRuntime.integration "crio") }}
install/kubernetes/cilium/charts/agent/templates/daemonset.yaml:{{- if not (eq .Values.global.containerRuntime.integration "crio") }}
$

Comment thread Documentation/gettingstarted/kata.rst Outdated
@errordeveloper
Copy link
Copy Markdown
Contributor

Thanks @rolinh, I think this is a very nice a concise guide now, I added a few comments/suggestions... let me know how else I can help!

This commit updates the getting started guide for kata containers in the
following ways:

- Remove all custom instructions that were likely copied over from
  external sources, namely the official Kata Containers, CRI-O and
  containerd guides. These turned out to be outdated for the most part.
  Instead, this guide now points the reader to the official guides from
  the Kata Containers documentation to setup Kata Containers and a
  Kubernetes cluster.
- By removing custom instructions and linking to the official Kata
  Containers documentation, this guide is now also more generic in that
  it should work for any platform that supports the Kata Containers
  runtime instead of being specific to Google Compute Engine (GCE).
- This guide now being generic, rename it, including the file name, to
  just kata instead of kata-gce.
- Include `k8s-install-download-release.rst` instead of duplicating the
  instructions.
- Add a note that this guide has only been validated using instructions
  for GCE.

Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>
@rolinh rolinh force-pushed the pr/rolinh/doc-kata-revamp branch from 9b767cb to 70f2eae Compare June 18, 2020 15:56
@rolinh rolinh marked this pull request as ready for review June 18, 2020 15:56
@rolinh rolinh requested a review from a team as a code owner June 18, 2020 15:56
@rolinh
Copy link
Copy Markdown
Member Author

rolinh commented Jun 18, 2020

Thanks for your feedback @errordeveloper, I included your suggested changes. I'm not sure about removing global.containerRuntime.integration though as I'm not sure of the impact and I haven't tested it with CRI-O.

@errordeveloper errordeveloper added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 18, 2020
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.006%) to 37.1% when pulling 70f2eae on pr/rolinh/doc-kata-revamp into e601e8f on master.

@borkmann borkmann merged commit 0e535cb into master Jun 19, 2020
@borkmann borkmann deleted the pr/rolinh/doc-kata-revamp branch June 19, 2020 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants