Skip to content

NETOBSERV-2174: check if LokiStack exists#1257

Merged
jotak merged 2 commits intonetobserv:mainfrom
jotak:check-lokistack
Mar 20, 2025
Merged

NETOBSERV-2174: check if LokiStack exists#1257
jotak merged 2 commits intonetobserv:mainfrom
jotak:check-lokistack

Conversation

@jotak
Copy link
Member

@jotak jotak commented Mar 17, 2025

Description

If configured LokiStack doesn't exist, it will be reported as a FlowCollector status condition.

Capture d’écran du 2025-03-17 14-28-37

Dependencies

PR is based on #1255 and #1258

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

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Mar 17, 2025

@jotak: This pull request references NETOBSERV-2174 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.19.0" version, but no target version was set.

Details

In response to this:

Description

If configured LokiStack doesn't exist, it will be reported as a FlowCollector status condition.

Dependencies

PR is based on #1255

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

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
Copy link

openshift-ci bot commented Mar 17, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@jotak jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Mar 17, 2025
@github-actions
Copy link

New images:

  • quay.io/netobserv/network-observability-operator:c693e20
  • quay.io/netobserv/network-observability-operator-bundle:v0.0.0-c693e20
  • quay.io/netobserv/network-observability-operator-catalog:v0.0.0-c693e20

They will expire after two weeks.

To deploy this build:

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

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

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-c693e20
  displayName: NetObserv development catalog
  publisher: Me
  updateStrategy:
    registryPoll:
      interval: 1m

@jotak jotak force-pushed the check-lokistack branch from 2f93554 to 3218953 Compare March 17, 2025 13:26
@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 Mar 17, 2025
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Mar 17, 2025

@jotak: This pull request references NETOBSERV-2174 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.19.0" version, but no target version was set.

Details

In response to this:

Description

If configured LokiStack doesn't exist, it will be reported as a FlowCollector status condition.

Dependencies

PR is based on #1255 and #1258

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

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
Copy link
Collaborator

openshift-ci-robot commented Mar 17, 2025

@jotak: This pull request references NETOBSERV-2174 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.19.0" version, but no target version was set.

Details

In response to this:

Description

If configured LokiStack doesn't exist, it will be reported as a FlowCollector status condition.

Capture d’écran du 2025-03-17 14-28-37

Dependencies

PR is based on #1255 and #1258

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

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.

@Amoghrd
Copy link
Member

Amoghrd commented Mar 17, 2025

/ok-to-test

@openshift-ci openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Mar 17, 2025
@github-actions
Copy link

New images:

  • quay.io/netobserv/network-observability-operator:6c5ceff
  • quay.io/netobserv/network-observability-operator-bundle:v0.0.0-6c5ceff
  • quay.io/netobserv/network-observability-operator-catalog:v0.0.0-6c5ceff

They will expire after two weeks.

To deploy this build:

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

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

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-6c5ceff
  displayName: NetObserv development catalog
  publisher: Me
  updateStrategy:
    registryPoll:
      interval: 1m

@jotak jotak force-pushed the check-lokistack branch from 3218953 to cc092d7 Compare March 19, 2025 09:56
@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 Mar 19, 2025
@jotak jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Mar 19, 2025
@github-actions
Copy link

New images:

  • quay.io/netobserv/network-observability-operator:84bcb28
  • quay.io/netobserv/network-observability-operator-bundle:v0.0.0-84bcb28
  • quay.io/netobserv/network-observability-operator-catalog:v0.0.0-84bcb28

They will expire after two weeks.

To deploy this build:

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

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

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-84bcb28
  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 Mar 19, 2025
@jotak jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Mar 19, 2025
@github-actions
Copy link

New images:

  • quay.io/netobserv/network-observability-operator:2993123
  • quay.io/netobserv/network-observability-operator-bundle:v0.0.0-2993123
  • quay.io/netobserv/network-observability-operator-catalog:v0.0.0-2993123

They will expire after two weeks.

To deploy this build:

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

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

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-2993123
  displayName: NetObserv development catalog
  publisher: Me
  updateStrategy:
    registryPoll:
      interval: 1m

@codecov
Copy link

codecov bot commented Mar 19, 2025

Codecov Report

Attention: Patch coverage is 54.16667% with 22 lines in your changes missing coverage. Please review.

Project coverage is 62.65%. Comparing base (887751b) to head (388cd77).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
pkg/manager/status/status_manager.go 52.27% 19 Missing and 2 partials ⚠️
main.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1257      +/-   ##
==========================================
- Coverage   62.70%   62.65%   -0.06%     
==========================================
  Files          76       76              
  Lines       11551    11599      +48     
==========================================
+ Hits         7243     7267      +24     
- Misses       3845     3868      +23     
- Partials      463      464       +1     
Flag Coverage Δ
unittests 62.65% <54.16%> (-0.06%) ⬇️

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

Files with missing lines Coverage Δ
pkg/manager/manager.go 58.18% <ø> (ø)
pkg/test/envtest.go 98.11% <100.00%> (+0.03%) ⬆️
main.go 0.00% <0.00%> (ø)
pkg/manager/status/status_manager.go 77.04% <52.27%> (-7.17%) ⬇️

... and 3 files with indirect coverage changes

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

@Amoghrd
Copy link
Member

Amoghrd commented Mar 19, 2025

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved QE has approved this pull request label Mar 19, 2025
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Mar 19, 2025

@jotak: This pull request references NETOBSERV-2174 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.19.0" version, but no target version was set.

Details

In response to this:

Description

If configured LokiStack doesn't exist, it will be reported as a FlowCollector status condition.

Capture d’écran du 2025-03-17 14-28-37

Dependencies

PR is based on #1255 and #1258

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

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.

@jotak jotak merged commit 9410e55 into netobserv:main Mar 20, 2025
15 of 17 checks passed
@jotak jotak deleted the check-lokistack branch March 20, 2025 07:55
Comment on lines +182 to +197
if !helper.UseLoki(&fc.Spec) {
return metav1.Condition{
Type: LokiIssue,
Reason: "Unused",
Status: metav1.ConditionUnknown,
Message: "Loki is disabled",
}
}
if fc.Spec.Loki.Mode != flowslatest.LokiModeLokiStack {
return metav1.Condition{
Type: LokiIssue,
Reason: "Unused",
Status: metav1.ConditionUnknown,
Message: "Loki is not configured in LokiStack mode",
}
}
Copy link
Member

Choose a reason for hiding this comment

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

How does that reports in the conditions ? 🤔

LokiIssue - Unused - Unknown - Loki is disabled
LokiIssue - Unused - Unknown - Loki is not configured in LokiStack

Maybe we could rename LokiIssue to Loki to report all the states as these are not issues.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think because what matters here is that it's a condition that needs to be either TRUE or FALSE (or UNKNOWN). Just "Loki" being true or false doesn't make sense, whereas having LokiIssue true or false does make sense.
When the condition is true, users see in the Console "LokiIssue" so they know there's something to check. If it was just "Loki", that would be unclear. When the condition is false, LokiIssue isn't displayed in the condensed status view, so that's fine

Copy link
Member Author

Choose a reason for hiding this comment

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

See my previous PR #1255 that was precisely about making conditions being "negative" so that they render better in the console

Copy link
Member

Choose a reason for hiding this comment

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

Ok make sense !

Comment on lines +220 to +224
return metav1.Condition{
Type: LokiIssue,
Reason: "NoIssue",
Status: metav1.ConditionFalse,
}
Copy link
Member

Choose a reason for hiding this comment

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

Why not going one step further here checking LokiStack status and mode ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference 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.

4 participants