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

Fix CRI-O installation information #23000

Merged
merged 1 commit into from
Aug 17, 2020

Conversation

haircommander
Copy link
Contributor

update Ubuntu, CentOS and Debian installation instructions
add Fedora instructions

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 6, 2020
@k8s-ci-robot
Copy link
Contributor

Welcome @haircommander!

It looks like this is your first PR to kubernetes/website 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/website has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 6, 2020
@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Aug 6, 2020
@haircommander
Copy link
Contributor Author

PTAL @TomSweeneyRedHat

@netlify
Copy link

netlify bot commented Aug 6, 2020

Deploy preview for kubernetes-io-master-staging ready!

Built with commit ed4506c

https://deploy-preview-23000--kubernetes-io-master-staging.netlify.app

<br />
Then, set `$VERSION` to be the cri-o version matching your kubernetes version.
For instance, if you want to install cri-o 1.17, `VERSION=1.17`
We also support pinning to a particular release. To install 1.17.3, `VERSION=1.17:1.17.3`

Choose a reason for hiding this comment

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

is 1.17 the latest on CENTOS? If 1.18 is, I'd bump this example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@TomSweeneyRedHat
Copy link

One question, otherwise LGTM

| Centos 8 | `CentOS_8` |
| Centos 8 Stream | `CentOS_8_Stream` |
| Centos 7 | `CentOS_7` |
<br />
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the <br> required to provide spacing between the table and the following text?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah!

@haircommander
Copy link
Contributor Author

addressed as suggested @kbhawkey PTAL

@inductor
Copy link
Member

@haircommander

Is this design intended? looks kinda weird to me 🤔

image

@haircommander
Copy link
Contributor Author

If I'm being honest, the table format is not being rendered super well. I think it is clear enough to be understood, but I'm happy to adopt a change that would look better.

| Ubuntu 19.10 | `xUbuntu_19.10` |
| Ubuntu 19.04 | `xUbuntu_19.04` |
| Ubuntu 18.04 | `xUbuntu_18.04` |
<br />
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @haircommander .
I suspect the parser is not happy with the <br> elements.
What happens if these are removed? Is there no spacing after the table
and before the next line of text?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah the table comes immediately before. It definitely looks weird. I can push an updated version if you'd like

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lmk what you think, I left it as a separate commit to be easily reverted. I think the oddly spaced out table is better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually, I updated the version, adding a newline between br and the table. that fixed the issue

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @haircommander. The tables look good.

@kbhawkey
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 12, 2020
Copy link
Contributor

@zacharysarah zacharysarah left a comment

Choose a reason for hiding this comment

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

@haircommander 👋 Thanks for the update! Please follow the style guide, specifically:

  • Capitalize consistently
  • Avoid "we"
  • Use in-line code formatting

{{% /tab %}}
{{% tab name="Fedora" %}}

Set `$VERSION` to be the cri-o version matching your kubernetes version.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Set `$VERSION` to be the cri-o version matching your kubernetes version.
Set `$VERSION` to the CRI-O version that matches your Kubernetes version.

{{% tab name="Fedora" %}}

Set `$VERSION` to be the cri-o version matching your kubernetes version.
For instance, if you want to install cri-o 1.18, `VERSION=1.18`
Copy link
Contributor

Choose a reason for hiding this comment

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

Capitalize consistently.

Suggested change
For instance, if you want to install cri-o 1.18, `VERSION=1.18`
For instance, if you want to install CRI-O 1.18, `VERSION=1.18`

```shell
dnf module list cri-o
```
We do not support pinning to specific releases on Fedora.
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid using "we".

Suggested change
We do not support pinning to specific releases on Fedora.
CRI-O does not support pinning to specific releases on Fedora.

echo 'deb http://download.opensuse.org/repositories/devel:/kubic:/libcontainers:/stable/Debian_Unstable/ /' > /etc/apt/sources.list.d/devel:kubic:libcontainers:stable.list
wget -nv https://download.opensuse.org/repositories/devel:kubic:libcontainers:stable/Debian_Unstable/Release.key -O- | sudo apt-key add -
```
To install CRI-O on the following operating systems, set the environment variable $OS as the appropriate field in the following table:
Copy link
Contributor

Choose a reason for hiding this comment

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

Use code format for inline code.

Suggested change
To install CRI-O on the following operating systems, set the environment variable $OS as the appropriate field in the following table:
To install CRI-O on the following operating systems, set the environment variable $OS to the appropriate field in the following table:

wget -nv https://download.opensuse.org/repositories/devel:kubic:libcontainers:stable/Debian_10/Release.key -O- | sudo apt-key add -
```
<br />
Then, set `$VERSION` to be the cri-o version matching your kubernetes version.
Copy link
Contributor

Choose a reason for hiding this comment

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

Capitalize consistently.

Suggested change
Then, set `$VERSION` to be the cri-o version matching your kubernetes version.
Then, set `$VERSION` to be the CRI-O version matching your kubernetes version.

| Ubuntu 18.04 | `xUbuntu_18.04` |

<br />
Then, set `$VERSION` to be the cri-o version matching your kubernetes version.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Then, set `$VERSION` to be the cri-o version matching your kubernetes version.
Then, set `$VERSION` to the CRI-O version that matches your Kubernetes version.


<br />
Then, set `$VERSION` to be the cri-o version matching your kubernetes version.
For instance, if you want to install cri-o 1.18, set `VERSION=1.18`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Capitalize consistently.

Suggested change
For instance, if you want to install cri-o 1.18, set `VERSION=1.18`.
For instance, if you want to install CRI-O 1.18, set `VERSION=1.18`.

curl -L -o /etc/yum.repos.d/devel:kubic:libcontainers:stable.repo https://download.opensuse.org/repositories/devel:kubic:libcontainers:stable/CentOS_7/devel:kubic:libcontainers:stable.repo
curl -L -o /etc/yum.repos.d/devel:kubic:libcontainers:stable:cri-o:{{< skew latestVersion >}}.repo https://download.opensuse.org/repositories/devel:kubic:libcontainers:stable:cri-o:{{< skew latestVersion >}}/CentOS_7/devel:kubic:libcontainers:stable:cri-o:{{< skew latestVersion >}}.repo
```
To install on the following operating systems, set the environment variable $OS as the appropriate field in the following table:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
To install on the following operating systems, set the environment variable $OS as the appropriate field in the following table:
To install CRI- O on the following operating systems, set the environment variable $OS as the appropriate field in the following table:

| Centos 7 | `CentOS_7` |

<br />
Then, set `$VERSION` to be the cri-o version matching your kubernetes version.
Copy link
Contributor

Choose a reason for hiding this comment

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

Capitalize consistently.

Suggested change
Then, set `$VERSION` to be the cri-o version matching your kubernetes version.
Then, set `$VERSION` to the CRI-O version that matches your Kubernetes version.


<br />
Then, set `$VERSION` to be the cri-o version matching your kubernetes version.
For instance, if you want to install cri-o 1.18, set `VERSION=1.18`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For instance, if you want to install cri-o 1.18, set `VERSION=1.18`.
For instance, if you want to install CRI-O 1.18, set `VERSION=1.18`.

update Ubuntu, CentOS and Debian installation instructions
add Fedora instructions

Signed-off-by: Peter Hunt <pehunt@redhat.com>
@haircommander
Copy link
Contributor Author

thank you @zacharysarah, updated as suggested

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 17, 2020
@zacharysarah
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 17, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zacharysarah

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 17, 2020
@k8s-ci-robot k8s-ci-robot merged commit 46ba002 into kubernetes:master Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants