OCPBUGS-7310: Fix test checking if Machine addresses match VMs

[dulek]

The aforementioned test blindly tries to check all the Machines in the cluster, assuming each have a VM in OpenStack. That might not be true as in other tests we create Machines that are supposed to fail provisioning and never produce a VM. This commit fixes this by adding several precautions:

• We only consider machines having addresses set in .status.
• We skip machines without openstack-resourceId annotation.
• We skip checking machines that do not have VM in OpenStack (could have been deleted between we fetched the Machines and did OpenStack query).

[openshift-ci-robot]

@dulek: This pull request references Jira Issue OCPBUGS-7310, 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.13.0) matches configured target version for branch (4.13.0)
• bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @eurijon

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

Details

In response to this:

The aforementioned test blindly tries to check all the Machines in the cluster, assuming each have a VM in OpenStack. That might not be true as in other tests we create Machines that are supposed to fail provisioning and never produce a VM. This commit fixes this by adding several precautions:

• We only consider machines having addresses set in .status.
• We skip machines without openstack-resourceId annotation.
• We skip checking machines that do not have VM in OpenStack (could have been deleted between we fetched the Machines and did OpenStack query).

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.

[dulek]

/cc @mandre
/cc @pierreprinetti

[dulek]

/cherry-pick release-4.12

[openshift-cherrypick-robot]

@dulek: once the present PR merges, I will cherry-pick it on top of release-4.12 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-4.12

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.

[rlobillo]

Hi @dulek,

I think we should not skip machines without openstack-resourceId annotation. That is one of the main checks in the test.

Isnt "only consider machines having addresses set in .status" enough for stabilizing the test?

[dulek]

Hi @dulek,

I think we should not skip machines without openstack-resourceId annotation. That is one of the main checks in the test.

Isnt "only consider machines having addresses set in .status" enough for stabilizing the test?

But if openstackResourceId == "", then the try to fetch the VM from Nova will imminently fail.

[dulek]

/retest

[rlobillo]

Hi @dulek,
I think we should not skip machines without openstack-resourceId annotation. That is one of the main checks in the test.
Isnt "only consider machines having addresses set in .status" enough for stabilizing the test?

But if openstackResourceId == "", then the try to fetch the VM from Nova will imminently fail.

Please ignore my comment. The openstack-resourceId annotation is checked on the previous test

openstack-test/test/extended/openstack/servers.go

Line 89 in 85a5fb2

o.Expect(err).NotTo(o.HaveOccurred(), "Error gathering Openstack info for machine %v", machine_name)

Sorry for the noise

[mandre]

Just a small nit, otherwise LGTM.

[mandre]

/lgtm
/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mandre

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:

• OWNERS [mandre]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci]

@dulek: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

[openshift-ci-robot]

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

• openshift/openstack-test#84
• openshift/openstack-test#88

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

Details

In response to this:

The aforementioned test blindly tries to check all the Machines in the cluster, assuming each have a VM in OpenStack. That might not be true as in other tests we create Machines that are supposed to fail provisioning and never produce a VM. This commit fixes this by adding several precautions:

• We only consider machines having addresses set in .status.
• We skip machines without openstack-resourceId annotation.
• We skip checking machines that do not have VM in OpenStack (could have been deleted between we fetched the Machines and did OpenStack query).

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-cherrypick-robot]

@dulek: new pull request created: #90

Details

In response to this:

/cherry-pick release-4.12

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.