Skip to content

Conversation

@tchap
Copy link

@tchap tchap commented Sep 29, 2025

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This reverts commit 6d6b285

It removes isTerminating check from logging HTTP requests.
This check has actually been untested and broken. Reverting the commit
does not actually change any behaviour, it only removes duplicate HTTP
request logs, which are actually an issue.

Which issue(s) this PR fixes:

N/A

Special notes for your reviewer:

N/A

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

N/A

@openshift-ci-robot openshift-ci-robot added the backports/unvalidated-commits Indicates that not all commits come to merged upstream PRs. label Sep 29, 2025
@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. labels Sep 29, 2025
@openshift-ci-robot
Copy link

@tchap: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

@tchap tchap changed the title WIP: Rollback httplog termination logging WIP: Test drop carry httplog termination logging Sep 29, 2025
@openshift-ci openshift-ci bot added the vendor-update Touching vendor dir or related files label Sep 29, 2025
@p0lyn0mial
Copy link

lgtm

the original pr is very old (found in 4.15) and never worked. by removing the commit we won't change any behaviour. the code will work as before. i am ok with that.

@tchap we need to change the commit message.
@tchap please verify it manually.

@p0lyn0mial
Copy link

@bertinatto once this pr merges we should also remove the original commit from the current rebase PR. we will let you know.

@openshift-ci-robot
Copy link

@tchap: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

@tchap tchap changed the title WIP: Test drop carry httplog termination logging UPSTREAM: <drop>: kas: Remove isTerminating flag Sep 30, 2025
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 30, 2025
@tchap
Copy link
Author

tchap commented Sep 30, 2025

we need to change the commit message.

@p0lyn0mial How is it supposed to look like? Sorry, I am new to this.

@bertinatto
Copy link
Member

bertinatto commented Sep 30, 2025

we need to change the commit message.

@p0lyn0mial How is it supposed to look like? Sorry, I am new to this.

Please use the same title as 6d6b285, including the <carry> rather than <drop>.

Edit: oh, please keep the current description, where you say it reverts 6d6b285.

@p0lyn0mial
Copy link

Please use the same title as 6d6b285, including the rather than .

Edit: oh, please keep the current description, where you say it reverts 6d6b285.

ok, thanks, so we change the tile to match the original PR but keep the description where we say it reverts the original commit.

…ndler chain

This reverts commit 6d6b285

It removes isTerminating check from logging HTTP requests.
This check has actually been untested and broken. Reverting the commit
does not actually change any behaviour, it only removes duplicate HTTP
request logs, which are actually an issue.
@tchap tchap changed the title UPSTREAM: <drop>: kas: Remove isTerminating flag UPSTREAM: <carry>: kube-apiserver: wire through isTerminating into handler chain Sep 30, 2025
@openshift-ci-robot
Copy link

@tchap: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

@tchap
Copy link
Author

tchap commented Sep 30, 2025

Please use the same title as 6d6b285

Renamed.

@p0lyn0mial
Copy link

@tchap have you verified it solves the duplicate entries issue ?

@tchap
Copy link
Author

tchap commented Sep 30, 2025

have you verified it solves the duplicate entries issue ?

@p0lyn0mial I am on it, will let you know.

@p0lyn0mial
Copy link

/lgtm

/hold
for manual check #2477 (comment)

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 30, 2025
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 30, 2025
@openshift-ci
Copy link

openshift-ci bot commented Sep 30, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: p0lyn0mial, tchap

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 30, 2025
@p0lyn0mial p0lyn0mial removed the backports/unvalidated-commits Indicates that not all commits come to merged upstream PRs. label Sep 30, 2025
@bertinatto
Copy link
Member

/retest-required

@tchap tchap changed the title UPSTREAM: <carry>: kube-apiserver: wire through isTerminating into handler chain OCPBUGS-43994: UPSTREAM: <carry>: kube-apiserver: wire through isTerminating into handler chain Sep 30, 2025
@openshift-ci-robot openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Sep 30, 2025
@openshift-ci-robot
Copy link

@tchap: This pull request references Jira Issue OCPBUGS-43994, which is valid.

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

Requesting review from QA contact:
/cc @wangke19

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

In response to this:

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This reverts commit 6d6b285

It removes isTerminating check from logging HTTP requests.
This check has actually been untested and broken. Reverting the commit
does not actually change any behaviour, it only removes duplicate HTTP
request logs, which are actually an issue.

Which issue(s) this PR fixes:

N/A

Special notes for your reviewer:

N/A

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

N/A

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot requested a review from wangke19 September 30, 2025 18:42
@tchap
Copy link
Author

tchap commented Sep 30, 2025

I tested this manully

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 30, 2025
@p0lyn0mial
Copy link

/retest-required
/hold cancel
/verified by @tchap

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Oct 1, 2025
@openshift-ci-robot
Copy link

@p0lyn0mial: This PR has been marked as verified by @tchap.

In response to this:

/retest-required
/hold cancel
/verified by @tchap

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 openshift-eng/jira-lifecycle-plugin repository.

@p0lyn0mial
Copy link

@bertinatto @jacobsee please don't forget to remove the patch from the current rebase PR.

@openshift-ci
Copy link

openshift-ci bot commented Oct 1, 2025

@tchap: The following tests 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-csi f6d00c9 link false /test e2e-aws-csi
ci/prow/e2e-aws-ovn-techpreview f6d00c9 link false /test e2e-aws-ovn-techpreview
ci/prow/e2e-aws-ovn-techpreview-serial f6d00c9 link false /test e2e-aws-ovn-techpreview-serial
ci/prow/k8s-e2e-aws-ovn-serial f6d00c9 link false /test k8s-e2e-aws-ovn-serial
ci/prow/okd-scos-e2e-aws-ovn f6d00c9 link false /test okd-scos-e2e-aws-ovn

Full PR test history. Your PR dashboard.

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.

@openshift-merge-bot openshift-merge-bot bot merged commit ecfa8cc into openshift:master Oct 1, 2025
20 of 25 checks passed
@openshift-ci-robot
Copy link

@tchap: Jira Issue OCPBUGS-43994: Some pull requests linked via external trackers have merged:

The following pull request, linked via external tracker, has not merged:

All associated pull requests must be merged or unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with /jira refresh.

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

This PR is marked as verified. If the remaining PRs listed above are marked as verified before merging, the issue will automatically be moved to VERIFIED after all of the changes from the PRs are available in an accepted nightly payload.

In response to this:

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This reverts commit 6d6b285

It removes isTerminating check from logging HTTP requests.
This check has actually been untested and broken. Reverting the commit
does not actually change any behaviour, it only removes duplicate HTTP
request logs, which are actually an issue.

Which issue(s) this PR fixes:

N/A

Special notes for your reviewer:

N/A

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

N/A

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 openshift-eng/jira-lifecycle-plugin repository.

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. vendor-update Touching vendor dir or related files verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants