Skip to content

Support ssh-key-dir on OKD/FCOS#2688

Merged
openshift-merge-robot merged 2 commits intoopenshift:masterfrom
LorbusChris:ssh-key-dir
Nov 22, 2021
Merged

Support ssh-key-dir on OKD/FCOS#2688
openshift-merge-robot merged 2 commits intoopenshift:masterfrom
LorbusChris:ssh-key-dir

Conversation

@LorbusChris
Copy link
Copy Markdown
Contributor

@LorbusChris LorbusChris commented Jul 22, 2021

- What I did
In older versions of OKD, the SSH keys were written to
/home/core/.ssh/authorized_keys. Newer versions of OKD will
expect the keys at /home/core/.ssh/authorized_keys.d/ignition.
In the case of a newer version of OKD, the keys file is removed
from the legacy path in the updateSSHKeys function.
It will then be recreated at the new fragment path by the
atomicallyWriteSSHKey function that is called right after.

- How to verify it
CI

- Description for the changelog
Support ssh-key-dir on OKD/FCOS

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 22, 2021
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jul 22, 2021

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@LorbusChris
Copy link
Copy Markdown
Contributor Author

/test e2e-aws

@LorbusChris
Copy link
Copy Markdown
Contributor Author

/test e2e-aws

@LorbusChris
Copy link
Copy Markdown
Contributor Author

/test e2e-aws

@kikisdeliveryservice kikisdeliveryservice added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 26, 2021
@fortinj66
Copy link
Copy Markdown
Contributor

@LorbusChris, #2087 will need to be reverted for this to work correct?

@LorbusChris
Copy link
Copy Markdown
Contributor Author

@fortinj66 yes. The plan is to manage the ignition key fragment in the future.

@LorbusChris LorbusChris force-pushed the ssh-key-dir branch 2 times, most recently from 1bdb52a to 2f91cc6 Compare September 28, 2021 23:37
@LorbusChris LorbusChris marked this pull request as ready for review September 28, 2021 23:43
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 28, 2021
@LorbusChris
Copy link
Copy Markdown
Contributor Author

/cc @bgilbert
/cc @travier

@openshift-ci openshift-ci Bot requested review from bgilbert and travier September 28, 2021 23:44
@LorbusChris
Copy link
Copy Markdown
Contributor Author

/retest-required

@LorbusChris
Copy link
Copy Markdown
Contributor Author

/test e2e-aws

@travier
Copy link
Copy Markdown
Member

travier commented Oct 1, 2021

The order in which we will merge things here will matter unfortunately as this involves Ignition package changes too. One option we have is to make sure we coordinate this to happen on a specific day.

Copy link
Copy Markdown
Member

@travier travier left a comment

Choose a reason for hiding this comment

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

Code LGTM but I'm not really familiar with the MCO code base.

@LorbusChris
Copy link
Copy Markdown
Contributor Author

/test e2e-agnostic-upgrade

@LorbusChris
Copy link
Copy Markdown
Contributor Author

/test e2e-aws
/test e2e-gcp-op

@LorbusChris
Copy link
Copy Markdown
Contributor Author

/test e2e-aws
/test e2e-gcp-op
/test e2e-agnostic-upgrade

@LorbusChris LorbusChris force-pushed the ssh-key-dir branch 3 times, most recently from 875ded7 to 40394f8 Compare October 27, 2021 19:22
@LorbusChris
Copy link
Copy Markdown
Contributor Author

I've removed the RHCOS9 parts from this PR

@LorbusChris LorbusChris changed the title Support ssh-key-dir on FCOS and future RHCOS9 Support ssh-key-dir on OKD/FCOS Nov 16, 2021
Comment thread pkg/daemon/update.go Outdated
Comment thread pkg/daemon/update.go Outdated
Comment thread pkg/daemon/update.go Outdated
In older versions of OKD, the SSH keys were written to
`/home/core/.ssh/authorized_keys`. Newer versions of OKD will
expect the keys at `/home/core/.ssh/authorized_keys.d/ignition`.
In the case of a newer version of OKD, the keys file is removed
from the legacy path in the updateSSHKeys function.
It will then be recreated at the new fragment path by the
atomicallyWriteSSHKey function that is called right after.
Usage of ssh-key-dir is now supported on FCOS (OKD)
Copy link
Copy Markdown
Contributor

@sinnykumari sinnykumari left a comment

Choose a reason for hiding this comment

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

Thanks Christain for making the changes.
/lgtm

@sinnykumari
Copy link
Copy Markdown
Contributor

Just noticed #2688 (comment) , @LorbusChris do we need any ignition update here? Putting hold until this question is addressed.
/hold

@LorbusChris
Copy link
Copy Markdown
Contributor Author

Just noticed #2688 (comment) , @LorbusChris do we need any ignition update here? Putting hold until this question is addressed. /hold

No, this will only be needed for RHCOS9

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Nov 22, 2021
@LorbusChris
Copy link
Copy Markdown
Contributor Author

/hold cancel

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 22, 2021
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Nov 22, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LorbusChris, sinnykumari, travier

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

The pull request process is described here

Details 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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 22, 2021
@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Nov 22, 2021

@LorbusChris: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-techpreview-featuregate 2f91cc6d819677c1b0c3a3428901de580b9c6f42 link false /test e2e-aws-techpreview-featuregate
ci/prow/e2e-vsphere-upgrade 05142f3 link false /test e2e-vsphere-upgrade
ci/prow/e2e-ovn-step-registry 05142f3 link false /test e2e-ovn-step-registry
ci/prow/okd-e2e-aws 05142f3 link false /test okd-e2e-aws
ci/prow/e2e-aws-workers-rhel7 05142f3 link false /test e2e-aws-workers-rhel7
ci/prow/e2e-aws-disruptive 05142f3 link false /test e2e-aws-disruptive
ci/prow/e2e-metal-ipi 05142f3 link false /test e2e-metal-ipi

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 21fd183 into openshift:master Nov 22, 2021
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. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants