Skip to content

Bug 1897543: DR: Make restore pod yaml specific to restoring node #497

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
openshift-merge-robot merged 1 commit into from
Dec 5, 2020
Merged

Bug 1897543: DR: Make restore pod yaml specific to restoring node #497

openshift-merge-robot merged 1 commit into from
Dec 5, 2020

Conversation

@retroflexer
Copy link

@retroflexer retroflexer commented Nov 17, 2020

The current restore pod yaml has node specific URLs which makes it not usable across the cluster on any node to restore.

This PR replaces node specific values in the backup at the time of restore with the values relevant to the node on which it is being restored.

@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 17, 2020

@retroflexer: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

Details

In response to this:

DR: Make restore pod yaml specific to restoring node

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.

@retroflexer retroflexer force-pushed the makerestore-pod-yaml-node-specific-4.5 branch 2 times, most recently from ce7d993 to 479e95b Compare November 17, 2020 21:06
@retroflexer retroflexer changed the title DR: Make restore pod yaml specific to restoring node [wip]: DR: Make restore pod yaml specific to restoring node Nov 17, 2020
@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 17, 2020

@retroflexer: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

Details

In response to this:

[wip]: DR: Make restore pod yaml specific to restoring node

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 do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 17, 2020
@marun
Copy link
Contributor

marun commented Nov 18, 2020

/retest

@marun
Copy link
Contributor

marun commented Nov 18, 2020

@retroflexer Is this really preferable to replacing the node name env var once with multi-line sed? Also, in the absence of automated testing of different-node restore (working on it but not yet), maybe document the test procedure for QA in the bz?

hexfusion
hexfusion reviewed Nov 18, 2020
View reviewed changes
@retroflexer retroflexer force-pushed the makerestore-pod-yaml-node-specific-4.5 branch from 479e95b to 14f1f51 Compare November 20, 2020 15:18
@retroflexer retroflexer changed the title [wip]: DR: Make restore pod yaml specific to restoring node Bug 1897543: DR: Make restore pod yaml specific to restoring node Nov 20, 2020
@openshift-ci-robot openshift-ci-robot added bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Nov 20, 2020
@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 20, 2020

@retroflexer: This pull request references Bugzilla bug 1897543, which is invalid:

  • expected dependent Bugzilla bug 1897542 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is MODIFIED instead
  • expected dependent Bugzilla bug 1895509 to target a release in 4.6.0, 4.6.z, but it targets "4.7.0" instead

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

Details

In response to this:

Bug 1897543: DR: Make restore pod yaml specific to restoring node

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.

@retroflexer
Copy link
Author

retroflexer commented Nov 20, 2020

/bugzilla refresh

@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 20, 2020

@retroflexer: This pull request references Bugzilla bug 1897543, which is invalid:

  • expected dependent Bugzilla bug 1897542 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is MODIFIED instead

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

Details

In response to this:

/bugzilla refresh

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.

@retroflexer retroflexer force-pushed the makerestore-pod-yaml-node-specific-4.5 branch from 14f1f51 to 9f39e18 Compare November 20, 2020 16:38
hexfusion
hexfusion reviewed Nov 20, 2020
View reviewed changes
hexfusion
hexfusion reviewed Nov 20, 2020
View reviewed changes
hexfusion
hexfusion reviewed Nov 20, 2020
View reviewed changes
@retroflexer retroflexer force-pushed the makerestore-pod-yaml-node-specific-4.5 branch 2 times, most recently from 420b2bf to 74c02be Compare November 20, 2020 17:24
marun
marun reviewed Nov 20, 2020
View reviewed changes
@marun
Copy link
Contributor

marun commented Nov 24, 2020 •
edited
Loading

@retroflexer Revisiting this reminds me how frustrated I am reasoning about the mix of rendering substitution and execution substitution (e.g. ${NODE_NODE_ENVAR_NAME_IP}).

I'm fine with what you've proposed provided it can be proven to work. Have you validated locally? I should have an e2e for you soon and maybe we'll want to backport.

@retroflexer
Copy link
Author

retroflexer commented Nov 24, 2020 •
edited
Loading

@retroflexer Revisiting this reminds me how frustrated I am reasoning about the mix of rendering substitution and execution substitution (e.g. ${NODE_NODE_ENVAR_NAME_IP}).

I'm fine with what you've proposed provided it can be proven to work. Have you validated locally? I should have an e2e for you soon and maybe we'll want to backport.

@marun yes, I have tested manually with:

  1. Backups taken on a different node with this patch
  2. Backups taken on a diffferent node without this patch, but restoring with this patch
  3. Backups taken on the same node with this patch

All of them have recovered successfully.

@retroflexer
Copy link
Author

retroflexer commented Nov 24, 2020

/retest

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 25, 2020

/retest

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

2 similar comments
@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 25, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 25, 2020

/retest

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

@retroflexer
Copy link
Author

retroflexer commented Nov 25, 2020

/retest

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 25, 2020

/retest

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

4 similar comments
@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 26, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 26, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 26, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 26, 2020

/retest

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

@geliu2016
Copy link

geliu2016 commented Dec 1, 2020

/bugzilla cc-qa

@openshift-ci-robot
Copy link

openshift-ci-robot commented Dec 1, 2020

@geliu2016: This pull request references Bugzilla bug 1897543, which is valid.

6 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.5.z) matches configured target release for branch (4.5.z)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
  • dependent bug Bugzilla bug 1897542 is in the state CLOSED (ERRATA), which is one of the valid states (VERIFIED, RELEASE_PENDING, CLOSED (ERRATA))
  • dependent Bugzilla bug 1897542 targets the "4.6.z" release, which is one of the valid target releases: 4.6.0, 4.6.z
  • bug has dependents

Requesting review from QA contact:
/cc @geliu2016

Details

In response to this:

/bugzilla cc-qa

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.

@geliu2016
Copy link

geliu2016 commented Dec 1, 2020

/lgtm

@openshift-ci-robot
Copy link

openshift-ci-robot commented Dec 1, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: geliu2016, hexfusion, marun, retroflexer

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

@hexfusion
Copy link
Contributor

hexfusion commented Dec 1, 2020

cc @sdodson @ecordell

@sdodson
Copy link
Member

sdodson commented Dec 1, 2020

cc @sdodson @ecordell

Once the PR is waiting only on cherry-pick-approved it should be reviewed by the patch manager for the week according to when the merge window opens without any need to ping someone.

@sdodson sdodson added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Dec 2, 2020
@retroflexer
Copy link
Author

retroflexer commented Dec 2, 2020

/retest

@sdodson
Copy link
Member

sdodson commented Dec 3, 2020

Putting a temporary hold on things to prioritize items that fix test flakes while we're hitting AWS quota limits. I'll clear this later today.
/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 3, 2020
@sdodson
Copy link
Member

sdodson commented Dec 4, 2020

/hold cancel
/retest

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 4, 2020
@sdodson
Copy link
Member

sdodson commented Dec 5, 2020

/retest

1 similar comment
@retroflexer
Copy link
Author

retroflexer commented Dec 5, 2020

/retest

@openshift-merge-robot openshift-merge-robot merged commit f213ba9 into openshift:release-4.5 Dec 5, 2020
@openshift-ci-robot
Copy link

openshift-ci-robot commented Dec 5, 2020

@retroflexer: All pull requests linked via external trackers have merged:

Bugzilla bug 1897543 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1897543: DR: Make restore pod yaml specific to restoring node

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.

@retroflexer retroflexer deleted the makerestore-pod-yaml-node-specific-4.5 branch December 6, 2020 00:09
@retroflexer
Copy link
Author

retroflexer commented Dec 6, 2020

/cherry-pick release-4.4

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Dec 6, 2020
Merged
@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Dec 6, 2020

@retroflexer: new pull request created: #506

Details

In response to this:

/cherry-pick release-4.4

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

Reviewers

@ironcladlou ironcladlou Awaiting requested review from ironcladlou

@geliu2016 geliu2016 Awaiting requested review from geliu2016

2 more reviewers

@marun marun marun left review comments

@hexfusion hexfusion hexfusion left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@marun marun

@hexfusion hexfusion

@geliu2016 geliu2016

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@retroflexer @openshift-ci-robot @marun @hexfusion @openshift-bot @geliu2016 @sdodson @openshift-cherrypick-robot @openshift-merge-robot