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

Private registries #2194

Merged
merged 3 commits into from
Jan 21, 2020
Merged

Conversation

galal-hussein
Copy link
Contributor

No description provided.

`key_file` | The client key path that will be used to authenticate with the registry
`ca_file` | Defines the CA certificate path to be used to verify the registry's server cert file

The credentials consist of either username/password or authentication token:
Copy link
Contributor

Choose a reason for hiding this comment

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

I removed cred in my copy edit because I didn't see it in the code examples. Is that OK?

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be fine.


Note that server nodes are schedulable by default. If you have not tainted the server nodes and will be running workloads on them, please ensure you also create the registires.yaml file on each server as well.

Configuration in Containerd can be used to connect to a private registry with a TLS connection and with registries that enable authentication as well. The following section will explain the `registries.yaml` file and give different examples of using private registry configuration in K3s.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: contianerd should not be capitalized if you go off how they use it such as here https://github.com/containerd/containerd/blob/master/README.md
If we want to leave it capitalized I won't care too much but they don't do it.
If we decide to make it lower case other changes are needed in the document, not just here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Changed Containerd to containerd throughout, except when it appears at the beginning of a sentence.

`key_file` | The client key path that will be used to authenticate with the registry
`ca_file` | Defines the CA certificate path to be used to verify the registry's server cert file

The credentials consist of either username/password or authentication token:
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be fine.

{{% tabs %}}
{{% tab "With Authentication" %}}
With Auth Content here:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like the "With Auth Content here:" why not completely remove these lines -- just show pure YAML for each of the tabs. (so also need to remove the other lines like this below)

Copy link
Contributor

Choose a reason for hiding this comment

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

Removed "with auth content" from the tabs


> In case of no TLS communication, you need to specify `http://` for the endpoints, otherwise it will default to https.

In order for Containerd to take an effect, you need to restart K3s on each node in order to leverage the private registry.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick:

Re word the last line to something like:

In order for the registry changes to take effect, you need to restart K3s on each node.

Copy link
Contributor

Choose a reason for hiding this comment

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

Updated

Copy link
Contributor

@davidnuzik davidnuzik left a comment

Choose a reason for hiding this comment

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

LGTM
Thank you both!!

@catherineluse catherineluse merged commit 4dfa1da into rancher:master Jan 21, 2020
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

Successfully merging this pull request may close these issues.

3 participants