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

(PE-36789) R10k Known hosts upgrade path #382

Merged
merged 6 commits into from Sep 15, 2023
Merged

(PE-36789) R10k Known hosts upgrade path #382

merged 6 commits into from Sep 15, 2023

Conversation

ragingra
Copy link
Contributor

Adding optional parameter for r10k known hosts
Alerting user to set known hosts if they are upgrading to or past 2023.3

Summary

Provide a detailed description of all the changes present in this pull request.

Additional Context

Add any additional context about the problem here.

Related Issues (if any)

Mention any related issues or pull requests.

Checklist

  • 🟢 Spec tests.
  • 🟢 Acceptance tests.

Changes include test coverage?

  • Yes
  • Not needed

Have you updated the documentation?

  • Yes, I've updated the appropriate docs
  • Not needed

plans/upgrade.pp Outdated Show resolved Hide resolved
Adding optional parameter for r10k known hosts
Alerting user to set known hosts if they are upgrading to or past 2023.3
@CoMfUcIoS
Copy link
Contributor

@ragingra should this still be a draft?

@ragingra ragingra marked this pull request as ready for review September 14, 2023 17:07
@ragingra ragingra requested review from a team as code owners September 14, 2023 17:07
Copy link
Contributor

@CoMfUcIoS CoMfUcIoS left a comment

Choose a reason for hiding this comment

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

LGTM. I would like to see some tests around the r10k upgrade!!!

@CoMfUcIoS
Copy link
Contributor

@jpartlow if you are happy with it, please merge

documentation/upgrade.md Outdated Show resolved Hide resolved
Noticed that the get_pe_conf/update_pe_conf functions were expecting a
target but $primary_target is actually an array. So I went ahead added
specs covering the basic cases for upgrade and r10k_known_hosts. They
aren't the best specs due to difficulties testing write_file,
upload_file and out_message, but they at least validate that the plan
completes with r10k_known_hosts set.
@jpartlow
Copy link
Contributor

Added upgrade specs because I found a breaking issue while fiddling with them.

Would probably be good to add a github workflow that tests install/upgrade with r10k settings, validates puppet-code deploy, but that was more than I had time for.

I did verify a manual standard upgrade from 2021.7.4 to 2023.3.0 with code manager setup still had working code deployment with r10k_known_hosts set.

@jpartlow jpartlow merged commit 30f71f8 into main Sep 15, 2023
52 checks passed
@jpartlow jpartlow deleted the PE-36789 branch September 15, 2023 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants