Skip to content

Remove condition from run_logs tasks of cifmw_setup#3204

Closed
cescgina wants to merge 1 commit intoopenstack-k8s-operators:mainfrom
cescgina:restore_logging_cifmw_setup
Closed

Remove condition from run_logs tasks of cifmw_setup#3204
cescgina wants to merge 1 commit intoopenstack-k8s-operators:mainfrom
cescgina:restore_logging_cifmw_setup

Conversation

@cescgina
Copy link
Copy Markdown
Contributor

After merging[1], jobs have stopped running must-gather to collect
service logs, e.g [2,3]. This seems to be caused by the change in [4].
Where before the 99-logs.yml playbook was run from an 'ansible-playbook'
invocation, without any inventory and thus the check for
'zuul_log_collection' was never triggered because the variable was never
set. Now, when importing the cifmw_setup role, the cifmw variables are
loaded and that variable which is in most jobs set to True prevents the
log collection, and most critically, must-gather collection.
This change removes the condition to preserve the previous behaviour.

[1] #3032
[2] https://softwarefactory-project.io/zuul/t/rdoproject.org/build/63673310b4f94883a9aff2db719a2095
[3] https://softwarefactory-project.io/zuul/t/rdoproject.org/build/03e87102fbf04dc7b482ecdba7128085
[4] cc87f54#diff-691dae244b4fb4bfc1270fb5d3b708c6be6073b0e33d997db15850edfa3fd976L23

@cescgina cescgina requested a review from a team as a code owner August 20, 2025 06:39
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Aug 20, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found 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

amartyasinha
amartyasinha previously approved these changes Aug 20, 2025
@amartyasinha amartyasinha requested a review from bshewale August 20, 2025 07:36
danpawlik
danpawlik previously approved these changes Aug 20, 2025
Copy link
Copy Markdown
Contributor

@danpawlik danpawlik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sens. Thanks

@amartyasinha amartyasinha enabled auto-merge (rebase) August 20, 2025 07:53
evallesp
evallesp previously approved these changes Aug 20, 2025
fmount
fmount previously approved these changes Aug 20, 2025
Copy link
Copy Markdown
Contributor

@fmount fmount left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@fmount
Copy link
Copy Markdown
Contributor

fmount commented Aug 20, 2025

thank you

bshewale
bshewale previously approved these changes Aug 20, 2025
@cescgina
Copy link
Copy Markdown
Contributor Author

/hold this is not working yet, needs more changes

@cescgina cescgina force-pushed the restore_logging_cifmw_setup branch from 91a4b3a to e6bd4f3 Compare August 20, 2025 08:31
@openshift-ci openshift-ci Bot removed the lgtm label Aug 20, 2025
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Aug 20, 2025

New changes are detected. LGTM label has been removed.

After merging[1], jobs have stopped running must-gather to collect
service logs, e.g [2,3]. This seems to be caused by the change in [4].
Where before the 99-logs.yml playbook was run from an 'ansible-playbook'
invocation, without any inventory and thus the check for
'zuul_log_collection' was never triggered because the variable was never
set. Now, when importing the cifmw_setup role, the cifmw variables are
loaded and that variable which is in most jobs set to True prevents the
log collection, and most critically, must-gather collection.
This change removes the condition to preserve the previous behaviour.

[1] openstack-k8s-operators#3032
[2] https://softwarefactory-project.io/zuul/t/rdoproject.org/build/63673310b4f94883a9aff2db719a2095
[3] https://softwarefactory-project.io/zuul/t/rdoproject.org/build/03e87102fbf04dc7b482ecdba7128085
[4] openstack-k8s-operators@cc87f54#diff-691dae244b4fb4bfc1270fb5d3b708c6be6073b0e33d997db15850edfa3fd976L23
@cescgina cescgina force-pushed the restore_logging_cifmw_setup branch from e6bd4f3 to e09cac1 Compare August 20, 2025 10:21
@cescgina
Copy link
Copy Markdown
Contributor Author

I don't think this approach will work. There are calls to include_vars like in https://github.com/openstack-k8s-operators/ci-framework/pull/3032/files#diff-20b693089f9adf9a3c7c6053da094f591663657757d9c7c335f5eb85e79af43cR20 and other which will not work as intended now. They are executed in the controller (the zuul controller), but are passed paths of the controller cifmw vm. This was fine before because the playbook was called as a an ansible-playbook from the controller vm, not the zuul-controller, but is now failing

@softwarefactory-project-zuul
Copy link
Copy Markdown

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://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/55d8ff7b77d6434aa39a90f048a1f6c9

✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 08m 18s
podified-multinode-edpm-deployment-crc POST_FAILURE in 1h 16m 23s
cifmw-crc-podified-edpm-baremetal POST_FAILURE in 1h 18m 09s
adoption-standalone-to-crc-ceph-provider POST_FAILURE in 2h 51m 38s
✔️ noop SUCCESS in 0s
✔️ cifmw-pod-ansible-test SUCCESS in 8m 18s
✔️ cifmw-pod-pre-commit SUCCESS in 8m 20s
✔️ build-push-container-cifmw-client SUCCESS in 21m 29s
✔️ cifmw-molecule-cifmw_setup SUCCESS in 2m 24s

@cescgina cescgina closed this Aug 20, 2025
auto-merge was automatically disabled August 20, 2025 13:48

Pull request was closed

@cescgina
Copy link
Copy Markdown
Contributor Author

Closing this in favor of #3206

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants