CORENET-6714: Enable Network Observability on Day 0 - #3087
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@OlivierCazade: This pull request references CORENET-6714 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 story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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. |
WalkthroughAdds a Network Observability controller, installer manifests, FlowCollector configuration, OLM readiness detection, server-side resource application, policy handling, status tracking, and comprehensive reconciliation tests. ChangesNetwork Observability
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant NetworkCR
participant ObservabilityController
participant OLM
participant KubernetesAPI
NetworkCR->>ObservabilityController: trigger reconciliation
ObservabilityController->>NetworkCR: read policy and feature-gate state
ObservabilityController->>OLM: check operator readiness
ObservabilityController->>KubernetesAPI: apply operator manifest
ObservabilityController->>KubernetesAPI: create namespace and FlowCollector
ObservabilityController->>NetworkCR: update deployment condition
Suggested labels: Suggested reviewers: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (6 errors, 1 warning)
✅ Passed checks (17 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
pkg/controller/observability/observability_controller.go (1)
45-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused observability polling constants
checkIntervalandcheckTimeoutare no longer referenced, so they can be removed to keep the package surface clean.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/controller/observability/observability_controller.go` around lines 45 - 46, Remove the unused checkInterval and checkTimeout constants from the observability controller package, leaving the remaining configuration declarations unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bindata/observability/07-observability-operator.yaml`:
- Around line 1-4: Add NetworkPolicy resources alongside the netobserv-operator
Namespace to explicitly isolate both ingress and egress. Follow the repository’s
established operator-namespace pattern: include default-deny policies and
narrowly scoped allow policies required by the operator, its workloads, and
necessary cluster communication.
In `@pkg/controller/observability/observability_controller_test.go`:
- Around line 1521-1550: Update the concurrent reconciliation test’s
ReconcileObservability setup to assign featureGate using
createEnabledFeatureGate(), and configure the fake client with
WithStatusSubresource(&operatorv1.Network{}) plus an operatorv1.Network
resource. Preserve the existing five-goroutine execution and conflict filtering
so the test exercises concurrent status updates rather than returning at the
feature-gate guard.
In `@pkg/controller/observability/observability_controller.go`:
- Around line 346-370: Update the CRD-present detection flow around the final
installation error to recognize when an OLMv1 ClusterExtension exists but is not
yet Installed as an in-progress state. Return the non-failure result used for
pending installation so Reconcile does not set DeploymentFailed, while
preserving the existing error for cases where neither OLMv1 nor OLMv0
installation objects exist.
- Around line 544-558: Update createFlowCollector when constructing the
Namespace for NetObservNamespace to include the standard OpenShift empty
scheduler node-selector annotation, while preserving the existing namespace
creation and error handling behavior.
---
Nitpick comments:
In `@pkg/controller/observability/observability_controller.go`:
- Around line 45-46: Remove the unused checkInterval and checkTimeout constants
from the observability controller package, leaving the remaining configuration
declarations unchanged.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a91f757-7d7a-4049-bf55-c95308c93b39
📒 Files selected for processing (6)
bindata/observability/07-observability-operator.yamlbindata/observability/08-flowcollector.yamlpkg/controller/add_networkconfig.gopkg/controller/observability/observability_controller.gopkg/controller/observability/observability_controller_test.gosample-cluster-network-config.yaml
| apiVersion: v1 | ||
| kind: Namespace | ||
| metadata: | ||
| name: netobserv-operator |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Add NetworkPolicy resources to isolate the netobserv-operator namespace.
This new operator namespace is introduced without any NetworkPolicy to explicitly isolate ingress and egress, unlike the established operator-namespace pattern in this repo. NetObserv runs highly privileged workloads, so locking down the namespace is important.
Add default-deny + scoped-allow ingress/egress NetworkPolicy objects for netobserv-operator following the existing operator-namespace pattern.
Based on learnings: whenever introducing an operator namespace in bindata/, ensure the namespace includes NetworkPolicy resources that explicitly isolate both ingress and egress traffic, following the existing operator-namespace NetworkPolicy pattern.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bindata/observability/07-observability-operator.yaml` around lines 1 - 4, Add
NetworkPolicy resources alongside the netobserv-operator Namespace to explicitly
isolate both ingress and egress. Follow the repository’s established
operator-namespace pattern: include default-deny policies and narrowly scoped
allow policies required by the operator, its workloads, and necessary cluster
communication.
Source: Learnings
|
|
||
| r := &ReconcileObservability{ | ||
| client: client, | ||
| } | ||
|
|
||
| req := reconcile.Request{NamespacedName: types.NamespacedName{Name: "cluster"}} | ||
|
|
||
| // Run 5 concurrent reconciliations | ||
| errChan := make(chan error, 5) | ||
| for i := 0; i < 5; i++ { | ||
| go func() { | ||
| _, err := r.Reconcile(context.TODO(), req) | ||
| errChan <- err | ||
| }() | ||
| } | ||
|
|
||
| // Wait for all to complete and collect errors | ||
| var unexpectedErrors []error | ||
| for i := 0; i < 5; i++ { | ||
| if err := <-errChan; err != nil { | ||
| // Filter out 409 conflict errors which are expected when multiple | ||
| // goroutines try to update the same resource status concurrently | ||
| if !errors.IsConflict(err) { | ||
| unexpectedErrors = append(unexpectedErrors, err) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Assert no unexpected errors occurred (safe to do in main test goroutine) | ||
| g.Expect(unexpectedErrors).To(BeEmpty(), "All concurrent reconciliations should complete without unexpected errors") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Concurrency test is vacuous — featureGate is nil, so Reconcile returns early.
r is built without a featureGate, so isFeatureGateEnabled() returns false and every goroutine exits Reconcile at the feature-gate guard before any install/status logic runs. The 5 goroutines never contend on shared state, and the errors.IsConflict filter (Line 1543) is dead code. This test does not actually validate concurrent reconciliation.
Set featureGate: createEnabledFeatureGate() (and add WithStatusSubresource(&operatorv1.Network{}) + an operatorv1.Network) so the reconcile actually reaches the concurrent status-update path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/controller/observability/observability_controller_test.go` around lines
1521 - 1550, Update the concurrent reconciliation test’s ReconcileObservability
setup to assign featureGate using createEnabledFeatureGate(), and configure the
fake client with WithStatusSubresource(&operatorv1.Network{}) plus an
operatorv1.Network resource. Preserve the existing five-goroutine execution and
conflict filtering so the test exercises concurrent status updates rather than
returning at the feature-gate guard.
| // If CRD doesn't exist but either OLM installation is present, this is an error condition | ||
| if !crdExists { | ||
| if olmv0Installed || olmv1Installed { | ||
| olmVersion := "OLMv0" | ||
| if olmv1Installed { | ||
| olmVersion = "OLMv1" | ||
| } | ||
| return false, olmv1CEExists, fmt.Errorf("network Observability Operator was deployed via %s but FlowCollector CRD is missing (manually removed)", olmVersion) | ||
| } | ||
| // If CRD doesn't exist and no OLM installation, operator is not installed | ||
| return false, olmv1CEExists, nil | ||
| } | ||
|
|
||
| if olmv1Installed { | ||
| klog.V(4).Info("Network Observability Operator installed via OLMv1 (ClusterExtension)") | ||
| return true, true, nil | ||
| } | ||
|
|
||
| if olmv0Installed { | ||
| klog.V(4).Info("Network Observability Operator installed via OLMv0 (ClusterServiceVersion)") | ||
| return true, false, nil | ||
| } | ||
|
|
||
| // CRD exists but neither OLMv0 nor OLMv1 shows a successful installation | ||
| return false, olmv1CEExists, fmt.Errorf("FlowCollector CRD is present but could not identify how Network Observability Operator was installed (neither OLMv1 ClusterExtension nor OLMv0 ClusterServiceVersion found)") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Transient install-in-progress can be reported as a hard failure.
During normal OLM v1 installation, the FlowCollector CRD is created by the bundle before the ClusterExtension reports Installed=True. In that window crdExists == true while olmv1Installed/olmv0Installed are false, so this falls through to the "could not identify how Network Observability Operator was installed" error (Line 370). Reconcile then sets the NetworkObservabilityDeployed condition to DeploymentFailed, which is misleading for a benign in-progress state. It self-heals on requeue, but the false-negative condition may trip alerting.
Consider treating "CRD present + ClusterExtension exists but not yet Installed" as in-progress (requeue without a failure condition), distinct from "CRD present + no OLM object at all".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/controller/observability/observability_controller.go` around lines 346 -
370, Update the CRD-present detection flow around the final installation error
to recognize when an OLMv1 ClusterExtension exists but is not yet Installed as
an in-progress state. Return the non-failure result used for pending
installation so Reconcile does not set DeploymentFailed, while preserving the
existing error for cases where neither OLMv1 nor OLMv0 installation objects
exist.
| func (r *ReconcileObservability) createFlowCollector(ctx context.Context) error { | ||
| // Ensure the netobserv namespace exists before applying manifests. | ||
| ns := &corev1.Namespace{} | ||
| if err := r.client.Get(ctx, types.NamespacedName{Name: NetObservNamespace}, ns); err != nil { | ||
| if errors.IsNotFound(err) { | ||
| if err := r.client.Create(ctx, &corev1.Namespace{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: NetObservNamespace}, | ||
| }); err != nil { | ||
| return fmt.Errorf("failed to create namespace %s: %w", NetObservNamespace, err) | ||
| } | ||
| klog.Infof("Created namespace %s", NetObservNamespace) | ||
| } else { | ||
| return err | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'observability_controller.go$' . || true
echo "== relevant code =="
if [ -f pkg/controller/observability/observability_controller.go ]; then
sed -n '1,90p' pkg/controller/observability/observability_controller.go
echo "---"
sed -n '330,390p' pkg/controller/observability/observability_controller.go
echo "---"
sed -n '520,590p' pkg/controller/observability/observability_controller.go
fi
echo "== search namespace annotation patterns =="
rg -n "openshift\.io/node-selector|NetObservNamespace|CreateFlowCollector|FlowCollector|defaultNodeSelector|NodeSelector" --glob '*.go' -S .
echo "== search manifests for netobserv namespace annotations =="
rg -n "namespace: *netobserv|netobserv|flows-collector|agent" manifests --glob '*.{yaml,yml,json}' -S || trueRepository: openshift/cluster-network-operator
Length of output: 50391
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== flowcollector manifest =="
sed -n '1,220p' bindata/observability/08-flowcollector.yaml
echo "== operator namespace annotation references across repository, excluding vendor =="
rg -n "netobserv-operator|netobserv\s*$|openshift\.io/node-selector|defaultNodeSelector|default node selector|NodeSelector" --glob '!vendor/**' -S .
echo "== netobserv-operator manifests, excluding vendor =="
fd -a 'netobserv-operator|07-observability-operator.yaml|08-flowcollector.yaml' . --exclude vendor | sed -n '1,80p'
sed -n '1,240p' bindata/observability/07-observability-operator.yaml
echo "== Kubernetes default/namespace node selector docs from vendor comments or API =="
rg -n "openshift.io/node-selector|default node selector|NodeSelector" vendor/k8s.io/client-go vendor/k8s.io/api/k8s.io/apimachinery vendor/k8s.io/kubectl --glob '*.go' -S | head -n 120 || trueRepository: openshift/cluster-network-operator
Length of output: 39863
🌐 Web query:
OpenShift defaultNodeSelector openshift.io/node-selector namespace annotation documentation
💡 Result:
In OpenShift, the openshift.io/node-selector namespace annotation is used to enforce project-specific node placement for pods [1][2]. When this annotation is applied to a Namespace, it directs the scheduler to place all pods created within that project onto nodes that match the specified label selectors [1][3][2]. Key behavioral details include: 1. Preference over Cluster-wide Defaults: The openshift.io/node-selector annotation takes precedence over the cluster-wide defaultNodeSelector defined in the Scheduler configuration object [4][5]. If a project has this annotation set, the cluster-wide default is ignored for pods in that project [6][2]. 2. Configuration: You can apply this by editing the Namespace object [1][2]. Example YAML: apiVersion: v1 kind: Namespace metadata: name: my-project annotations: openshift.io/node-selector: "type=user-node,region=east" 3. Disabling/Clearing: To disable the project-level node selector and revert to the cluster-wide default (or no selector), you can set the annotation value to an empty string [7]: oc patch namespace myproject -p '{"metadata": {"annotations": {"openshift.io/node-selector": ""}}}' The cluster-wide defaultNodeSelector (configured under the Scheduler resource) acts as a fallback or global constraint applied to pods in all namespaces that do not have their own specific node selector requirements or project-level annotations [4][8]. When both a cluster-wide default and a project-level annotation are present, the project-level annotation is used [4][6].
Citations:
- 1: https://docs.okd.io/latest/nodes/scheduling/nodes-scheduler-node-selectors.html
- 2: https://docs.okd.io/4.21/nodes/scheduling/nodes-scheduler-node-selectors.html
- 3: https://docs.okd.io/4.15/nodes/scheduling/nodes-scheduler-node-selectors.html
- 4: https://docs.okd.io/latest/rest_api/config_apis/scheduler-config-openshift-io-v1.html
- 5: https://docs.redhat.com/en/documentation/openshift_container_platform/4.10/html/nodes/controlling-pod-placement-onto-nodes-scheduling
- 6: https://docs.redhat.com/en/documentation/openshift_container_platform/4.20/html/config_apis/scheduler-config-openshift-io-v1
- 7: https://docs.redhat.com/en/documentation/openshift_container_platform/4.18/html/nodes/using-jobs-and-daemon-sets
- 8: https://docs.okd.io/4.9/nodes/scheduling/nodes-scheduler-default.html
Pre-created netobserv namespace must clear the default scheduler node selector.
FlowCollector.spec.agent.ebpf creates the eBPF agent DaemonSet into netobserv, but this path only creates the namespace with no annotations. In clusters with Scheduler.defaultNodeSelector, pods in netobserv inherit it and can be kept unschedulable across excluded nodes. Add the standard OpenShift empty annotation when creating the namespace:
🛠️ Proposed fix
if err := r.client.Create(ctx, &corev1.Namespace{
- ObjectMeta: metav1.ObjectMeta{Name: NetObservNamespace},
+ ObjectMeta: metav1.ObjectMeta{
+ Name: NetObservNamespace,
+ Annotations: map[string]string{"openshift.io/node-selector": ""},
+ },
}); err != nil {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (r *ReconcileObservability) createFlowCollector(ctx context.Context) error { | |
| // Ensure the netobserv namespace exists before applying manifests. | |
| ns := &corev1.Namespace{} | |
| if err := r.client.Get(ctx, types.NamespacedName{Name: NetObservNamespace}, ns); err != nil { | |
| if errors.IsNotFound(err) { | |
| if err := r.client.Create(ctx, &corev1.Namespace{ | |
| ObjectMeta: metav1.ObjectMeta{Name: NetObservNamespace}, | |
| }); err != nil { | |
| return fmt.Errorf("failed to create namespace %s: %w", NetObservNamespace, err) | |
| } | |
| klog.Infof("Created namespace %s", NetObservNamespace) | |
| } else { | |
| return err | |
| } | |
| } | |
| func (r *ReconcileObservability) createFlowCollector(ctx context.Context) error { | |
| // Ensure the netobserv namespace exists before applying manifests. | |
| ns := &corev1.Namespace{} | |
| if err := r.client.Get(ctx, types.NamespacedName{Name: NetObservNamespace}, ns); err != nil { | |
| if errors.IsNotFound(err) { | |
| if err := r.client.Create(ctx, &corev1.Namespace{ | |
| ObjectMeta: metav1.ObjectMeta{ | |
| Name: NetObservNamespace, | |
| Annotations: map[string]string{"openshift.io/node-selector": ""}, | |
| }, | |
| }); err != nil { | |
| return fmt.Errorf("failed to create namespace %s: %w", NetObservNamespace, err) | |
| } | |
| klog.Infof("Created namespace %s", NetObservNamespace) | |
| } else { | |
| return err | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/controller/observability/observability_controller.go` around lines 544 -
558, Update createFlowCollector when constructing the Namespace for
NetObservNamespace to include the standard OpenShift empty scheduler
node-selector annotation, while preserving the existing namespace creation and
error handling behavior.
Source: Path instructions
|
/retest |
|
/pipeline required |
|
Scheduling required tests: Scheduling tests matching the |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bindata/observability/07-observability-operator.yaml (1)
240-253: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winPin the operator version in the
ClusterExtensionfor upgrade safety.
spec.source.catalogsetspackageNameandchannels: [stable]but noversion, so OLM installs the latest available bundle instable. For a CNO-driven install, add an explicitversionconstraint underspec.source.catalog(for example a specific SemVer or a tested range) to keep deployments reproducible and avoid unexpected operator upgrades.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bindata/observability/07-observability-operator.yaml` around lines 240 - 253, Add an explicit tested version constraint under the ClusterExtension spec.source.catalog for netobserv-operator, alongside packageName and channels, so CNO installs a reproducible operator version while preserving the existing stable channel selection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@bindata/observability/07-observability-operator.yaml`:
- Around line 240-253: Add an explicit tested version constraint under the
ClusterExtension spec.source.catalog for netobserv-operator, alongside
packageName and channels, so CNO installs a reproducible operator version while
preserving the existing stable channel selection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f741204-d73c-460f-a6bc-f739bc9ccd2d
📒 Files selected for processing (6)
bindata/observability/07-observability-operator.yamlbindata/observability/08-flowcollector.yamlpkg/controller/add_networkconfig.gopkg/controller/observability/observability_controller.gopkg/controller/observability/observability_controller_test.gosample-cluster-network-config.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
- bindata/observability/08-flowcollector.yaml
- sample-cluster-network-config.yaml
- pkg/controller/add_networkconfig.go
- pkg/controller/observability/observability_controller.go
- pkg/controller/observability/observability_controller_test.go
|
/pipeline required |
|
Scheduling required tests: Scheduling tests matching the |
This reverts commit dfb8dd5.
|
Last push is to rebase the branch |
|
/pipeline required |
|
Scheduling required tests: Scheduling tests matching the |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
bindata/observability/07-observability-operator.yaml (2)
38-44: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueCluster-wide
createonvalidatingwebhookconfigurationsis a privilege-escalation vector, but appears required by OLMv1 pre-auth.The mutating verbs are correctly narrowed via
resourceNameson Line 43; onlycreate/list/watchremain cluster-wide, which RBAC cannot scope by name. Worth an inline note documenting this as an OLMv1 pre-authorization requirement so future readers don't try to tighten it and break installation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bindata/observability/07-observability-operator.yaml` around lines 38 - 44, Document the OLMv1 pre-authorization requirement beside the cluster-wide validatingwebhookconfigurations rule in the observability operator manifest, noting that create/list/watch cannot be narrowed by resourceNames and must remain cluster-wide for installation. Leave the resourceNames-scoped mutating rule unchanged.Source: Linters/SAST tools
240-252: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winBounded the NetObserv channel install with a version requirement.
channels: [stable]allows the catalog’s current stable bundle on Day-0 installs and automated channel upgrades without an explicitversionconstraint. Add a bounded semantic range, e.g.version: ">=1.9.0 <2.0.0", to avoid non-deterministic installation and unbounded upgrades;upgradeConstraintPolicyis optional withCatalogProvidedas the default.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bindata/observability/07-observability-operator.yaml` around lines 240 - 252, Add a semantic version constraint to the netobserv-operator catalog configuration under the ClusterExtension source, alongside channels, limiting installation and upgrades to the 1.x range starting at 1.9.0 (for example, >=1.9.0 <2.0.0). Leave the existing stable channel and default upgrade policy unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@bindata/observability/07-observability-operator.yaml`:
- Around line 38-44: Document the OLMv1 pre-authorization requirement beside the
cluster-wide validatingwebhookconfigurations rule in the observability operator
manifest, noting that create/list/watch cannot be narrowed by resourceNames and
must remain cluster-wide for installation. Leave the resourceNames-scoped
mutating rule unchanged.
- Around line 240-252: Add a semantic version constraint to the
netobserv-operator catalog configuration under the ClusterExtension source,
alongside channels, limiting installation and upgrades to the 1.x range starting
at 1.9.0 (for example, >=1.9.0 <2.0.0). Leave the existing stable channel and
default upgrade policy unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 24cc7576-d4e1-48da-ab96-d2624192fd02
📒 Files selected for processing (6)
bindata/observability/07-observability-operator.yamlbindata/observability/08-flowcollector.yamlpkg/controller/add_networkconfig.gopkg/controller/observability/observability_controller.gopkg/controller/observability/observability_controller_test.gosample-cluster-network-config.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
- sample-cluster-network-config.yaml
- bindata/observability/08-flowcollector.yaml
- pkg/controller/add_networkconfig.go
- pkg/controller/observability/observability_controller_test.go
- pkg/controller/observability/observability_controller.go
|
/lgtm |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: arkadeepsen, OlivierCazade The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/verified by @kapjain-rh |
openshift-ci-robot
added
the
verified
|
@kapjain-rh: This PR has been marked as verified by DetailsIn response to this:
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. |
|
/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview |
|
@arkadeepsen: trigger 4 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/1e8d0a90-89e2-11f1-950f-8a2b16b5d434-0 |
|
/pipeline required |
|
Scheduling required tests: Scheduling tests matching the |
|
/pipeline required |
|
Scheduling required tests: Scheduling tests matching the |
|
@OlivierCazade: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
/hold The techpreview serial lanes are still failing with the same issue: #3087 (comment) @OlivierCazade PTAL |
openshift-ci
Bot
added
the
do-not-merge/hold
4 participants
Reverting the revert and adding TLS to the metric endpoints.
#2925
This fix a failed techpreview test that was failing because some of network observability pods were exposing the metric endpoint through HTTP and not HTTPS.
The change is done in the the bindata/observability/08-flowcollector.yaml file.