Skip to content

"Redeploying Master Certificates Only" recreates "Service Serving Certificates" CA Certificate #8373

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 16, 2018
Merged

"Redeploying Master Certificates Only" recreates "Service Serving Certificates" CA Certificate #8373

merged 1 commit into from
Apr 16, 2018

Conversation

@mburke5678
Copy link
Contributor

@mburke5678 mburke5678 commented Mar 22, 2018

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 22, 2018
gpei
gpei reviewed Apr 12, 2018
View reviewed changes
install_config/redeploying_certificates.adoc Outdated
Copy link

@gpei gpei Apr 12, 2018

Choose a reason for hiding this comment

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

The playbook name should not be changed. The previous one is correct for ocp-3.9.
What we may need to improve is "redeploy-master-certificates.yml playbook" in line380, for more specifically, we should name it as "master redeploy-certificates.yml playbook".
So is in line 402 and line 421, which should be:
The etcd redeploy-certificates.yml playbook
The node redeploy-certificates.yml playbook

install_config/redeploying_certificates.adoc Outdated
Copy link

@gpei gpei Apr 12, 2018

Choose a reason for hiding this comment

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

This part lgtm 👍

@mburke5678
Copy link
Contributor Author

mburke5678 commented Apr 12, 2018 •
edited
Loading

@gpei I changed the redeploy-certificates.yml to redeploy-master-certificates.yml because it appears to have been changed through a different commit. It looks like it was changed incorrectly?? Previous versions of the docs use redeploy-master-certificates in this section, which matched the introductory text.
The redeploy-master-certificates.yml name was also used in the BZ and customer case.

@gpei
Copy link

gpei commented Apr 13, 2018

@mburke5678 For 3.7 and previous version, the playbook used for redeploying master cert is playbooks/byo/openshift-cluster/redeploy-master-certificates.yml
From 3.9, it changed as
playbooks/openshift-master/redeploy-certificates.yml
So there's no redeploy-master-certificates.yml file in 3.9

@mburke5678 mburke5678 added this to the Next Release milestone Apr 13, 2018
@mburke5678
Copy link
Contributor Author

mburke5678 commented Apr 13, 2018

@gpei Does this look good to merge to 3.9 and 3.10?
I can manually add the note to 3.7 and previous versions back to 3.3. Is this an acceptable plan?

@gpei
Copy link

gpei commented Apr 16, 2018

@mburke5678 sorry, I'm not able to catch any new changes in this PR.

@mburke5678
Copy link
Contributor Author

mburke5678 commented Apr 16, 2018

@openshift/team-documentation PTAL Small change

kalexand-rh
kalexand-rh reviewed Apr 16, 2018
View reviewed changes
install_config/redeploying_certificates.adoc Outdated
Copy link
Contributor

@kalexand-rh kalexand-rh Apr 16, 2018

Choose a reason for hiding this comment

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

executing should be running
certificate/key pairs should be certificates or key pairs

install_config/redeploying_certificates.adoc Outdated
Copy link
Contributor

@kalexand-rh kalexand-rh Apr 16, 2018

Choose a reason for hiding this comment

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

containing should be that contain
Is there another way to say "service serving certificates?"

Copy link
Contributor Author

@mburke5678 mburke5678 Apr 16, 2018

Choose a reason for hiding this comment

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

@kalexand-rh It appears that "service serving certificates" was the term when the feature was added to OpenShift.
https://github.com/openshift/openshift-docs/pull/2324/files

Copy link
Contributor

@kalexand-rh kalexand-rh Apr 16, 2018

Choose a reason for hiding this comment

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

@mburke5678, that's fair. A bit confusing still, but fair. :)

@mburke5678 mburke5678 merged commit 6268284 into openshift:master Apr 16, 2018
@mburke5678
Copy link
Contributor Author

mburke5678 commented Apr 16, 2018

/cherrypick enterprise-3.9

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Apr 16, 2018
Merged
@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Apr 16, 2018

@mburke5678: new pull request created: #8746

In response to this:

/cherrypick enterprise-3.9

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.

@mburke5678
Copy link
Contributor Author

mburke5678 commented Apr 16, 2018

/cherrypick enterprise-3.10

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Apr 16, 2018
Merged
@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Apr 16, 2018

@mburke5678: new pull request created: #8747

In response to this:

/cherrypick enterprise-3.10

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.

This was referenced Apr 16, 2018
Merged
Merged
Merged
Merged
Merged
@mburke5678 mburke5678 deleted the BZ-1550250 branch August 2, 2018 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kalexand-rh kalexand-rh kalexand-rh left review comments

+1 more reviewer

@gpei gpei gpei left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

branch/enterprise-3.9 branch/enterprise-3.10 size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

Next Release

Development

Successfully merging this pull request may close these issues.

5 participants

@mburke5678 @gpei @openshift-cherrypick-robot @kalexand-rh @openshift-ci-robot