Skip to content

WIP: oc adm node-logs: Add compatibility with NodeLogQuery feature#1392

Closed
ardaguclu wants to merge 1 commit intoopenshift:masterfrom
ardaguclu:new-node-log-params
Closed

WIP: oc adm node-logs: Add compatibility with NodeLogQuery feature#1392
ardaguclu wants to merge 1 commit intoopenshift:masterfrom
ardaguclu:new-node-log-params

Conversation

@ardaguclu
Copy link
Copy Markdown
Member

This PR aligns query parameters in oc adm node-log command with the new upstream feature NodeLogQuery. For a couple of oc versions, new and old APIs will both be supported by setting query parameters blindly to cover both versions.

@openshift-ci openshift-ci Bot requested review from deads2k and mfojtik March 30, 2023 05:06
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Mar 30, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ardaguclu

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:

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 Mar 30, 2023
@ardaguclu
Copy link
Copy Markdown
Member Author

/uncc @deads2k @mfojtik
/cc @soltysh @aravindhp

@openshift-ci openshift-ci Bot requested review from aravindhp and soltysh and removed request for deads2k and mfojtik March 30, 2023 05:09
@ardaguclu
Copy link
Copy Markdown
Member Author

/test e2e-agnostic-ovn-cmd

This PR aligns query parameters in oc adm node-log command with the
new upstream feature NodeLogQuery. For a couple of oc versions, new and
old APIs will both be supported by setting query parameters blindly to
cover both versions.
@ardaguclu ardaguclu force-pushed the new-node-log-params branch from 321d893 to 3dfe80a Compare April 4, 2023 07:08
@ardaguclu ardaguclu changed the title oc adm node-logs: Add compatibility with NodeLogQuery feature WIP: oc adm node-logs: Add compatibility with NodeLogQuery feature Apr 4, 2023
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 4, 2023
}
}
if len(o.Grep) > 0 {
req.Param("pattern", o.Grep)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@aravindhp are pattern and grep interchangeable?.


// TODO: deprecate this flag in 4.16 and use --pattern flag
req.Param("grep", o.Grep)
req.Param("case-sensitive", fmt.Sprintf("%t", o.GrepCaseSensitive))
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There is no corresponding flag of case-sensitive?

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Apr 4, 2023

@ardaguclu: 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-aws-ovn-serial 3dfe80a link true /test e2e-aws-ovn-serial

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.

@ardaguclu
Copy link
Copy Markdown
Member Author

After discussions on Slack, it seems that this PR does not solve the problem we have. Therefore, I'm closing this one;

/close

@openshift-ci openshift-ci Bot closed this Apr 4, 2023
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Apr 4, 2023

@ardaguclu: Closed this PR.

Details

In response to this:

After discussions on Slack, it seems that this PR does not solve the problem we have. Therefore, I'm closing this one;

/close

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.

@soltysh
Copy link
Copy Markdown
Contributor

soltysh commented Apr 26, 2023

Replaced with #1403

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. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants