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

OCPBUGS-19403: updates etcd procedure with OVN-K i/c #64939

Merged
merged 1 commit into from
Sep 28, 2023

Conversation

JoeAldinger
Copy link
Contributor

@JoeAldinger JoeAldinger commented Sep 19, 2023

@openshift-ci-robot openshift-ci-robot added jira/severity-critical Referenced Jira bug's severity is critical for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. labels Sep 19, 2023
@openshift-ci-robot
Copy link

@JoeAldinger: This pull request references Jira Issue OCPBUGS-19403, which is invalid:

  • expected the bug to target the "4.15.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Version(s):

Issue:

Link to docs preview:

QE review:

  • QE has approved this change.

Additional information:

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.

@openshift-ci-robot openshift-ci-robot added the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Sep 19, 2023
@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 19, 2023
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Sep 19, 2023

🤖 Updated build preview is available at:
https://64939--docspreview.netlify.app

Build log: https://circleci.com/gh/ocpdocs-previewbot/openshift-docs/26118

@palonsoro
Copy link
Contributor

@JoeAldinger please wherever it says $<node> please use <node>. It is me (again) using the bash-like syntax, while the docs better benefit from the <whatever> syntax.

In new step 13.d, I'd warn that it may take long for the pod to be ready (up to several minutes). Users may get nervous otherwise.

For the rest, it looks perfect to me. Many thanks! Also looking forward for the QE testing, just to be sure that this doesn't break under scenarios I might have not considered.

@palonsoro
Copy link
Contributor

It now looks better 🙂

@JoeAldinger JoeAldinger force-pushed the OCPBUGS-19403 branch 2 times, most recently from c1f75a6 to 2871212 Compare September 20, 2023 16:06
@JoeAldinger JoeAldinger added peer-review-needed Signifies that the peer review team needs to review this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Sep 20, 2023
@geliu2016
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 22, 2023
@openshift-ci-robot
Copy link

@JoeAldinger: This pull request references Jira Issue OCPBUGS-19403, which is invalid:

  • expected the bug to target the "4.15.0" version, but no target version was set
  • expected the bug to be in one of the following states: NEW, ASSIGNED, POST, but it is MODIFIED instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Version(s):
4.14+

Issue:
https://issues.redhat.com/browse/OCPBUGS-19403

Link to docs preview:
https://64939--docspreview.netlify.app/openshift-enterprise/latest/backup_and_restore/control_plane_backup_and_restore/disaster_recovery/scenario-2-restoring-cluster-state#:~:text=If%20you%20are%20using%20the%20OVNKubernetes%20network%20plugin%2C%20you%20must%20restart%20ovnkube%2Dcontrolplane%20pods.

QE review:

  • QE has approved this change.

Additional information:
PTAL: @geliu2016

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.

@JoeAldinger JoeAldinger added the peer-review-needed Signifies that the peer review team needs to review this PR label Sep 22, 2023
@bergerhoffer
Copy link
Contributor

/label peer-review-in-progress

@bergerhoffer bergerhoffer added this to the Planned for 4.14 GA milestone Sep 22, 2023
@openshift-ci openshift-ci bot added the peer-review-in-progress Signifies that the peer review team is reviewing this PR label Sep 22, 2023
Copy link
Contributor

@bergerhoffer bergerhoffer left a comment

Choose a reason for hiding this comment

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

A few suggestions!

modules/dr-restoring-cluster-state.adoc Outdated Show resolved Hide resolved
modules/dr-restoring-cluster-state.adoc Outdated Show resolved Hide resolved
modules/dr-restoring-cluster-state.adoc Outdated Show resolved Hide resolved
modules/dr-restoring-cluster-state.adoc Outdated Show resolved Hide resolved
modules/dr-restoring-cluster-state.adoc Outdated Show resolved Hide resolved
modules/dr-restoring-cluster-state.adoc Outdated Show resolved Hide resolved
modules/dr-restoring-cluster-state.adoc Show resolved Hide resolved
@palonsoro
Copy link
Contributor

@JoeAldinger re-posting here because, for some reason, when I write on reviews, github sometimes doesn't forward my comments:

Re-reading, I saw that you deleted the following paragraphs

Validating and mutating admission webhooks can reject pods. If you add any additional webhooks with the `failurePolicy` set to `Fail`, then they can reject pods and the restoration process can fail. You can avoid this by saving and deleting webhooks while restoring the cluster state. After the cluster state is restored successfully, you can enable the webhooks again.

and

Alternatively, you can temporarily set the `failurePolicy` to `Ignore` while restoring the cluster state. After the cluster state is restored successfully, you can set the `failurePolicy` to `Fail`.

Please don't delete them. They are the solution for a previous bug, so it would be a regression.

Sorry that I didn't notice before.

@palonsoro
Copy link
Contributor

As said in the review, in case the comment doesn't make it: command at newer line 295 should be "delete", not "get".

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 25, 2023
@bergerhoffer bergerhoffer removed the peer-review-in-progress Signifies that the peer review team is reviewing this PR label Sep 27, 2023
@bergerhoffer bergerhoffer added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Sep 27, 2023
@JoeAldinger JoeAldinger added peer-review-needed Signifies that the peer review team needs to review this PR and removed peer-review-done Signifies that the peer review team has reviewed this PR labels Sep 27, 2023
@palonsoro
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 27, 2023
@kowen-rh
Copy link
Contributor

/remove-label peer-review-needed
/label peer-review-in-progress

@openshift-ci openshift-ci bot added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Sep 27, 2023
Copy link
Contributor

@kowen-rh kowen-rh left a comment

Choose a reason for hiding this comment

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

A few minor nitpicks, but overall this looks good to me!

/remove-label peer-review-in-progress
/label peer-review-done

modules/dr-restoring-cluster-state.adoc Outdated Show resolved Hide resolved
modules/dr-restoring-cluster-state.adoc Outdated Show resolved Hide resolved
modules/dr-restoring-cluster-state.adoc Outdated Show resolved Hide resolved
modules/dr-restoring-cluster-state.adoc Outdated Show resolved Hide resolved
@openshift-ci openshift-ci bot added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR labels Sep 27, 2023
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 27, 2023
@geliu2016
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 28, 2023
@JoeAldinger JoeAldinger merged commit 16bf3d1 into openshift:main Sep 28, 2023
1 check passed
@openshift-ci-robot
Copy link

@JoeAldinger: Jira Issue OCPBUGS-19403 is in an unrecognized state (MODIFIED) and will not be moved to the MODIFIED state.

In response to this:

Version(s):
4.14+

Issue:
https://issues.redhat.com/browse/OCPBUGS-19403

Link to docs preview:
https://64939--docspreview.netlify.app/openshift-enterprise/latest/backup_and_restore/control_plane_backup_and_restore/disaster_recovery/scenario-2-restoring-cluster-state#:~:text=If%20you%20are%20using%20the%20OVNKubernetes%20network%20plugin%2C%20you%20must%20restart%20ovnkube%2Dcontrolplane%20pods.

QE review:

  • QE has approved this change.

Additional information:
PTAL: @geliu2016
Looking for another review since I changed so much based on excellent feedback.

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.

@JoeAldinger JoeAldinger deleted the OCPBUGS-19403 branch September 28, 2023 12:09
@JoeAldinger
Copy link
Contributor Author

/cherrypick enterprise-4.14

@openshift-cherrypick-robot

@JoeAldinger: new pull request created: #65367

In response to this:

/cherrypick enterprise-4.14

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.14 jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. jira/severity-critical Referenced Jira bug's severity is critical for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants