Skip to content

Comments

adding correlationID to cleanup job output#4138

Draft
sclarkso wants to merge 3 commits intomainfrom
cleanup-correlation-id-output
Draft

adding correlationID to cleanup job output#4138
sclarkso wants to merge 3 commits intomainfrom
cleanup-correlation-id-output

Conversation

@sclarkso
Copy link
Collaborator

What

add correlationID to error output on cleanup job

Why

frontend was broken in int today and troubleshooting the cleanup job would have been easier with correlationID.

Special notes for your reviewer

@openshift-ci
Copy link

openshift-ci bot commented Feb 18, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sclarkso
Once this PR has been reviewed and has the lgtm label, please assign patriksuba for approval. For more information see the Code Review Process.

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

@openshift-ci
Copy link

openshift-ci bot commented Feb 18, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@sclarkso
Copy link
Collaborator Author

/test e2e-parallel

@sclarkso
Copy link
Collaborator Author

sclarkso commented Feb 18, 2026

@roivaz Can you take a look at this change? When I tried to commit it, it said that this file was in the .gitignore, so not sure if this is the correct way of changing this or not?

Comment on lines 111 to 123
var respErr *azcore.ResponseError
if errors.As(err, &respErr) && respErr.RawResponse != nil {
// Extract the Correlation ID header
corrID := respErr.RawResponse.Header.Get("x-ms-correlation-request-id")

logger.Error(err, "Failed to delete some resource groups",
"count", len(resourceGroupsToDelete),
"correlationID", corrID)

return fmt.Errorf("cleanup failed (CorrelationID: %s): %w", corrID, err)
}

// Fallback for non-Azure errors
Copy link
Collaborator

Choose a reason for hiding this comment

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

This won't work as expected. tc.CleanupResourceGroups(..) can return several errors if more than one resource group fails. In that scenario you'd be returning the correlation-id for just one of them. I suggests moving this to the tc.CleanupResourceGroups(..) function so you can add the specific correlation-id for each failed operation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I've moved it to the correct spot. Thank you.

@roivaz
Copy link
Collaborator

roivaz commented Feb 18, 2026

When I tried to commit it, it said that this file was in the .gitignore, so not sure if this is the correct way of changing this or not?

I see that in the top .gitignore there's a rule that matches resourcegroups. The pattern is too wide and probably matching this file incorrectly.

@sclarkso
Copy link
Collaborator Author

/test e2e-parallel

@openshift-ci
Copy link

openshift-ci bot commented Feb 18, 2026

@sclarkso: 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-parallel c179b3a link true /test e2e-parallel

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

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.

2 participants