search

openshift / origin

Conformance test suite for OpenShift
http://www.openshift.org
Apache License 2.0
8.49k stars 4.7k forks source link

OCPBUGS-44244: use node node in deleted machine phases #29269

Closed kannon92 closed 1 week ago

kannon92 commented 2 weeks ago

This PR makes sure to add a node name to the DeletedMachine monitor.

Once that is added, we also added some unit tests to this code to make sure that we are correctly detecting UnexpectedNodeNotReady / UnexpectedUnreachable.

The main edge case we had to work around was when Machines get deleted and that node goes not ready. This is correct behavior but our test was flagging this as this code path does not go through a Machine Config change.

openshift-ci-robot commented 2 weeks ago

@kannon92: This pull request references Jira Issue OCPBUGS-44244, 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 version (4.18.0) matches configured target version for branch (4.18.0) * bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

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

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

In response to [this](https://github.com/openshift/origin/pull/29269): > Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Forigin). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
kannon92 commented 2 weeks ago

/jira refresh

openshift-ci-robot commented 2 weeks ago

@kannon92: This pull request references Jira Issue OCPBUGS-44244, which is valid.

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

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

In response to [this](https://github.com/openshift/origin/pull/29269#issuecomment-2458226342): >/jira refresh Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Forigin). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
kannon92 commented 2 weeks ago

/hold

I need to write some unit tests.

openshift-ci-robot commented 2 weeks ago

@kannon92: An error was encountered querying GitHub for users with public email (schoudha@redhat.com) for bug OCPBUGS-44244 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details.

Full error message. Post "http://ghproxy/graphql": dial tcp 172.30.229.2:80: i/o timeout

Please contact an administrator to resolve this issue, then request a bug refresh with /jira refresh.

In response to [this](https://github.com/openshift/origin/pull/29269): >This PR makes sure to add a node name to the DeletedMachine monitor. > >Once that is added, we also added some unit tests to this code to make sure that we are correctly detecting UnexpectedNodeNotReady / UnexpectedUnreachable. > >The main edge case we had to work around was when Machines get deleted and that node goes not ready. This is correct behavior but our test was flagging this as this code path does not go through a Machine Config change. Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Forigin). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
kannon92 commented 2 weeks ago

cc @rphillips @deads2k @dgoodwin

This is ready. I want to run the tests off of this PR so please don't merge until we confirm this works.

rphillips commented 2 weeks ago

Nice. This looks good... Let's see how the tests go.

kannon92 commented 2 weeks ago

/hold cancel

Something is up with these tests..

kannon92 commented 2 weeks ago

/jira refresh

openshift-ci-robot commented 2 weeks ago

@kannon92: This pull request references Jira Issue OCPBUGS-44244, which is valid.

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

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

In response to [this](https://github.com/openshift/origin/pull/29269#issuecomment-2460992960): >/jira refresh Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Forigin). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
openshift-ci-robot commented 2 weeks ago

@kannon92: This pull request references Jira Issue OCPBUGS-44244. The bug has been updated to no longer refer to the pull request using the external bug tracker. All external bug links have been closed. The bug has been moved to the NEW state.

In response to [this](https://github.com/openshift/origin/pull/29269): >This PR makes sure to add a node name to the DeletedMachine monitor. > >Once that is added, we also added some unit tests to this code to make sure that we are correctly detecting UnexpectedNodeNotReady / UnexpectedUnreachable. > >The main edge case we had to work around was when Machines get deleted and that node goes not ready. This is correct behavior but our test was flagging this as this code path does not go through a Machine Config change. Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Forigin). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
openshift-ci-robot commented 2 weeks ago

@kannon92: This pull request references Jira Issue OCPBUGS-44244, 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 version (4.18.0) matches configured target version for branch (4.18.0) * bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

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

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

In response to [this](https://github.com/openshift/origin/pull/29269): >This PR makes sure to add a node name to the DeletedMachine monitor. > >Once that is added, we also added some unit tests to this code to make sure that we are correctly detecting UnexpectedNodeNotReady / UnexpectedUnreachable. > >The main edge case we had to work around was when Machines get deleted and that node goes not ready. This is correct behavior but our test was flagging this as this code path does not go through a Machine Config change. Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Forigin). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
openshift-trt-bot commented 2 weeks ago

Job Failure Risk Analysis for sha: 8b023ce9126bf0a0d392fc5a1d55a083f0550878

Job Name Failure Risk
pull-ci-openshift-origin-master-e2e-aws-ovn-single-node-serial IncompleteTests
Tests for this run (26) are below the historical average (1642): IncompleteTests (not enough tests ran to make a reasonable risk analysis; this could be due to infra, installation, or upgrade problems)
pacevedom commented 2 weeks ago

/test e2e-aws-ovn-microshift

kannon92 commented 2 weeks ago

Found a little issue and kicked off a new round of testing.

I did not realize that the MachinePhase interval was constructed. So I added it to the machineDeletedInAPI but it wasn't in the MachinePhase interval. This should be fixed.

Once a test finishes, I will verify the artifacts to see if this is correct.

kannon92 commented 2 weeks ago

/retest

/hold cancel

Confirmed that the machines have the right node names in the MachinePhase Intervals.

openshift-trt-bot commented 2 weeks ago

Job Failure Risk Analysis for sha: dd5eb1662f5a3e959d28441bf97a0cac1576376c

Job Name Failure Risk
pull-ci-openshift-origin-master-e2e-aws-ovn-single-node-upgrade Low
[sig-node] static pods should start after being created
This test has passed 71.43% of 154 runs on release 4.18 [Architecture:amd64 FeatureSet:default Installer:ipi Network:ovn NetworkStack:ipv4 Platform:aws SecurityMode:default Topology:single Upgrade:micro] in the last week.

Open Bugs
Static pod controller pods sometimes fail to start [etcd]
---
[sig-node] static pods should start after being created
This test has passed 71.43% of 154 runs on release 4.18 [Architecture:amd64 FeatureSet:default Installer:ipi Network:ovn NetworkStack:ipv4 Platform:aws SecurityMode:default Topology:single Upgrade:micro] in the last week.

Open Bugs
Static pod controller pods sometimes fail to start [etcd]
kannon92 commented 1 week ago

@deads2k @dgoodwin @rphillips This PR is ready to merge. PTAL.

dgoodwin commented 1 week ago

/lgtm

openshift-ci-robot commented 1 week ago

/retest-required

Remaining retests: 0 against base HEAD 9cffbdd50a09dce4272e8f8dc006be549c00be97 and 2 for PR HEAD 1bf8c85a141a72d4117d29abdda980689364b140 in total

openshift-ci-robot commented 1 week ago

/retest-required

Remaining retests: 0 against base HEAD 23ca269161964f6da927967d5d30943607848f96 and 1 for PR HEAD 1bf8c85a141a72d4117d29abdda980689364b140 in total

openshift-ci-robot commented 1 week ago

/retest-required

Remaining retests: 0 against base HEAD 6807d0c65895662ec24f90aa3291466101abf6f2 and 0 for PR HEAD 1bf8c85a141a72d4117d29abdda980689364b140 in total

openshift-ci-robot commented 1 week ago

/hold

Revision 1bf8c85a141a72d4117d29abdda980689364b140 was retested 3 times: holding

openshift-trt-bot commented 1 week ago

Job Failure Risk Analysis for sha: 1bf8c85a141a72d4117d29abdda980689364b140

Job Name Failure Risk
pull-ci-openshift-origin-master-e2e-aws-ovn-edge-zones IncompleteTests
Tests for this run (102) are below the historical average (3107): IncompleteTests (not enough tests ran to make a reasonable risk analysis; this could be due to infra, installation, or upgrade problems)
kannon92 commented 1 week ago

/hold cancel

dgoodwin commented 1 week ago

/lgtm

openshift-ci[bot] commented 1 week ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dgoodwin, kannon92

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: - ~~[OWNERS](https://github.com/openshift/origin/blob/master/OWNERS)~~ [dgoodwin] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
openshift-ci[bot] commented 1 week ago

@kannon92: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-ovn-upgrade de21794cf4faa136a92c9325cc8d7d6d898ee447 link false /test e2e-aws-ovn-upgrade

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
openshift-ci-robot commented 1 week ago

@kannon92: Jira Issue OCPBUGS-44244: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-44244 has been moved to the MODIFIED state.

In response to [this](https://github.com/openshift/origin/pull/29269): >This PR makes sure to add a node name to the DeletedMachine monitor. > >Once that is added, we also added some unit tests to this code to make sure that we are correctly detecting UnexpectedNodeNotReady / UnexpectedUnreachable. > >The main edge case we had to work around was when Machines get deleted and that node goes not ready. This is correct behavior but our test was flagging this as this code path does not go through a Machine Config change. Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Forigin). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
openshift-bot commented 1 week ago

[ART PR BUILD NOTIFIER]

Distgit: openshift-enterprise-tests This PR has been included in build openshift-enterprise-tests-container-v4.18.0-202411122008.p0.g5e59368.assembly.stream.el9. All builds following this will include this PR.

kannon92 commented 2 days ago

/cherry-pick release-4.17

openshift-cherrypick-robot commented 2 days ago

@kannon92: #29269 failed to apply on top of branch "release-4.17":

Applying: Fix UnexpectedNodeNotReady and UnexpectedNodeUnreachable
Using index info to reconstruct a base tree...
M   pkg/monitortests/node/kubeletlogcollector/node.go
M   pkg/monitortests/node/kubeletlogcollector/node_test.go
M   pkg/monitortests/node/watchnodes/monitortest.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/monitortests/node/watchnodes/monitortest.go
CONFLICT (content): Merge conflict in pkg/monitortests/node/watchnodes/monitortest.go
Auto-merging pkg/monitortests/node/kubeletlogcollector/node_test.go
CONFLICT (content): Merge conflict in pkg/monitortests/node/kubeletlogcollector/node_test.go
Auto-merging pkg/monitortests/node/kubeletlogcollector/node.go
CONFLICT (content): Merge conflict in pkg/monitortests/node/kubeletlogcollector/node.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Fix UnexpectedNodeNotReady and UnexpectedNodeUnreachable
In response to [this](https://github.com/openshift/origin/pull/29269#issuecomment-2485580826): >/cherry-pick release-4.17 Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.