Skip to content

NETOBSERV-2376 fix statuses#2677

Merged
openshift-merge-bot[bot] merged 1 commit intonetobserv:mainfrom
jpinsonneau:2376-followup
Apr 23, 2026
Merged

NETOBSERV-2376 fix statuses#2677
openshift-merge-bot[bot] merged 1 commit intonetobserv:mainfrom
jpinsonneau:2376-followup

Conversation

@jpinsonneau
Copy link
Copy Markdown
Member

@jpinsonneau jpinsonneau commented Apr 23, 2026

Description

  • onHold component statuses forced to unused
  • kafka condition removed when disabling it
image

Also see netobserv/netobserv-web-console#1445 for colors and icons in plugin

Dependencies

n/a

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

  • Is this PR backed with a JIRA ticket? If so, make sure it is written as a title prefix (in general, PRs affecting the NetObserv/Network Observability product should be backed with a JIRA ticket - especially if they bring user facing changes).
  • Does this PR require product documentation?
    • If so, make sure the JIRA epic is labeled with "documentation" and provides a description relevant for doc writers, such as use cases or scenarios. Any required step to activate or configure the feature should be documented there, such as new CRD knobs.
  • Does this PR require a product release notes entry?
    • If so, fill in "Release Note Text" in the JIRA.
  • Is there anything else the QE team should know before testing? E.g: configuration changes, environment setup, etc.
    • If so, make sure it is described in the JIRA ticket.
  • QE requirements (check 1 from the list):
    • Standard QE validation, with pre-merge tests unless stated otherwise.
    • Regression tests only (e.g. refactoring with no user-facing change).
    • No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

Summary by CodeRabbit

  • Bug Fixes
    • Improved status reporting for on-hold components; they now correctly display as "unused" instead of "ready."
    • Removed stale Kafka-related status conditions when Kafka is not in use, preventing outdated status information.
    • Enhanced state preservation during concurrent operations to maintain accurate agent and plugin status tracking.

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Apr 23, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 23, 2026

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title references JIRA ticket and describes main changes (status fixes) clearly and concisely.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed PR description addresses the main changes (onHold statuses, Kafka condition removal) but lacks detail on implementation and testing specifics required by the template.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jpinsonneau jpinsonneau requested a review from kapjain-rh April 23, 2026 08:18
@jpinsonneau jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Apr 23, 2026
@github-actions
Copy link
Copy Markdown

New images:

quay.io/netobserv/network-observability-operator:e2c6794
quay.io/netobserv/network-observability-operator-bundle:v0.0.0-sha-e2c6794
quay.io/netobserv/network-observability-operator-catalog:v0.0.0-sha-e2c6794

They will expire in two weeks.

To deploy this build:

# Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:e2c6794 make deploy

# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-sha-e2c6794

Or as a Catalog Source:

apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
  name: netobserv-dev
  namespace: openshift-marketplace
spec:
  sourceType: grpc
  image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-sha-e2c6794
  displayName: NetObserv development catalog
  publisher: Me
  updateStrategy:
    registryPoll:
      interval: 1m

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

❌ Patch coverage is 90.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.83%. Comparing base (88ab9fe) to head (9e7d744).

Files with missing lines Patch % Lines
internal/pkg/manager/status/status_manager.go 88.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2677      +/-   ##
==========================================
+ Coverage   70.60%   70.83%   +0.22%     
==========================================
  Files         107      107              
  Lines       13619    13646      +27     
==========================================
+ Hits         9616     9666      +50     
+ Misses       3502     3478      -24     
- Partials      501      502       +1     
Flag Coverage Δ
unittests 70.83% <90.00%> (+0.22%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
internal/controller/flp/flp_controller.go 73.07% <100.00%> (+0.60%) ⬆️
internal/pkg/manager/status/status_manager.go 93.80% <88.00%> (-0.39%) ⬇️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/pkg/manager/status/status_manager.go`:
- Around line 313-318: GetKafkaCondition() can return nil for a placeholder
"Unknown" state, so do not unconditionally call
meta.RemoveStatusCondition(&fc.Status.Conditions, "KafkaReady") when it returns
nil; instead, change the logic to only remove the "KafkaReady" condition when
Kafka is actually disabled/not configured. Implement this by either: (a)
modifying GetKafkaCondition to return a distinct signal (or error) when Kafka is
truly disabled vs when it's a temporary placeholder, or (b) add a helper like
IsKafkaDisabled/IsKafkaInUse and check that before calling
meta.RemoveStatusCondition; update the three sites that currently do the
unconditional remove (the blocks around GetKafkaCondition, including the
occurrences near ForComponent(FLPTransformer)) to use this new check so
KafkaReady is only removed when Kafka is definitively not in use.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c70ceeaf-dd27-4637-a1c2-d6d81e371416

📥 Commits

Reviewing files that changed from the base of the PR and between 88ab9fe and 476174e.

📒 Files selected for processing (3)
  • internal/controller/flp/flp_controller.go
  • internal/pkg/manager/status/status_manager.go
  • internal/pkg/manager/status/status_manager_test.go

Comment thread internal/pkg/manager/status/status_manager.go
@github-actions github-actions Bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Apr 23, 2026
@jpinsonneau jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Apr 23, 2026
@github-actions
Copy link
Copy Markdown

New images:

quay.io/netobserv/network-observability-operator:486fdeb
quay.io/netobserv/network-observability-operator-bundle:v0.0.0-sha-486fdeb
quay.io/netobserv/network-observability-operator-catalog:v0.0.0-sha-486fdeb

They will expire in two weeks.

To deploy this build:

# Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:486fdeb make deploy

# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-sha-486fdeb

Or as a Catalog Source:

apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
  name: netobserv-dev
  namespace: openshift-marketplace
spec:
  sourceType: grpc
  image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-sha-486fdeb
  displayName: NetObserv development catalog
  publisher: Me
  updateStrategy:
    registryPoll:
      interval: 1m

@github-actions github-actions Bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Apr 23, 2026
@jpinsonneau jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Apr 23, 2026
@jpinsonneau jpinsonneau marked this pull request as ready for review April 23, 2026 08:47
@github-actions
Copy link
Copy Markdown

New images:

quay.io/netobserv/network-observability-operator:41433e6
quay.io/netobserv/network-observability-operator-bundle:v0.0.0-sha-41433e6
quay.io/netobserv/network-observability-operator-catalog:v0.0.0-sha-41433e6

They will expire in two weeks.

To deploy this build:

# Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:41433e6 make deploy

# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-sha-41433e6

Or as a Catalog Source:

apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
  name: netobserv-dev
  namespace: openshift-marketplace
spec:
  sourceType: grpc
  image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-sha-41433e6
  displayName: NetObserv development catalog
  publisher: Me
  updateStrategy:
    registryPoll:
      interval: 1m

@jpinsonneau
Copy link
Copy Markdown
Member Author

/test e2e-operator

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Apr 23, 2026

@jpinsonneau: 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-operator 9e7d744 link false /test e2e-operator

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-sigs/prow repository. I understand the commands that are listed here.

@kapjain-rh
Copy link
Copy Markdown
Member

/label qe-approved

@openshift-ci openshift-ci Bot added the qe-approved QE has approved this pull request label Apr 23, 2026
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Apr 23, 2026

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

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

@jpinsonneau
Copy link
Copy Markdown
Member Author

/test e2e-operator

@openshift-merge-bot openshift-merge-bot Bot merged commit 62eed57 into netobserv:main Apr 23, 2026
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. qe-approved QE has approved this pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants