cluster-logging-operator icon indicating copy to clipboard operation
cluster-logging-operator copied to clipboard

LOG-6277: Fix LokiStack is not gathered even if it exists.

Open kattz-kawa opened this issue 1 year ago • 5 comments

Description

The must-gather code[1] gathers LokiStack only when "lokistack" wasn't found in the result of "oc get crd". It means that LokiStack isn't gathered even if it exists.

[1] https://github.com/openshift/cluster-logging-operator/blob/b8e2d71d573fca3717c232d3778aa021f63fc15d/must-gather/collection-scripts/gather#L94

The current line should use && instead of ||. This change is important because, in bash, if the left expression of || is true, the right expression is not evaluated. By using &&, the right expression will be evaluated if LokiStack exists, ensuring that the information is gathered correctly.

Here is an example to demonstrate this point:

$ bash -c 'echo first expression || echo second expression'
first expression  # <--- second expression is not evaluated
$ bash -c 'echo first expression && echo second expression'
first expression
second expression

/cc @Clee2691 /assign @jcantrill

Links

JIRA: https://issues.redhat.com/browse/LOG-6277

kattz-kawa avatar Oct 24 '24 22:10 kattz-kawa

@kattz-kawa: This pull request references LOG-6277 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.8.0" version, but no target version was set.

In response to this:

Description

The must-gather code[1] gathers LokiStack only when "lokistack" wasn't found in the result of "oc get crd". It means that LokiStack isn't gathered even if it exists.

[1] https://github.com/openshift/cluster-logging-operator/blob/b8e2d71d573fca3717c232d3778aa021f63fc15d/must-gather/collection-scripts/gather#L94

The current line should use && instead of ||. This change is important because, in bash, if the left expression of || is true, the right expression is not evaluated. By using &&, the right expression will be evaluated if LokiStack exists, ensuring that the information is gathered correctly.

Here is an example to demonstrate this point:

$ bash -c 'echo first expression || echo second expression'
first expression  # <--- second expression is not evaluated
$ bash -c 'echo first expression && echo second expression'
first expression
second expression

/cc @Clee2691 /assign @jcantrill

Links

JIRA: https://issues.redhat.com/browse/LOG-6277

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-robot avatar Oct 24 '24 22:10 openshift-ci-robot

Hi @kattz-kawa. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

openshift-ci[bot] avatar Oct 24 '24 22:10 openshift-ci[bot]

/ok-to-test

jcantrill avatar Oct 25 '24 17:10 jcantrill

/hold

jcantrill avatar Nov 05 '24 13:11 jcantrill

/hold cancel

jcantrill avatar Nov 15 '24 18:11 jcantrill

/cherrypick release-6.1

jcantrill avatar Nov 15 '24 18:11 jcantrill

@jcantrill: once the present PR merges, I will cherry-pick it on top of release-6.1 in a new PR and assign it to you.

In response to this:

/cherrypick release-6.1

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.

/approved /lgtm

jcantrill avatar Nov 15 '24 18:11 jcantrill

/approved /lgtm

Hi @jcantrill, I noticed that the '/approved' might be a typo and should perhaps be '/approve'. Could we confirm this?

kattz-kawa avatar Nov 22 '24 15:11 kattz-kawa

/approve

cahartma avatar Nov 26 '24 18:11 cahartma

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cahartma, kattz-kawa

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[bot] avatar Nov 26 '24 18:11 openshift-ci[bot]

@kattz-kawa: all tests passed!

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-ci[bot] avatar Nov 26 '24 20:11 openshift-ci[bot]

@jcantrill: new pull request created: #2883

In response to this:

/cherrypick release-6.1

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.

/cherrypick release-6.0

jcantrill avatar Jan 10 '25 17:01 jcantrill

@jcantrill: new pull request created: #2920

In response to this:

/cherrypick release-6.0

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.