CORS-4308: Enable Network Observability during installation#10382
CORS-4308: Enable Network Observability during installation#10382OlivierCazade wants to merge 1 commit intoopenshift:mainfrom
Conversation
|
@OlivierCazade: This pull request references CORS-4308 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 "4.22.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. |
openshift-ci-robot
added
the
jira/valid-reference
WalkthroughAdd optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.3)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions 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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
@OlivierCazade: This pull request references CORS-4308 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 "4.22.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. |
![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/types/defaults/installconfig_test.go (1)
290-315: Add the explicit empty-string case too.
""is part of the allowed enum, but the new table only exercisesniland"Disable". A case withInstallNetworkObservability: ptr("")would lock down whether defaults should preserve the empty string or normalize it to"Enable".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/types/defaults/installconfig_test.go` around lines 290 - 315, Add a third table-driven test case exercising the explicit empty-string enum for Networking.InstallNetworkObservability: create a new entry in the test slice with name like "InstallNetworkObservability empty string", set config: &types.InstallConfig{Networking: &types.Networking{InstallNetworkObservability: func() *string { s := ""; return &s }()}}, and set expected to the result you expect (either keep empty string or normalize to "Enable") by cloning defaultInstallConfig() and adjusting c.Networking.InstallNetworkObservability accordingly; this ensures the behavior of InstallNetworkObservability for "" is locked down in the tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@data/data/install.openshift.io_installconfigs.yaml`:
- Around line 4799-4802: The enum currently allows an empty-string value ("")
alongside "Enable" and "Disable" but the schema description only documents
omitted, "Enable" and "Disable"; update the enum to remove the blank entry or
explicitly document the blank as equivalent to the intended behavior (e.g.,
treat "" as omitted or as "Enable") so there's no ambiguity—modify the enum list
(remove the line with - "") or add a sentence to the field description
clarifying how "" is interpreted.
- Around line 4794-4803: Add an explicit default for the
installNetworkObservability property in the CRD schema so schema-driven tooling
matches runtime behavior: update the installNetworkObservability property (the
block with description, enum, and type) to include a default: "Enable" entry
(ensuring the string value matches the enum entries), e.g., add default: Enable
under the same indentation as enum/type; keep the existing enum and type
unchanged.
In `@go.mod`:
- Line 393: Remove the temporary replace directive that points openshift/api to
the personal fork: delete the line "replace github.com/openshift/api
v0.0.0-20260228183123-9b2ee997d297 => github.com/OlivierCazade/api
v0.0.0-20260310172058-df85794acda8" from go.mod and, if needed, run `go get` or
`go mod tidy` to restore a proper official github.com/openshift/api revision so
all imports of github.com/openshift/api (e.g., config/v1, machine/v1,
machine/v1beta1, machineconfiguration/v1, operator/v1) resolve to the upstream
module rather than the fork.
In `@pkg/types/installconfig.go`:
- Around line 429-435: In validateNetworking(), add an explicit validation for
the InstallNetworkObservability field to reject any value other than the allowed
set ("", "Enable", "Disable"); locate the InstallNetworkObservability pointer on
the InstallConfig type and if non-nil check its value and append a field error
(using the same error accumulation approach used in validateNetworking) for the
json field "installNetworkObservability" when it is not one of the three allowed
strings so invalid values are rejected before manifest generation.
---
Nitpick comments:
In `@pkg/types/defaults/installconfig_test.go`:
- Around line 290-315: Add a third table-driven test case exercising the
explicit empty-string enum for Networking.InstallNetworkObservability: create a
new entry in the test slice with name like "InstallNetworkObservability empty
string", set config: &types.InstallConfig{Networking:
&types.Networking{InstallNetworkObservability: func() *string { s := ""; return
&s }()}}, and set expected to the result you expect (either keep empty string or
normalize to "Enable") by cloning defaultInstallConfig() and adjusting
c.Networking.InstallNetworkObservability accordingly; this ensures the behavior
of InstallNetworkObservability for "" is locked down in the tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 87623f3e-7184-4fb6-99f7-4eb911b3e0bb
⛔ Files ignored due to path filters (6)
go.sumis excluded by!**/*.sumpkg/types/zz_generated.deepcopy.gois excluded by!**/zz_generated*vendor/github.com/openshift/api/config/v1/types_network.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (6)
data/data/install.openshift.io_installconfigs.yamlgo.modpkg/asset/manifests/network.gopkg/types/defaults/installconfig.gopkg/types/defaults/installconfig_test.gopkg/types/installconfig.go
| installNetworkObservability: | ||
| description: |- | ||
| InstallNetworkObservability is an optional field that enables network observability | ||
| when omitted or set to "Enable". If the field is set to "Disable", it does nothing. | ||
| Valid values are "", "Enable", "Disable". | ||
| enum: | ||
| - "" | ||
| - Enable | ||
| - Disable | ||
| type: string |
There was a problem hiding this comment.
Declare the default in the schema.
The PR defaults this field to "Enable" when it is omitted, but the CRD schema does not advertise that with a default: entry. That leaves schema-driven tooling and CRD defaulting out of sync with the documented behavior.
Suggested fix
installNetworkObservability:
description: |-
InstallNetworkObservability is an optional field that enables network observability
when omitted or set to "Enable". If the field is set to "Disable", it does nothing.
Valid values are "", "Enable", "Disable".
+ default: Enable
enum:
- ""
- Enable
- Disable
type: string📝 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.
| installNetworkObservability: | |
| description: |- | |
| InstallNetworkObservability is an optional field that enables network observability | |
| when omitted or set to "Enable". If the field is set to "Disable", it does nothing. | |
| Valid values are "", "Enable", "Disable". | |
| enum: | |
| - "" | |
| - Enable | |
| - Disable | |
| type: string | |
| installNetworkObservability: | |
| description: |- | |
| InstallNetworkObservability is an optional field that enables network observability | |
| when omitted or set to "Enable". If the field is set to "Disable", it does nothing. | |
| Valid values are "", "Enable", "Disable". | |
| default: Enable | |
| enum: | |
| - "" | |
| - Enable | |
| - Disable | |
| type: string |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@data/data/install.openshift.io_installconfigs.yaml` around lines 4794 - 4803,
Add an explicit default for the installNetworkObservability property in the CRD
schema so schema-driven tooling matches runtime behavior: update the
installNetworkObservability property (the block with description, enum, and
type) to include a default: "Enable" entry (ensuring the string value matches
the enum entries), e.g., add default: Enable under the same indentation as
enum/type; keep the existing enum and type unchanged.
go.mod
Outdated
| sigs.k8s.io/cluster-api-provider-azure => github.com/mboersma/cluster-api-provider-azure v0.3.1-0.20251030205607-3161b9cc8d3e | ||
| ) | ||
|
|
||
| replace github.com/openshift/api v0.0.0-20260228183123-9b2ee997d297 => github.com/OlivierCazade/api v0.0.0-20260310172058-df85794acda8 No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== replace directive =="
sed -n '390,394p' go.mod
echo
echo "== repo imports that still resolve through github.com/openshift/api =="
rg -n -C1 '"github\.com/openshift/api/' --type=go -g '!vendor/**'Repository: openshift/installer
Length of output: 50375
Do not merge with the personal-fork openshift/api replace still in place.
This makes the branch depend on github.com/OlivierCazade/api instead of an official github.com/openshift/api revision. The codebase imports from github.com/openshift/api across numerous packages (config/v1, machine/v1, machine/v1beta1, machineconfiguration/v1, features, operator/v1, and more), all of which would be satisfied by the temporary fork if merged. This introduces a supply-chain and reproducibility risk to main and must be reverted before merge.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` at line 393, Remove the temporary replace directive that points
openshift/api to the personal fork: delete the line "replace
github.com/openshift/api v0.0.0-20260228183123-9b2ee997d297 =>
github.com/OlivierCazade/api v0.0.0-20260310172058-df85794acda8" from go.mod
and, if needed, run `go get` or `go mod tidy` to restore a proper official
github.com/openshift/api revision so all imports of github.com/openshift/api
(e.g., config/v1, machine/v1, machine/v1beta1, machineconfiguration/v1,
operator/v1) resolve to the upstream module rather than the fork.
pkg/types/installconfig.go
Outdated
| // InstallNetworkObservability is an optional field that enables network observability | ||
| // when omitted or set to "Enable". If the field is set to "Disable", it does nothing. | ||
| // Valid values are "", "Enable", "Disable". | ||
| // | ||
| // +kubebuilder:validation:Enum="";Enable;Disable | ||
| // +optional | ||
| InstallNetworkObservability *string `json:"installNetworkObservability,omitempty"` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== validate install-config entry points =="
fd 'installconfig\.go$' pkg/types/validation -x sed -n '120,220p' {}
echo
echo "== references to the new field in validation code =="
rg -n -C3 'InstallNetworkObservability|validateNetworking\s*\(' pkg/types/validationRepository: openshift/installer
Length of output: 6902
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the validateNetworking function starting from line 505
echo "=== validateNetworking function (full implementation) ==="
sed -n '505,700p' pkg/types/validation/installconfig.go
# Also check if InstallNetworkObservability is mentioned anywhere in the validation files
echo ""
echo "=== Search for InstallNetworkObservability in validation ==="
rg -n 'InstallNetworkObservability' pkg/types/validation/Repository: openshift/installer
Length of output: 9479
Add enum validation for InstallNetworkObservability in the validation logic.
The validateNetworking() function in pkg/types/validation/installconfig.go does not validate the InstallNetworkObservability field. The kubebuilder tag documents the enum but does not enforce it at the installer level. Add an explicit check to reject values outside "", "Enable", and "Disable" to prevent invalid values from reaching manifest generation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/types/installconfig.go` around lines 429 - 435, In validateNetworking(),
add an explicit validation for the InstallNetworkObservability field to reject
any value other than the allowed set ("", "Enable", "Disable"); locate the
InstallNetworkObservability pointer on the InstallConfig type and if non-nil
check its value and append a field error (using the same error accumulation
approach used in validateNetworking) for the json field
"installNetworkObservability" when it is not one of the three allowed strings so
invalid values are rejected before manifest generation.
barbacbd
left a comment
barbacbd
left a comment
There was a problem hiding this comment.
- you should squash or remove multiple commits here. Your first three commits are all overwritting/renaming the same variable.
- We generally only add to the install-config when absolutely necessary so I will take a closer look to see if this is that case.
- If this is a new feature, we default to previous settings. In this case I believe that the default should be disabled (unless that is going to break something or that is not the equivalent to a previous setting).
- It does happen but very rare for us to merge something with a local version of a dependency. If you wish to merge this, the api change will probably need to be applied first then we can change the reference in your final commit.
openshift-ci
bot
added
the
needs-rebase
openshift-ci
bot
removed
the
needs-rebase
|
@OlivierCazade: This pull request references CORS-4308 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 "4.22.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. |
|
@barbacbd thanks for your feedback.
|
There was a problem hiding this comment.
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 `@data/data/install.openshift.io_installconfigs.yaml`:
- Around line 5009-5022: The schema allows an explicit empty
networkObservability object to bypass defaults and manifest emission; either
make installationPolicy required in the networkObservability schema (so
installationPolicy must be present) or change the defaulting logic so that when
networkObservability is present but has no installationPolicy (i.e.,
networkObservability: {}), the defaulting routine that sets installationPolicy
(see installconfig defaulting code) populates a value; after implementing the
schema or defaulting change, add a regression test that validates behavior for
networkObservability: {} to ensure installationPolicy is set and manifests are
emitted as expected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 73a72032-3040-4194-8303-28237406c2b4
⛔ Files ignored due to path filters (6)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/config/v1/types_network.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (9)
data/data/install.openshift.io_installconfigs.yamlgo.modpkg/asset/agent/installconfig_test.gopkg/asset/installconfig/installconfig_test.gopkg/asset/manifests/network.gopkg/types/defaults/installconfig.gopkg/types/defaults/installconfig_test.gopkg/types/installconfig.gopkg/types/zz_generated.deepcopy.go
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/types/defaults/installconfig.go
- pkg/asset/manifests/network.go
- go.mod
- pkg/types/installconfig.go
| properties: | ||
| installationPolicy: | ||
| description: |- | ||
| InstallationPolicy controls whether network observability is installed during cluster deployment. | ||
| Valid values are "", "InstallAndEnable" and "DoNotInstall". | ||
| When set to "", network observability will be installed unless this is a SNO cluster. | ||
| When set to "InstallAndEnable", network observability will be installed and enabled. | ||
| When set to "DoNotInstall", network observability will not be installed. | ||
| enum: | ||
| - "" | ||
| - InstallAndEnable | ||
| - DoNotInstall | ||
| type: string | ||
| type: object |
There was a problem hiding this comment.
Handle explicit networkObservability: {} deterministically.
installationPolicy is optional here, but installer defaulting only runs when the whole networkObservability object is nil (pkg/types/defaults/installconfig.go:59-66), and manifest emission only runs when installationPolicy is non-nil (pkg/asset/manifests/network.go:83-88). An explicit empty object can therefore bypass day-0 defaulting and leave behavior implicit downstream.
Please either require installationPolicy in this schema or ensure defaulting fills it when networkObservability is present but empty, and add a regression test for networkObservability: {}.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@data/data/install.openshift.io_installconfigs.yaml` around lines 5009 - 5022,
The schema allows an explicit empty networkObservability object to bypass
defaults and manifest emission; either make installationPolicy required in the
networkObservability schema (so installationPolicy must be present) or change
the defaulting logic so that when networkObservability is present but has no
installationPolicy (i.e., networkObservability: {}), the defaulting routine that
sets installationPolicy (see installconfig defaulting code) populates a value;
after implementing the schema or defaulting change, add a regression test that
validates behavior for networkObservability: {} to ensure installationPolicy is
set and manifests are emitted as expected.
Add support for controlling network observability installation via the networkObservability field in the install config. This field contains an installationPolicy that accepts "InstallAndEnable", "DoNotInstall", or empty string values, and defaults to "InstallAndEnable" when omitted. The field is propagated from the install config through to the cluster Network resource spec during manifest generation, allowing network observability to be installed by default with an opt-out mechanism.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/types/defaults/installconfig_test.go (1)
292-324: Add a regression case forNetworkObservability: {}with nilInstallationPolicyThe new tests cover
NetworkObservability == niland explicitDoNotInstall, but not the case whereNetworkObservabilityexists andInstallationPolicyis omitted. That case is important for defaulting safety.Suggested test case
+ { + name: "NetworkObservability present with nil InstallationPolicy", + config: &types.InstallConfig{ + Networking: &types.Networking{ + NetworkObservability: &types.NetworkObservability{}, + }, + }, + expected: defaultInstallConfig(), + },As per coding guidelines, Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/types/defaults/installconfig_test.go` around lines 292 - 324, Add a regression test case that exercises an InstallConfig where Networking.NetworkObservability is non-nil but its InstallationPolicy is nil: create a test entry similar to the existing cases using config: &types.InstallConfig{Networking: &types.Networking{NetworkObservability: &types.NetworkObservability{ /* no InstallationPolicy set */ }}}, and set expected to defaultInstallConfig() with NetworkObservability present and its InstallationPolicy defaulted as the code under test should produce; reference types.InstallConfig, types.Networking, types.NetworkObservability, InstallationPolicy and defaultInstallConfig to locate where to add the new case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/types/defaults/installconfig.go`:
- Around line 61-66: The current defaulting only sets
c.Networking.NetworkObservability when it's nil, so if
c.Networking.NetworkObservability exists but its InstallationPolicy is nil the
default is not applied; update the logic around
c.Networking.NetworkObservability to ensure that after ensuring the struct
exists (or creating it) you also check if
c.Networking.NetworkObservability.InstallationPolicy == nil and, if so, assign a
new variable set to types.NetworkObservabilityInstallAndEnable and assign its
address to InstallationPolicy; reference the existing symbols
c.Networking.NetworkObservability, InstallationPolicy, and
types.NetworkObservabilityInstallAndEnable when making this change.
---
Nitpick comments:
In `@pkg/types/defaults/installconfig_test.go`:
- Around line 292-324: Add a regression test case that exercises an
InstallConfig where Networking.NetworkObservability is non-nil but its
InstallationPolicy is nil: create a test entry similar to the existing cases
using config: &types.InstallConfig{Networking:
&types.Networking{NetworkObservability: &types.NetworkObservability{ /* no
InstallationPolicy set */ }}}, and set expected to defaultInstallConfig() with
NetworkObservability present and its InstallationPolicy defaulted as the code
under test should produce; reference types.InstallConfig, types.Networking,
types.NetworkObservability, InstallationPolicy and defaultInstallConfig to
locate where to add the new case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 39f1f094-ddf3-4fc8-8190-15e509e670a3
⛔ Files ignored due to path filters (7)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/config/v1/types_network.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!vendor/**,!**/vendor/**vendor/modules.txtis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (9)
data/data/install.openshift.io_installconfigs.yamlgo.modpkg/asset/agent/installconfig_test.gopkg/asset/installconfig/installconfig_test.gopkg/asset/manifests/network.gopkg/types/defaults/installconfig.gopkg/types/defaults/installconfig_test.gopkg/types/installconfig.gopkg/types/zz_generated.deepcopy.go
✅ Files skipped from review due to trivial changes (3)
- pkg/asset/installconfig/installconfig_test.go
- pkg/asset/manifests/network.go
- pkg/types/installconfig.go
🚧 Files skipped from review as they are similar to previous changes (2)
- go.mod
- pkg/asset/agent/installconfig_test.go
| if c.Networking.NetworkObservability == nil { | ||
| installationPolicy := types.NetworkObservabilityInstallAndEnable | ||
| c.Networking.NetworkObservability = &types.NetworkObservability{ | ||
| InstallationPolicy: &installationPolicy, | ||
| } | ||
| } |
There was a problem hiding this comment.
Default policy is not applied when NetworkObservability exists but InstallationPolicy is omitted
Line 61 only defaults when NetworkObservability is nil. If a user sets networkObservability: {}, InstallationPolicy remains nil, so the “default Enable” behavior is skipped.
Proposed fix
- if c.Networking.NetworkObservability == nil {
- installationPolicy := types.NetworkObservabilityInstallAndEnable
- c.Networking.NetworkObservability = &types.NetworkObservability{
- InstallationPolicy: &installationPolicy,
- }
- }
+ if c.Networking.NetworkObservability == nil {
+ c.Networking.NetworkObservability = &types.NetworkObservability{}
+ }
+ if c.Networking.NetworkObservability.InstallationPolicy == nil {
+ installationPolicy := types.NetworkObservabilityInstallAndEnable
+ c.Networking.NetworkObservability.InstallationPolicy = &installationPolicy
+ }📝 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.
| if c.Networking.NetworkObservability == nil { | |
| installationPolicy := types.NetworkObservabilityInstallAndEnable | |
| c.Networking.NetworkObservability = &types.NetworkObservability{ | |
| InstallationPolicy: &installationPolicy, | |
| } | |
| } | |
| if c.Networking.NetworkObservability == nil { | |
| c.Networking.NetworkObservability = &types.NetworkObservability{} | |
| } | |
| if c.Networking.NetworkObservability.InstallationPolicy == nil { | |
| installationPolicy := types.NetworkObservabilityInstallAndEnable | |
| c.Networking.NetworkObservability.InstallationPolicy = &installationPolicy | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/types/defaults/installconfig.go` around lines 61 - 66, The current
defaulting only sets c.Networking.NetworkObservability when it's nil, so if
c.Networking.NetworkObservability exists but its InstallationPolicy is nil the
default is not applied; update the logic around
c.Networking.NetworkObservability to ensure that after ensuring the struct
exists (or creating it) you also check if
c.Networking.NetworkObservability.InstallationPolicy == nil and, if so, assign a
new variable set to types.NetworkObservabilityInstallAndEnable and assign its
address to InstallationPolicy; reference the existing symbols
c.Networking.NetworkObservability, InstallationPolicy, and
types.NetworkObservabilityInstallAndEnable when making this change.
|
@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. |
3 participants
networking: Add day-0 installation support for Network Observability
github.com/openshift/apito include theinstallNetworkObservabilityfield in the Network CR spec. This dependency must be updated to point to the official release before merging.Summary
This PR implements day-0 installation support for Network Observability as described in the enhancement proposal.
Users can now enable Network Observability during cluster installation by setting the
installNetworkObservabilityfield in the install-config.yaml networking section. When enabled (or omitted, as it defaults to "Enable"), the installer will configure the cluster to deploy Network Observability on day-0.