tripleo_cleanup: Fix a task that was always skipped and it shouldn't be

jistr commented 4 months ago

Apparently the 'when' condition on a block is not evaluated only before the block begins executing, but it is evaluated for each task separately. That means that if the fact that is used in the 'when' condition changes during the block execution, it affects whether the following tasks in the block will execute or not.

In our case, the "Filter for tripleo services" task set the tripleo_services fact, and due to that the following "Filter services in skip list" task never executed because the "when" condition on the block says to execute only if tripleo_services is empty.

I've pulled the two set_fact tasks into a single one to get around this issue, since i didn't want to pollute the facts list with a temporary fact. (Facts can't be undefined.)

Resolves: OSPRH-8584

openshift-ci[bot] commented 4 months ago

@odyssey4me: changing LGTM is restricted to collaborators

In response to [this](https://github.com/openstack-k8s-operators/edpm-ansible/pull/695#pullrequestreview-2174451940): > 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.

jistr commented 4 months ago

Pending testing with testproject - this is only caught by tempest

jistr commented 4 months ago

/hold

softwarefactory-project-zuul[bot] commented 4 months ago

Build failed (check pipeline). Post recheck (without leading slash) to rerun all jobs. Make sure the failure cause has been resolved before you rerun jobs.

https://review.rdoproject.org/zuul/buildset/34970aea68774954a14d5408ef4ebd9b

:x: openstack-k8s-operators-content-provider ERROR Failed to update project openstack-k8s-operators/ci-framework in 2m 00s :warning: podified-multinode-edpm-deployment-crc SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider :warning: cifmw-crc-podified-edpm-baremetal SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider :heavy_check_mark: edpm-ansible-molecule-edpm_bootstrap SUCCESS in 6m 25s :heavy_check_mark: edpm-ansible-molecule-edpm_podman SUCCESS in 4m 54s :heavy_check_mark: edpm-ansible-molecule-edpm_module_load SUCCESS in 4m 39s :heavy_check_mark: edpm-ansible-molecule-edpm_kernel SUCCESS in 8m 20s :heavy_check_mark: edpm-ansible-molecule-edpm_libvirt SUCCESS in 8m 04s :heavy_check_mark: edpm-ansible-molecule-edpm_nova SUCCESS in 8m 20s :heavy_check_mark: edpm-ansible-molecule-edpm_frr SUCCESS in 6m 23s :heavy_check_mark: edpm-ansible-molecule-edpm_iscsid SUCCESS in 4m 04s :heavy_check_mark: edpm-ansible-molecule-edpm_ovn_bgp_agent SUCCESS in 7m 30s :heavy_check_mark: edpm-ansible-molecule-edpm_ovs SUCCESS in 11m 32s :heavy_check_mark: edpm-ansible-molecule-edpm_tripleo_cleanup SUCCESS in 3m 51s

jpodivin commented 4 months ago

recheck

marios commented 3 months ago

lgtm, can we also include the edpm_tripleo_cleanup role in

https://github.com/openstack-k8s-operators/edpm-ansible/blob/10eb44961c04c19aabcc81225ca54f38035a862f/zuul.d/projects.yaml#L24

could be followup as i think this is on the way to merge

ah, i just saw it has the do not merge label still so ... could update here goign to ping jistr now

marios commented 3 months ago

lgtm, can we also include the edpm_tripleo_cleanup role in https://github.com/openstack-k8s-operators/edpm-ansible/blob/10eb44961c04c19aabcc81225ca54f38035a862f/zuul.d/projects.yaml#L24

could be followup as i think this is on the way to merge

ah, i just saw it has the do not merge label still so ... could update here goign to ping jistr now

so, actually if we do this, we would get the 'ceph job' which is fine, but it wouldn't catch the issue being fixed here (which was hitting the non ceph job, and then only in periodic where we run tempest). Still though I think it is worth running the job on that role since it is being consumed by the adoption tests

openshift-ci[bot] commented 3 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fao89, jistr, jpodivin, odyssey4me, xek

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/openstack-k8s-operators/edpm-ansible/blob/main/OWNERS)~~ [fao89,jpodivin] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment