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

Bug 1986656: Fix missing case of BuildRAIDCleanSteps #170

Merged
merged 1 commit into from Aug 6, 2021

Conversation

Hellcatlk
Copy link

I will pick up the upstream PRs after that has been merged, and will add that commits into this one.
URL of upstream PRs are as follows.

Therefore, please pend the review of this PR.

@openshift-ci openshift-ci bot requested review from asalkeld and dtantsur July 22, 2021 12:10
@openshift-ci openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 22, 2021
@openshift-ci
Copy link

openshift-ci bot commented Jul 22, 2021

Hi @Hellcatlk. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@Hellcatlk Hellcatlk force-pushed the hardware-raid branch 3 times, most recently from d246b06 to 32ba3e9 Compare July 22, 2021 14:21
@hardys
Copy link

hardys commented Jul 27, 2021

@Hellcatlk thanks for the PR - typically we do periodic rebases to pick up merged PRs from the upstream BMO repo (see recent merged PRs from @honza) so it may be best to follow that process again here I think?

@Hellcatlk
Copy link
Author

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 28, 2021
@Hellcatlk
Copy link
Author

Hellcatlk commented Jul 28, 2021

@Hellcatlk thanks for the PR - typically we do periodic rebases to pick up merged PRs from the upstream BMO repo (see recent merged PRs from @honza) so it may be best to follow that process again here I think?

Understood, thanks for reminding, I will hold this PR

@Hellcatlk Hellcatlk changed the title Fix missing case of BuildRAIDCleanSteps Bug 1986656: Ironic node enters the clean failed state when the target node doesn't have a RAID controller. Jul 28, 2021
@openshift-ci openshift-ci bot added bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Jul 28, 2021
@openshift-ci
Copy link

openshift-ci bot commented Jul 28, 2021

@Hellcatlk: This pull request references Bugzilla bug 1986656, which is invalid:

  • expected the bug to target the "4.9.0" release, but it targets "---" 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.

In response to this:

Bug 1986656: Ironic node enters the clean failed state when the target node doesn't have a RAID controller.

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.

@hase1128
Copy link

/bugzilla refresh

@openshift-ci openshift-ci bot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Jul 28, 2021
@openshift-ci
Copy link

openshift-ci bot commented Jul 28, 2021

@hase1128: This pull request references Bugzilla bug 1986656, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.9.0) matches configured target release for branch (4.9.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla (augol@redhat.com), skipping review request.

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.

@hase1128
Copy link

@hardys

This PR's target is 4.9.0. Now there is the related BZ.
In this case, this PR is required to merge, right?

@rhjanders
Copy link

/bugzilla refresh

@openshift-ci openshift-ci bot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. and removed bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Jul 29, 2021
@openshift-ci
Copy link

openshift-ci bot commented Jul 29, 2021

@rhjanders: This pull request references Bugzilla bug 1986656, which is invalid:

  • expected the bug to target the "4.9.0" release, but it targets "---" 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.

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.

@openshift-ci openshift-ci bot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Jul 29, 2021
@rhjanders
Copy link

/bugzilla refresh

@openshift-ci openshift-ci bot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Jul 29, 2021
@openshift-ci
Copy link

openshift-ci bot commented Jul 29, 2021

@rhjanders: This pull request references Bugzilla bug 1986656, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.9.0) matches configured target release for branch (4.9.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla (augol@redhat.com), skipping review request.

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.

@openshift-ci openshift-ci bot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Jul 29, 2021
@openshift-ci openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 29, 2021
@Hellcatlk
Copy link
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 Aug 2, 2021
@Hellcatlk
Copy link
Author

/retest

1 similar comment
@hase1128
Copy link

hase1128 commented Aug 2, 2021

/retest

@honza
Copy link
Member

honza commented Aug 5, 2021

/hold

All but the last commit are already present on the master branch so we don't need to include them. The only commit that we might want to bring in is the one introduced by the PR mentioned in the description.

metal3-io#942

Also, may I suggest using the -x flag of git cherry-pick when backporting code? It aids in any future git archeology endeavours.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 5, 2021
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 6, 2021
When `hardwareRAIDVolumes` is nil and the target node doesn't have a RAID
controller. We shouldn't delete the RAID configuration on that node in
that case, otherwise the `ironic` node will enter the clean failed state
because of `delete_configuration` clean step failure. So in this case, hardware
RAID operations will be skipped.

Signed-off-by: Zou Yu <zouy.fnst@cn.fujitsu.com>
(cherry picked from commit 32ba3e9)
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 6, 2021
@Hellcatlk
Copy link
Author

/hold

All but the last commit are already present on the master branch so we don't need to include them. The only commit that we might want to bring in is the one introduced by the PR mentioned in the description.

metal3-io#942

Also, may I suggest using the -x flag of git cherry-pick when backporting code? It aids in any future git archeology endeavours.

@honza Thanks, done.

@rhjanders
Copy link

/test e2e-metal-ipi-ovn-ipv6

@rhjanders
Copy link

/retest

@hardys
Copy link

hardys commented Aug 6, 2021

/retitle Bug 1986656: Fix missing case of BuildRAIDCleanSteps

(This makes it clearer this PR is now just a backport of metal3-io#942)

@openshift-ci openshift-ci bot changed the title Bug 1986656: Ironic node enters the clean failed state when the target node doesn't have a RAID controller. Bug 1986656: Fix missing case of BuildRAIDCleanSteps Aug 6, 2021
@hardys
Copy link

hardys commented Aug 6, 2021

/approve

This looks like a valid backport of metal3-io#942 to me, thanks!

/cc @andfasano @dtantsur

@openshift-ci openshift-ci bot requested a review from andfasano August 6, 2021 08:35
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 6, 2021
@hardys
Copy link

hardys commented Aug 6, 2021

/hold cancel

This is now just a backport of as requested by @honza

@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 Aug 6, 2021
@sadasu
Copy link

sadasu commented Aug 6, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 6, 2021
@openshift-ci
Copy link

openshift-ci bot commented Aug 6, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hardys, Hellcatlk, sadasu

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

The pull request process is described here

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 merged commit 2e0f5db into openshift:master Aug 6, 2021
@openshift-ci
Copy link

openshift-ci bot commented Aug 6, 2021

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

Bugzilla bug 1986656 has been moved to the MODIFIED state.

In response to this:

Bug 1986656: Fix missing case of BuildRAIDCleanSteps

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.

@Hellcatlk Hellcatlk deleted the hardware-raid branch January 30, 2023 05:48
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. bugzilla/severity-medium Referenced Bugzilla bug's severity is medium 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. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants