install: Refactor installation instructions #264
Conversation
|
I still need to figure out where to upload the signing public key. This can be added in the install instruction code block, and |
There was a problem hiding this comment.
Hi @marcov - I like what you're doing here :)! A few comments...
install/README.md
Outdated
|
|
||
| The table below provides a list of the currently supported distributions. | ||
|
|
||
| - If your distribution is supported, you can easily install Kata packages [using kata-manager](...). |
There was a problem hiding this comment.
-
Can you make "If your distrubution is supported" a link to the "Distributions" section in
README.md. That will make it clearer we mean "supported by Kata" rather than "supported by thekata-managerscript I think. -
Broken link - this should be https://github.com/kata-containers/tests/tree/master/cmd/kata-manager (or maybe the README in that directory)
I would be tempted to turn these three bullets into sub-sections (and add those to the table of contents of course).
I like this refactor but I do think it might be slightly confusing to users to know which option is most appropriate for them to select still.
Once we have subsections for the different options, you could make it slightly clearer what type of user might want to select each option:
- Automatic install using
kata-manager(Recommended option for the impatient / quickest option to get a working system) - Manual installation (if you want to run each installation step by following written instructions).
- Scripted installation (if you want to be able to install Kata automatically, but also want to review the process first).
The wording above might need a bit of rework and these options could maybe be put into a table themselves but it should be very clear to users which option is most appropriate for them I think.
It's probably also worth stating that whichever option they select, the resulting system will be the same - it's just the method itself which differs.
There was a problem hiding this comment.
Thanks for the review, I agree about this being hard to read. But I pushed it out early to gather feedbacks like yours :)
My first attempt was to have the different options available in a single document. Also, I felt it was worth documenting how to use kata-manager and not just mention that it can be used as an option.
Let's attempt to structure it with subsections.
There was a problem hiding this comment.
Hi @marcov - sure and raising PRs like this is much appreciated! Please don't misunderstand - I think this is moving in the right direction but delicate doc rework like this can take a few iterations. It's also just as difficult as code refactoring imho (but arguably more important! ;)
I think it makes a lot of sense having all the options in the top-level README - we just need to make some tweaks to make it clear to both "power users" and "newbies" which option is most appropriate for them :)
It would be good in fact to get more input on this from the rest of the team and @klynnrif and @intelkevinputnam.
There was a problem hiding this comment.
hehe @jodh-intel please don't misunderstand me too 😆 I agree 100% with what you said.
What I meant is that I started with what looked like obvious to change and then this got out of hand.
install/README.md is the front page of Kata for the new users and it is important to get it right.
Maybe we can open the discussion to the ML or the next architecture call?
There was a problem hiding this comment.
@marcov - I think we're actually pretty close to having this done, but if we can't resolve by Monday, sure - feel free to add it to the agenda :)
/cc @grahamwhaley, @egernst for input.
There was a problem hiding this comment.
@jodh-intel what do you think about this structure?
I am not sure if it's better to name the section with the distributions table Supported Distribution or Manual Installation
install/README.md
Outdated
| * [OpenSuse](opensuse-installation-guide.md) | ||
| * [Ubuntu](ubuntu-installation-guide.md) | ||
| * [SLES](sles-installation-guide.md) | ||
| |Distro | Versions | Notes | |
There was a problem hiding this comment.
- Could you change the name of the "Distro" column to "Distro specific installation instructions" as it is not immediately obvious what this table is showing the user.
- The "Notes" column isn't being used so I think you could remove it?
There was a problem hiding this comment.
- The "Notes" column isn't being used so I think you could remove it?
gonna use vim CTRL-V for this :D
There was a problem hiding this comment.
<tongue-in-cheek>
Or for those readers using an editor requiring many, many fingers (and a reinforced tab key), you could use something like org-table-delete-column
</tongue-in-cheek>
install/README.md
Outdated
| |[Fedora](fedora-installation-guide.md) | 27, 28 | | | ||
| |[openSUSE](opensuse-installation-guide.md) | Leap (42.3) | | | ||
| |[Red Hat Enterprise Linux (RHEL)](rhel-installation-guide.md) | 7 | | | ||
| |[SUSE Linux Enterprise Server (SLES)](sles-installation-guide.md) | SLE 12 SP3 | | |
There was a problem hiding this comment.
"SLE" or "SLES" in column 2?
|
|
||
| ## Install the Kata packages | ||
| ``` | ||
| bash -c "$(curl -fsSL \ |
There was a problem hiding this comment.
All code entries should have a $ prefix - see https://github.com/kata-containers/documentation/blob/master/Documentation-Requirements.md#code-blocks.
| install-packages" | ||
| ``` | ||
|
|
||
| ## Install Docker |
There was a problem hiding this comment.
Might be worth mentioning that kubernetes support is not available with this tool. Also, you could mention that the user can run kata-manager.sh -h for full details.
| ``` | ||
| bash -c "$(curl -fsSL \ | ||
| https://raw.githubusercontent.com/kata-containers/tests/master/cmd/kata-manager/kata-manager.sh) \ | ||
| install-docker-system" |
There was a problem hiding this comment.
Note that install-docker-system implies install-packages. To get a fully working system, you only need to run install-docker-system so you could drop the install-packages section, or move it after the "Install Docker" section to say that if your chosen container manager is not Docker, you can install the Kata packages but need to also install k8s, etc.
marcov
force-pushed
the
refactor-install
branch
5 times, most recently
from
4607ebf to
60f6f99
Compare
|
Ok, I pushed a new version based on @jodh-intel suggestions. I can see some improvements but feel free to give some more feedbacks :) |
install/README.md
Outdated
| Installing Kata Containers on your system is a three step procedure: | ||
| 1. Install the Kata Container packages. | ||
| 2. Install a supported container runtime manager. | ||
| 3. Configure the container manager to use `kata-runtime` as OCI runtime. |
There was a problem hiding this comment.
correct me if I am wrong but we should update to use https? And probably add a comment that suggest add/verify gpg key with a secure channel?
curl -sL http://download.opensuse.org/repositories/home:/katacontainers:/releases:/${ARCH}:/master/xUbuntu_$(lsb_release -rs)/Release.key | sudo apt-key add -
$ sudo -E apt-get update
There was a problem hiding this comment.
My idea was to include the key import procedure in the installation instruction for each distribution, and to do this in a separate PR.
HTTPS will be used when importing the public key as secure channel, but it's not needed when downloading packages from OBS.
I am not sure if it's better to download the key from OBS, or have it uploaded on GitHub somewhere. Do you think it makes a difference for the tests infrastructure?
There was a problem hiding this comment.
having the key import done in the instructions means that this will also be run with tests,
and we can drop the --no-gpg-checks flags used now.
There was a problem hiding this comment.
Nice. It's one extra step, but having the key import step clearly stated will make it clearer to users what is happening from a security perspective.
install/README.md
Outdated
| > **Notes on package source verification**: | ||
| > - The Kata packages hosted on the download server are signed with GPG to ensure integrity and authenticity. | ||
| > | ||
| > - [This is the public key](....) used for signing the packages and [this is its fingerprint](...). |
There was a problem hiding this comment.
We need to fill in these blanks 😄
marcov
force-pushed
the
refactor-install
branch
5 times, most recently
from
972d7dd to
b778a82
Compare
|
I hope this is getting closer to a final version. I thought that the reference packages signing key can be uploaded at this location, given that that it will be used to automate packages installation when running CI tests: https://github.com/kata-containers/tests/data/rpm-signkey.pub |
|
lgtm ping @kata-containers/documentation.
|
install/README.md
Outdated
| Kata Containers requires nested virtualization or bare metal. | ||
| See the | ||
| [hardware requirements](https://github.com/kata-containers/runtime/blob/master/README.md#hardware-requirements) | ||
| to see if your system is capable of running Kata Containers. | ||
|
|
||
| ## Installing Kata Containers | ||
| ## Installing on a Linux System | ||
| Here's an overview of the different installation methods available. All of these will equally result in a system |
There was a problem hiding this comment.
Suggested rewrite: The following is an overview of the different installation methods available. All of these methods equally result in a system
install/README.md
Outdated
| |[Build from sources](#../Developer-Guide.md#initial-setup) |Developers and hackers |any distro | | ||
|
|
||
| ### Automatic Installation | ||
| You can easily install Kata packages [using kata-manager](installing-with-kata-manager.md). |
There was a problem hiding this comment.
Suggested rewrite: Use kata-manager to automatically install Kata packages.
install/README.md
Outdated
| You can easily install Kata packages [using kata-manager](installing-with-kata-manager.md). | ||
|
|
||
| ### Scripted Installation | ||
| You can generate installation scripts [using kata-doc-to-script](installing-with-kata-doc-to-script.md). |
There was a problem hiding this comment.
Suggested rewrite: Use kata-doc-to-script to generate installation scripts.
install/README.md
Outdated
| > | ||
| > - The public key used to sign packages is available [at this link](https://github.com/kata-containers/tests/data/rpm-signkey.pub); the fingerprint is `9FDC0CB6 3708CF80 3696E2DC D0B37B82 6063F3ED`. | ||
| > | ||
| > - Only trust these signing key and fingerprint and don't disable GPG checks, otherwise packages |
There was a problem hiding this comment.
Lines 62-63 suggested rewrite: Only trust the signing key and fingerprint listed in the previous bullet point. Do not disable GPG checks, otherwise package source and authenticity is not guaranteed.
| ${ID}-install.sh" | ||
| ``` | ||
|
|
||
| If for example your distribution is CentOS, this will generate a runnable shell script called `centos-install.sh`. |
There was a problem hiding this comment.
Suggested rewrite: For example, if your distribution is CentOS, the previous example will generate a runnable shell script called centos-install.sh.
| ${ID}-docker-install.sh" | ||
| ``` | ||
|
|
||
| If for example your distribution is CentOS, this will generate a runnable shell script called `centos-docker-install.sh`. |
There was a problem hiding this comment.
Suggested rewrite: For example, if your distribution is CentOS, the previous example will generate a runnable shell script called centos-docker-install.sh.
| > can still use `kata-manager` to [install Kata package](#install-kata-packages-only), and then setup your container manager manually. | ||
|
|
||
| ## Full Installation | ||
| This command will: |
There was a problem hiding this comment.
Lines 16-19 suggested rewrite:
This command does the following:
- Installs Kata Containers packages
- Installs Docker
- Configures Docker to use the kata OCI runtime by default
| ``` | ||
|
|
||
| ## Install the Kata packages only | ||
| This command will only install Kata Containers packages. |
There was a problem hiding this comment.
Suggested rewrite: Use the following command to only install Kata Containers packages:
| ``` | ||
|
|
||
| ## Further Information | ||
| `kata-manager` can do much more; refer to [`kata-manager` page](https://github.com/kata-containers/tests/blob/master/cmd/kata-manager) for more information. |
There was a problem hiding this comment.
Suggested rewrite: For more information on what kata-manager can do, refer to the kata-manager page.
Refactor installation instruction to minimize duplicate content, to document package source verification process, and to remove some of the typos. Fixes: kata-containers#263 Signed-off-by: Marco Vedovati <mvedovati@suse.com>
marcov
force-pushed
the
refactor-install
branch
from
b778a82 to
cc04649
Compare
|
Thanks @klynnrif, I integrated all your suggestions. |
marcov
changed the title
|
Removed the WIP tag, and waiting for an extra review :) |
|
Re-ping @kata-containers/documentation - it would be great to get this landed today... 😄 |
There was a problem hiding this comment.
Nice re-org, de-dup and cleanup :-)
lgtm
One small note - does not have to be a blocker - your call @marcov if you want to add ClearLinux to the distro list right now?
| |[openSUSE](opensuse-installation-guide.md) | Leap (42.3) | | ||
| |[Red Hat Enterprise Linux (RHEL)](rhel-installation-guide.md) | 7 | | ||
| |[SUSE Linux Enterprise Server (SLES)](sles-installation-guide.md) | SLES 12 SP3 | | ||
| |[Ubuntu](ubuntu-installation-guide.md) | 16.04, 18.04 | |
There was a problem hiding this comment.
We might want to add a Clear Linux entry here as well - we don't package for it on OBS, but Clear Linux packages Kata directly.
There was a problem hiding this comment.
@grahamwhaley I could add ClearLinux, but that table was made with the assumption that kata-manager or kata-doc-to-script can use the linked installation instructions.
So there are 2 possibilities:
- Add ClearLinux, and a note column, saying that installation cannot be scripted / automated for it
- Have a new PR with a dedicated installation instructions pages, as done for other distros. However this would duplicate the information, something that @jodh-intel does not like :)
There was a problem hiding this comment.
Although I think @grahamwhaley has a good point about mentioning Clear Linux here, since that distro isn't actually mentioned at all, I think it's just about out of scope of this PR.
Let's land this and update for CLR on #270.
|
lgtm
|
image-builder: re-implement image builder script
Refactor installation instruction to minimize duplicate content,
to document package source verification process, and to remove
some of the typos.
Fixes: #263
Signed-off-by: Marco Vedovati mvedovati@suse.com