-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Enhance troubleshooting for SSH key injection #795
Conversation
Hi @ldomb. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ldomb If they are not already assigned, you can assign the PR to them by writing 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 |
/assign @rajatchopra |
doc needs to be adjusted |
docs/user/troubleshooting.md
Outdated
@@ -52,7 +52,9 @@ If no pods are shown, etcd will need to be [investigated](#etcd-is-not-running). | |||
|
|||
### Unable to SSH into Master Nodes | |||
|
|||
In order to SSH into the master nodes, it is necessary to include an administrator's SSH key during the installation. If SSH authentication is failing, ensure that the proper SSH key is being used. | |||
In order to SSH into the master nodes as user `core`, it is necessary to include an administrator's SSH key during the installation. When asked by the installation wizzard for the ssh-key, make sure you define the absolute path to your key. This key will be added to the core user's authorized_keys file. If installed in aws you will not see this key attached to your instance as a keypair in the aws console. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nits: "ssh" -> "SSH". "SSH" is the protocol; ssh
is just the executable. And "core" -> "core
" and "authorized_keys" -> "authorized_keys
". And "aws" -> "AWS".
Re: "absolute path", the wizard prompt is a Select
of your ~/.ssh/*.pub
. I don't think it allows free-form input (or at least, it didn't allow it in my tests).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: "wizzard" -> "wizard"
docs/user/troubleshooting.md
Outdated
In order to SSH into the master nodes, it is necessary to include an administrator's SSH key during the installation. If SSH authentication is failing, ensure that the proper SSH key is being used. | ||
In order to SSH into the master nodes as user `core`, it is necessary to include an administrator's SSH key during the installation. When asked by the installation wizzard for the ssh-key, make sure you define the absolute path to your key. This key will be added to the core user's authorized_keys file. If installed in aws you will not see this key attached to your instance as a keypair in the aws console. | ||
|
||
If SSH authentication is failing, ensure that the proper SSH key is being used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add something like: "Verify your SSH Key by using oc get configmap -n kube-system cluster-config-v1 -o yaml
etc... after "ensure that the proper SSH key is being used."? Because I don't think it's clear for people how to verify the key without restarting the install.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think we should add something like this to the documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the oc get configmap ...
and associated lead-in above, I think we can drop this line.
Done, let me know if that works for you
…On Wed, Jan 2, 2019 at 12:50 PM W. Trevor King ***@***.***> wrote:
@ldomb <https://github.com/ldomb>, do you have time to come back to this
and address the remaining feedback? Just had another report here
<https://groups.google.com/d/msg/openshift-4-dev-preview/5wgpUsoxzDw/2PR5hBYiCAAJ>
from someone who could have used the "user is core" documentation. If you
don't have time, do you mind if I carry your commit into a new PR that
addresses the feedback?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#795 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABarZuDSBqAfaAG1ziL1SL5C4-lsqKvkks5u_PFdgaJpZM4ZFJDW>
.
|
docs/user/troubleshooting.md
Outdated
@@ -50,9 +50,9 @@ sudo crictl pods --name=etcd-member | |||
|
|||
If no pods are shown, etcd will need to be [investigated](#etcd-is-not-running). | |||
|
|||
### Unable to SSH into Master Nodes | |||
### Unable to access the Master Nodes via SSH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: trailing whitespace (and also in the next paragraph).
I'd rather leave this header alone (since I'm fine with "SSH" as a verb). But if you want to update it, you'll also need to update references to it:
$ git grep unable-to origin/pr/795
origin/pr/795:docs/user/troubleshooting.md:If SSH access to the master nodes isn't available, that will need to be [investigated next](#unable-to-ssh-into-master-node).
And it looks like that link is already broken in master because it uses -node
instead of -nodes
. I've filed #980 to fix that, so you don't have to worry about it if you leave the header alone.
docs/user/troubleshooting.md
Outdated
|
||
In order to SSH into the master nodes as user `core`, it is necessary to include an administrator's SSH key during the installation. When asked by the installation wizzard for the ssh-key, make sure you define the absolute path to your key. This key will be added to the core user's authorized_keys file. If installed in aws you will not see this key attached to your instance as a keypair in the aws console. | ||
In order to ssh into the master nodes as user **core**, it is necessary to include an administrator's ssh-key during the installation. When asked by the installation wizard for the ssh-key, make sure you select your ssh-key from the wizard prompt **~/.ssh/*.pub**. This key will be added to the **core** user's **authorized_keys** file. If installed in AWS you will **not** see this key attached to your instance as a keypair in the AWS console. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is moving in the wrong direction with SSH
-> ssh
. Per this comment, you should be using "SSH" (for the protocol) everywhere unless you are writing ssh ...
for a shell command.
Also, core
is more appropriate than core, because the username is literal code you could paste into shell scripts. Similarly for your paths (~/.ssh/*.pub
and authorized_keys
).
not would be more appropriate than not, because this is for local emphasis. For example, see "However, it should not be used..." and "This is not a drill". And the AWS docs use "key pair", not "keypair". So I'd rather see something like:
The public key is placed in
authorized_keys
by Ignition; it is not configured via platform-specific approaches like AWS key pairs.
instead of your last two sentences.
Is there a reason why you didn't add my suggestion here?: #795 (comment) |
Added section for comment 795 |
No no reason, added it to the doc
…On Wed, Jan 2, 2019 at 3:07 PM Kirsten ***@***.***> wrote:
Is there a reason why you didn't add my suggestion here?: #795 (comment)
<#795 (comment)>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#795 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABarZiIW3W8E3mXVWZtTGBdrc4kkhGTDks5u_RGcgaJpZM4ZFJDW>
.
|
docs/user/troubleshooting.md
Outdated
|
||
In order to SSH into the master nodes, it is necessary to include an administrator's SSH key during the installation. If SSH authentication is failing, ensure that the proper SSH key is being used. | ||
In order to SSH into the master nodes as user `core`, it is necessary to include an administrator's ssh-key during the installation. When asked by the installation wizard for the ssh-key, make sure you select your ssh-key from the wizard prompt `~/.ssh/*.pub`. This key will be added to the `core` user's `authorized_keys` file. The public key is placed in authorized_keys by Ignition and is not configured via platform-specific approaches like [AWS key pairs](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-key-pairs.html). To verify if you added the correct ssh-key during installation you can use thefollowing command: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"thefollowing" -> "the following". Or just end with:
You can verify the configured SSH key with:
oc get configmap ...
There's also an authorized_keys
that still needs backticks.
I'd prefer to use reference-style links for external links (like we do now for kubernetes-debug
), but if you're tired of tweaking, I'm ok with the inline link.
docs/user/troubleshooting.md
Outdated
In order to SSH into the master nodes, it is necessary to include an administrator's SSH key during the installation. If SSH authentication is failing, ensure that the proper SSH key is being used. | ||
In order to SSH into the master nodes as user `core`, it is necessary to include an administrator's SSH key during the installation. When asked by the installation wizzard for the ssh-key, make sure you define the absolute path to your key. This key will be added to the core user's authorized_keys file. If installed in aws you will not see this key attached to your instance as a keypair in the aws console. | ||
|
||
If SSH authentication is failing, ensure that the proper SSH key is being used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the oc get configmap ...
and associated lead-in above, I think we can drop this line.
…ion is failing ...
I've pushed 73aa0e4 with the remaining changes I'd like to see here, in case squashing that in is easier than working through GitHub review comments. You should be able to add that commit to your branch with: $ git pull https://github.com/wking/openshift-installer.git ssh-detail-fixups if it looks good to you. Then squash this branch down to a single commit, and we should be good to go :). |
Squashed commits and created a new PR
#986
Also added your changes. But had to modify the links as you have two [][]
instead of []()
…On Thu, Jan 3, 2019 at 3:01 AM W. Trevor King ***@***.***> wrote:
I've pushed 73aa0e4
<73aa0e4>
with the remaining changes I'd like to see here, in case squashing that in
is easier than working through GitHub review comments. You should be able
to add that commit to your branch with:
$ git pull https://github.com/wking/openshift-installer.git ssh-detail-fixups
if it looks good to you. Then squash this branch down to a single commit,
and we should be good to go :).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#795 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABarZvElQ2OaqWPyazlDF79WeDeKnesZks5u_bjtgaJpZM4ZFJDW>
.
|
In the future, it's better to force-push to your existing branch instead of creating a new pull request. That way it's easier for folks starting from a landed commit to find the PR discussion.
Those were intentional. See the docs for reference-style links and the implicit link name shortcut. But whatever, inline links work too ;). |
Added some more documentation around ssh. I don't think its clear to everyone that "core" is the user needed for ssh nor howto generate the config for the sshKey used in the installation.