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

[ST] fixing unnecessary error logs when cleaning resources #9667

Merged
merged 2 commits into from Feb 13, 2024

Conversation

henryZrncik
Copy link
Contributor

@henryZrncik henryZrncik commented Feb 9, 2024

Type of change

  • Refactoring

Description

all changes made in order to avoid error logs (mostly null pointers) when removing Kafka resources and namespaces.

  • added one check to make sure kafka is present when cleaning Kafka resource. (solve problems with "Kafka" null pointers)
  • added default value for "testMethodName" when setting up cluster Operator namespaces and Logs Collecting, this is expected by the way we are creating them e.g. causing unexpected behaviour when trying to remove namespace.

additionally removal of several explicit kafka deletions in the end of the test as they are not necessary there (and are commonly no longer used as we have cleaning set up).

Signed-off-by: hzrncik <hzrncik@redhat.com>
@henryZrncik henryZrncik self-assigned this Feb 9, 2024
@henryZrncik henryZrncik added this to the 0.40.0 milestone Feb 9, 2024
@im-konge
Copy link
Member

im-konge commented Feb 9, 2024

/azp run regression

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@henryZrncik
Copy link
Contributor Author

the fail of the test in Azure has nothing to do with changes. The test is actually flaky and fails on both this and main branch as well (approx 40% of time) will be fixed in separe from this PR.

@see-quick
Copy link
Member

Well but for sure you should resolve problem with spotbugs i.e.,

[INFO] BugInstance size is 1
[INFO] Error size is 0
[INFO] Total bugs: 1
[ERROR] Medium: Do not catch NullPointerException like in io.strimzi.systemtest.resources.ResourceManager.deleteResource(HasMetadata[]) [io.strimzi.systemtest.resources.ResourceManager] At ResourceManager.java:[line 349] DCN_NULLPOINTER_EXCEPTION
[INFO] 

Copy link
Member

@im-konge im-konge left a comment

Choose a reason for hiding this comment

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

This is a cleanup stuff, but I saw similar NPE also in other "wait for methods", which are a bit misleading in the log. Do you want to look at those in different PR?
Anyway, this LGTM, just fix the spotbugs check

@henryZrncik
Copy link
Contributor Author

  1. (@see-quick) yeah, spotbugs problem should be removed
  2. (@im-konge) Also I will cover similar cases when waiting for resources in other PR as well.

@henryZrncik henryZrncik added ready for merge Label for PRs which are ready for merge and removed needs review labels Feb 13, 2024
@see-quick see-quick merged commit 3e7e30d into strimzi:main Feb 13, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for merge Label for PRs which are ready for merge System tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants