Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collecting clusterlogging and clusterlogforwarder with inspect (#5) #2257

Closed
wants to merge 1 commit into from

Conversation

oarribas
Copy link
Contributor

Description

Get the clusterlogging and clusterlogforwarder with the oc adm inspect command to allow working with those resources with omc like in a "live" cluster. Due to this, it's no longer needed to get them again and put them into the clo directory.
The customresourcedefinitions are also required to allow omc to work with above (and other) resources (on testing, it adds only few MB to the full must-gather size for ~200 customresourcedefinitions).

Added a "Moved resources" section to the must-gather README to explain how to work with the clusterlogging and clusterlogforwarder after this change.

/cc @periklis @cahartma @jcantrill
/assign @cahartma

/cherry-pick release-5.8

Links

* Collect CRDs, CLs and CLFs in the gather script.
* Update README.md: document the resources already in `namespaces` dir.
Copy link
Contributor

openshift-ci bot commented Nov 17, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@oarribas
Copy link
Contributor Author

/retest

Copy link
Contributor

openshift-ci bot commented Nov 17, 2023

@oarribas: 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/test-infra repository. I understand the commands that are listed here.

@jcantrill
Copy link
Contributor

/hold

@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 Nov 17, 2023
@jcantrill
Copy link
Contributor

@oarribas have you tested the results of these changes. I like the intent but based upon your comments it would suggest 'openshift-logging' is still being collected like any other. Line 32, however, suggests to me that we are intentionally skipping that namespace https://github.com/openshift/cluster-logging-operator/pull/2257/files#diff-e5c2a69519b4a8fddd46d182316399558c9fd41434650a1196c7a9beaa14ada0R32 . Please verify. It would be also useful to write a JIRA ticket to express what we are fixing so that it can be included with an errata. Please do so.

@oarribas
Copy link
Contributor Author

oarribas commented Nov 17, 2023

@jcantrill , I think that namespace is intentionally not checked there because it's added always [1] (for the non-multi-forwarder configurations), and with this PR the clusterlogging and clusterlogforwarder are now collected by [2].

The logs generated by [2] shows the following:

[...]
2023-11-17 10:47:16 END inspecting namespace openshift-logging/events ...
2023-11-17 10:47:16 BEGIN inspecting namespace openshift-logging/clusterlogging ...
Wrote inspect data to /tmp/must-gather.
2023-11-17 10:47:16 END inspecting namespace openshift-logging/clusterlogging ...
2023-11-17 10:47:16 BEGIN inspecting namespace openshift-logging/clusterlogforwarder ...
Wrote inspect data to /tmp/must-gather.
2023-11-17 10:47:17 END inspecting namespace openshift-logging/clusterlogforwarder ...
2023-11-17 10:47:17 BEGIN inspecting namespace openshift-operators-redhat/pods ...
Wrote inspect data to /tmp/must-gather.
[...]

Checking the generated must-gather directory, the following

$ find . -name "clusterlog*"
./cluster-logging/clo/clusterlogging_instance.txt
./namespaces/openshift-logging/logging.openshift.io/clusterloggings
./namespaces/openshift-logging/logging.openshift.io/clusterlogforwarders
./namespaces/default/logging.openshift.io/clusterlogforwarders
./namespaces/test-httpd/logging.openshift.io/clusterlogforwarders
[...]

$ ls -ltrh namespaces/openshift-logging/logging.openshift.io/clusterloggings/ namespaces/openshift-logging/logging.openshift.io/clusterlogforwarders/ namespaces/default/logging.openshift.io/clusterlogforwa
rders/ namespaces/test-httpd/logging.openshift.io/clusterlogforwarders/

namespaces/openshift-logging/logging.openshift.io/clusterloggings/:
total 4.0K
-rwxr-xr-x. 1 x x 3.0K Nov 17 10:47 instance.yaml

namespaces/openshift-logging/logging.openshift.io/clusterlogforwarders/:
total 4.0K
-rwxr-xr-x. 1 x x 1.9K Nov 17 10:47 instance.yaml

namespaces/default/logging.openshift.io/clusterlogforwarders/:
total 4.0K
-rwxr-xr-x. 1 x x 810 Nov 17 10:47 instance.yaml

namespaces/test-httpd/logging.openshift.io/clusterlogforwarders/:
total 4.0K
-rwxr-xr-x. 1 x x 816 Nov 17 10:47 instance.yaml

Maybe we can use jira LOG-4792 to document this, as this will also fix that issue?

[1] https://github.com/openshift/cluster-logging-operator/pull/2257/files#diff-e5c2a69519b4a8fddd46d182316399558c9fd41434650a1196c7a9beaa14ada0R24
[2] https://github.com/openshift/cluster-logging-operator/pull/2257/files#diff-e5c2a69519b4a8fddd46d182316399558c9fd41434650a1196c7a9beaa14ada0R69-R84

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 17, 2023
@openshift-merge-robot
Copy link
Contributor

PR needs rebase.

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.

@cahartma
Copy link
Contributor

cahartma commented Jan 3, 2024

@oarribas if you will rebase your changes and force push, I will verify the changes and get this merged. We can then backport to 5.8 and update the Jira accordingly.

@cahartma
Copy link
Contributor

cahartma commented Jan 18, 2024

Going to close this and add these changes to the PR for LOG-4817

@cahartma
Copy link
Contributor

/close

@openshift-ci openshift-ci bot closed this Jan 18, 2024
Copy link
Contributor

openshift-ci bot commented Jan 18, 2024

@cahartma: Closed this PR.

In response to this:

/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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. release/5.9
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants