objectstate: use existing objects as a base#1502
Open
Tal-or wants to merge 5 commits into
Open
Conversation
Contributor
|
Caution There are some errors in your PipelineRun template.
|
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Tal-or The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Tal-or
changed the title
openshift-ci
Bot
added
approved
Collaborator
Author
|
We should test that we're getting similar results to #1194 |
Collaborator
Author
|
implementation of: |
Collaborator
Author
|
/retest |
Tal-or
force-pushed
the
decouple_from_deployer
branch
6 times, most recently
from
941bfcc to
7a35140
Compare
Collaborator
Author
|
/retest |
Tal-or
force-pushed
the
decouple_from_deployer
branch
2 times, most recently
from
88656a9 to
f2d2773
Compare
Tal-or
force-pushed
the
decouple_from_deployer
branch
2 times, most recently
from
12c7b46 to
75d611d
Compare
Tal-or
changed the title
openshift-ci
Bot
added
needs-rebase
Usually we're using a bare manifest as the base template for all the resources we own. The operator performs the relevant modifications on those manifests and apply them on the cluster. That usually creates unnecessary updates because the bare manifests are not containing some of the defaults which applied by the API or other controllers that present on the cluster. To fix that, we suggest to use the existing resources as the base, so those already contains all the defaults, perform the changes we care about and then apply those on the cluster. This is a different approach to accomplish: openshift-kni#1194 Signed-off-by: Talor Itzhak <titzhak@redhat.com>
Tal-or
force-pushed
the
decouple_from_deployer
branch
from
75d611d to
5cef96a
Compare
openshift-ci
Bot
removed
the
needs-rebase
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughController now builds mutated RTE manifests from existing cluster objects before apply; apply records a Prometheus counter for client Update calls; reusable pod volume helpers were added and integrated; toleration and config-volume behaviors adjusted; new e2e test asserts metadata-only reconciles do not trigger client Update; Dockerfile repo restriction tightened. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (4)
pkg/numaresourcesscheduler/objectstate/sched/sched.go (1)
227-237: Consider adding a defensive check before accessingpodSpec.Volumes[0].The code assumes
volume.AddConfigMapalways populatespodSpec.Volumes, but directly accessing index 0 without a length check could panic if the helper's behavior changes.🛡️ Optional defensive check
// Use the volume package to create the ConfigMap volume volume.AddConfigMap(podSpec, container, schedVolumeConfigName, "/tmp", configMapName, volume.DefaultMode, false, false) // Return just the volume part (we don't need the mount) + if len(podSpec.Volumes) == 0 { + return corev1.Volume{} + } return podSpec.Volumes[0]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/numaresourcesscheduler/objectstate/sched/sched.go` around lines 227 - 237, NewSchedConfigVolume assumes volume.AddConfigMap always appends a volume and directly returns podSpec.Volumes[0], which can panic if the helper changes; update NewSchedConfigVolume to defensively check that podSpec.Volumes is non-nil and has at least one element after calling volume.AddConfigMap (referencing NewSchedConfigVolume, volume.AddConfigMap, and podSpec.Volumes[0]) and return a zero-value corev1.Volume or an error/handle the empty case appropriately instead of indexing into an empty slice.pkg/objectupdate/volume/volume_test.go (1)
218-229: Test for unsupported type lacks assertions.This test only verifies that
Adddoesn't panic with an unsupported type, but doesn't assert the actual behavior. Consider adding assertions to document expected behavior (e.g., whether volume/mount entries are still added, or the call is a no-op).💡 Suggested improvement
func TestAdd_UnsupportedType(t *testing.T) { podSpec := &corev1.PodSpec{} container := &corev1.Container{} opts := Options{ VolumeName: "test-volume", MountPath: "/test", Type: VolumeType("unsupported"), } Add(podSpec, container, opts) + + // Document expected behavior for unsupported types + // Adjust assertions based on actual implementation behavior + if len(podSpec.Volumes) != 1 { + t.Errorf("expected 1 volume entry even for unsupported type, got %d", len(podSpec.Volumes)) + } + if len(container.VolumeMounts) != 1 { + t.Errorf("expected 1 volume mount even for unsupported type, got %d", len(container.VolumeMounts)) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/objectupdate/volume/volume_test.go` around lines 218 - 229, The test TestAdd_UnsupportedType currently only ensures Add(podSpec, container, opts) doesn't panic; update it to assert the expected behavior for an unsupported VolumeType by checking podSpec.Volumes and container.VolumeMounts after calling Add. Specifically, in the test for Options{VolumeName:"test-volume",MountPath:"/test",Type:VolumeType("unsupported")}, decide whether Add should be a no-op or still append entries and then assert accordingly (e.g., expect zero change: len(podSpec.Volumes)==0 and len(container.VolumeMounts)==0, or expect one entry with Name "test-volume" and MountPath "/test"). Reference the test function TestAdd_UnsupportedType and the Options/VolumeType usage to locate where to add these assertions.pkg/objectupdate/rte/rte.go (1)
307-332: Function signature returns error but always returns nil.
AddVolumeMountMemoryhas anerrorreturn type, but the function always returnsnil. Either remove the error return (if no error conditions exist) or add actual error handling for the volume operations.Additionally, the function name suggests it only adds a memory volume, but it also adds metrics secret, host-sys, and host-podresources volumes. Consider either renaming for clarity or splitting responsibilities.
Option 1: Remove unused error return
-func AddVolumeMountMemory(podSpec *corev1.PodSpec, cnt *corev1.Container, mountName, dirName string, sizeMiB int64) error { +func AddVolumeMountMemory(podSpec *corev1.PodSpec, cnt *corev1.Container, mountName, dirName string, sizeMiB int64) { // Add the requested memory volume mount volume.AddMemoryVolume(podSpec, cnt, mountName, dirName, sizeMiB) // ... rest of function ... - return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/objectupdate/rte/rte.go` around lines 307 - 332, The AddVolumeMountMemory function currently returns an error but always returns nil; either remove the error return from the signature and update all callers to match, or add concrete error handling and propagate errors from the underlying helpers (e.g., volume.AddMemoryVolume, volume.AddSecret, volume.AddHostPath) so AddVolumeMountMemory returns any failure encountered; additionally, because AddVolumeMountMemory also adds metrics secret, host-sys and host-podresources volumes, rename the function to something more descriptive (e.g., AddRteVolumes) or split it into smaller functions (e.g., AddMemoryVolumeMount, AddMetricsSecretVolume, AddHostVolumes) and update call sites accordingly to keep responsibilities clear.pkg/objectupdate/volume/volume.go (1)
172-175: Misleading parameter namesizeMiB.The parameter
sizeMiBis passed directly toresource.NewQuantityas bytes. Looking at the caller (8*_MiBwhere_MiB = 1024 * 1024), the value is already in bytes, not MiB. The parameter name should besizeBytesto avoid confusion.Proposed fix
// AddMemoryVolume adds an EmptyDir volume with memory storage -func AddMemoryVolume(podSpec *corev1.PodSpec, container *corev1.Container, volumeName, mountPath string, sizeMiB int64) { - AddEmptyDir(podSpec, container, volumeName, mountPath, corev1.StorageMediumMemory, resource.NewQuantity(sizeMiB, resource.BinarySI)) +func AddMemoryVolume(podSpec *corev1.PodSpec, container *corev1.Container, volumeName, mountPath string, sizeBytes int64) { + AddEmptyDir(podSpec, container, volumeName, mountPath, corev1.StorageMediumMemory, resource.NewQuantity(sizeBytes, resource.BinarySI)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/objectupdate/volume/volume.go` around lines 172 - 175, The parameter name sizeMiB in AddMemoryVolume is misleading because callers pass a byte count; rename the parameter to sizeBytes (and update its usage in the function and comment) so AddMemoryVolume(podSpec *corev1.PodSpec, container *corev1.Container, volumeName, mountPath string, sizeBytes int64) calls AddEmptyDir(..., corev1.StorageMediumMemory, resource.NewQuantity(sizeBytes, resource.BinarySI)); update all callers that pass 8*_MiB to use the new parameter name (no value change) and adjust the function comment to say "sizeBytes" instead of "sizeMiB".
🤖 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/objectstate/rte/rte.go`:
- Around line 238-244: The code in the loop over existingMF.daemonSets
nondeterministically picks the first daemon set spec and assigns it to
mf.Core.DaemonSet.Spec; change this to a deterministic selection by iterating
keys in a stable order (e.g., collect keys from existingMF.daemonSets, sort them
lexicographically, then pick the first key whose value has daemonSetError ==
nil) or by explicitly selecting a known key/name; update the block where
mf.Core.DaemonSet.Spec = *ds.daemonSet.Spec.DeepCopy() to use the
deterministically chosen ds (using the sorted key or explicit key lookup) so
reconciliations are stable.
In `@pkg/objectupdate/volume/volume.go`:
- Around line 117-121: The default branch of the volume type switch currently
logs an unsupported type but still appends an empty volume to podSpec.Volumes;
modify the default case to return early (or return an error) instead of
appending: in the switch default for handling opts.Type, replace the
klog.InfoS(...) + fallthrough to append with a short-circuit (e.g., log and
return nil or return fmt.Errorf(...)) so that volume (the variable created for
the switch) is not appended to podSpec.Volumes and invalid empty VolumeSource
objects are never added.
In `@test/e2e/serial/tests/reconcile_noop.go`:
- Around line 54-68: The parseApplyClientUpdatesScalar function currently
returns 0 when the metric regex doesn't match, which hides "metric not found"
from callers; change parseApplyClientUpdatesScalar to return a non-nil error
(e.g., fmt.Errorf("metric %s not found", apply.MetricApplyClientUpdatesTotal))
when m has length < 2 instead of returning 0, and update any callers (tests that
call parseApplyClientUpdatesScalar) to handle and fail on that error so missing
metrics fail tests rather than producing a silent zero delta.
---
Nitpick comments:
In `@pkg/numaresourcesscheduler/objectstate/sched/sched.go`:
- Around line 227-237: NewSchedConfigVolume assumes volume.AddConfigMap always
appends a volume and directly returns podSpec.Volumes[0], which can panic if the
helper changes; update NewSchedConfigVolume to defensively check that
podSpec.Volumes is non-nil and has at least one element after calling
volume.AddConfigMap (referencing NewSchedConfigVolume, volume.AddConfigMap, and
podSpec.Volumes[0]) and return a zero-value corev1.Volume or an error/handle the
empty case appropriately instead of indexing into an empty slice.
In `@pkg/objectupdate/rte/rte.go`:
- Around line 307-332: The AddVolumeMountMemory function currently returns an
error but always returns nil; either remove the error return from the signature
and update all callers to match, or add concrete error handling and propagate
errors from the underlying helpers (e.g., volume.AddMemoryVolume,
volume.AddSecret, volume.AddHostPath) so AddVolumeMountMemory returns any
failure encountered; additionally, because AddVolumeMountMemory also adds
metrics secret, host-sys and host-podresources volumes, rename the function to
something more descriptive (e.g., AddRteVolumes) or split it into smaller
functions (e.g., AddMemoryVolumeMount, AddMetricsSecretVolume, AddHostVolumes)
and update call sites accordingly to keep responsibilities clear.
In `@pkg/objectupdate/volume/volume_test.go`:
- Around line 218-229: The test TestAdd_UnsupportedType currently only ensures
Add(podSpec, container, opts) doesn't panic; update it to assert the expected
behavior for an unsupported VolumeType by checking podSpec.Volumes and
container.VolumeMounts after calling Add. Specifically, in the test for
Options{VolumeName:"test-volume",MountPath:"/test",Type:VolumeType("unsupported")},
decide whether Add should be a no-op or still append entries and then assert
accordingly (e.g., expect zero change: len(podSpec.Volumes)==0 and
len(container.VolumeMounts)==0, or expect one entry with Name "test-volume" and
MountPath "/test"). Reference the test function TestAdd_UnsupportedType and the
Options/VolumeType usage to locate where to add these assertions.
In `@pkg/objectupdate/volume/volume.go`:
- Around line 172-175: The parameter name sizeMiB in AddMemoryVolume is
misleading because callers pass a byte count; rename the parameter to sizeBytes
(and update its usage in the function and comment) so AddMemoryVolume(podSpec
*corev1.PodSpec, container *corev1.Container, volumeName, mountPath string,
sizeBytes int64) calls AddEmptyDir(..., corev1.StorageMediumMemory,
resource.NewQuantity(sizeBytes, resource.BinarySI)); update all callers that
pass 8*_MiB to use the new parameter name (no value change) and adjust the
function comment to say "sizeBytes" instead of "sizeMiB".
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 72abe787-ba5c-4f58-a860-3b010f01d24f
📒 Files selected for processing (12)
Dockerfileinternal/controller/numaresourcesoperator_controller.gopkg/apply/apply.gopkg/apply/metrics.gopkg/numaresourcesscheduler/objectstate/sched/sched.gopkg/objectstate/api/api.gopkg/objectstate/rte/rte.gopkg/objectupdate/rte/rte.gopkg/objectupdate/rte/rte_test.gopkg/objectupdate/volume/volume.gopkg/objectupdate/volume/volume_test.gotest/e2e/serial/tests/reconcile_noop.go
Tal-or
force-pushed
the
decouple_from_deployer
branch
3 times, most recently
from
316477f to
1469be1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/objectupdate/rte/rte.go`:
- Around line 307-332: Add an idempotency guard before calling
volume.AddMemoryVolume in AddVolumeMountMemory: check if the memory volume/mount
already exists using hasVolume(podSpec, mountName) and hasVolumeMount(cnt,
mountName) (or the same predicate used for other volumes) and only call
volume.AddMemoryVolume(podSpec, cnt, mountName, dirName, sizeMiB) when both
checks are false; this mirrors the existing patterns around volume.AddSecret,
volume.AddHostPath and prevents duplicate memory volumes when
AddVolumeMountMemory is invoked on an existing podSpec.
In `@pkg/objectupdate/volume/volume.go`:
- Around line 63-122: The Add function currently appends volumeMount to
container.VolumeMounts before validating opts.Type, which can leave an orphaned
VolumeMount if the switch falls into default and returns; move the type
validation/creation (the switch on opts.Type that constructs volume) before
appending the volumeMount, or alternatively defer appending
container.VolumeMounts until after the switch completes and podSpec.Volumes is
successfully appended, ensuring the volume is only mounted when the
corresponding volume (created in the switch) is added to podSpec.Volumes.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c32bb496-429b-4514-9d1d-c37da4a8b46c
📒 Files selected for processing (9)
Dockerfilepkg/apply/apply.gopkg/apply/metrics.gopkg/numaresourcesscheduler/objectstate/sched/sched.gopkg/objectupdate/rte/rte.gopkg/objectupdate/rte/rte_test.gopkg/objectupdate/volume/volume.gopkg/objectupdate/volume/volume_test.gotest/e2e/serial/tests/reconcile_noop.go
✅ Files skipped from review due to trivial changes (1)
- pkg/objectupdate/volume/volume_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/numaresourcesscheduler/objectstate/sched/sched.go
we're updating the volumes for the pod and the volume mounts for the RTE container on different stages during the operator flow. This commit generelize the update flow and make sure to set all the defaults bits to avoid any unnecessary updates calls to the server. Signed-off-by: Talor Itzhak <titzhak@redhat.com>
in case no subscription for rhel repos is available, the image failed to build. disabling rhel repo so it will be able to pull hwdata and other deps if necessary. Signed-off-by: Talor Itzhak <titzhak@redhat.com>
This metric addition intended to be use only in tests (for time being) to be able to track when the controller is calling Update() Signed-off-by: Talor Itzhak <titzhak@redhat.com>
to make sure that changes in the objects API won't causes unnecessary updates we're adding this test to count for updates when a dummy annotation added to the CR. The result should be zero calls for updates when dummy changes are made. Signed-off-by: Talor Itzhak <titzhak@redhat.com>
Tal-or
force-pushed
the
decouple_from_deployer
branch
from
1469be1 to
39861f9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/objectupdate/rte/rte.go (1)
307-334: Function name is misleading given its expanded scope.
AddVolumeMountMemorynow adds four different volumes (memory, metrics cert, host-sys, host-podresources), not just a memory volume. Consider renaming to better reflect its actual responsibility, e.g.,AddRTEVolumesorEnsureRTEVolumeMounts.The idempotency guards are correctly implemented, addressing the previous review concern about duplicate memory volumes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/objectupdate/rte/rte.go` around lines 307 - 334, The function AddVolumeMountMemory now performs more than just memory mounts (it also adds metrics cert, host-sys, and host-podresources), so rename it to a clearer name such as AddRTEVolumes or EnsureRTEVolumeMounts to reflect its actual responsibility; update the function declaration and all call sites to the new name, adjust any related comments/docs/tests to reference the new name, and ensure the implementation and idempotency checks (hasVolumeMount/hasVolume and calls to volume.AddMemoryVolume, volume.AddSecret, volume.AddHostPath) remain unchanged.pkg/objectupdate/volume/volume.go (1)
173-176: Parameter namesizeMiBis misleading - value is in bytes.The caller in
rte.gopasses8*_MiBwhere_MiB = 1024*1024, so the actual value is8388608(bytes). The parameter should be namedsizeBytesor the function should accept MiB and convert internally.-// AddMemoryVolume adds an EmptyDir volume with memory storage -func AddMemoryVolume(podSpec *corev1.PodSpec, container *corev1.Container, volumeName, mountPath string, sizeMiB int64) { - AddEmptyDir(podSpec, container, volumeName, mountPath, corev1.StorageMediumMemory, resource.NewQuantity(sizeMiB, resource.BinarySI)) +// AddMemoryVolume adds an EmptyDir volume with memory storage +func AddMemoryVolume(podSpec *corev1.PodSpec, container *corev1.Container, volumeName, mountPath string, sizeBytes int64) { + AddEmptyDir(podSpec, container, volumeName, mountPath, corev1.StorageMediumMemory, resource.NewQuantity(sizeBytes, resource.BinarySI)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/objectupdate/volume/volume.go` around lines 173 - 176, The parameter name sizeMiB on AddMemoryVolume is misleading because the value is actually in bytes; rename the parameter to sizeBytes in the function signature for AddMemoryVolume and update its use where it calls AddEmptyDir (resource.NewQuantity should be passed sizeBytes) and update all callers (e.g., in rte.go) to pass the byte-sized value or adjust their argument names accordingly; ensure references to AddMemoryVolume and the AddEmptyDir invocation are updated consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/serial/tests/reconcile_noop.go`:
- Around line 72-75: The curl invocation built in fetchManagerMetricsFromPod
currently can hang because it lacks per-request timeouts; modify the cmd slice
that constructs the curl call (the []string in fetchManagerMetricsFromPod) to
include conservative timeout flags such as "--connect-timeout", "5" and
"--max-time", "10" (or similar values consistent with other tests) so each curl
subprocess will fail fast on network/TLS stalls; apply the same change to the
analogous curl in metrics.go to keep behavior consistent.
---
Nitpick comments:
In `@pkg/objectupdate/rte/rte.go`:
- Around line 307-334: The function AddVolumeMountMemory now performs more than
just memory mounts (it also adds metrics cert, host-sys, and host-podresources),
so rename it to a clearer name such as AddRTEVolumes or EnsureRTEVolumeMounts to
reflect its actual responsibility; update the function declaration and all call
sites to the new name, adjust any related comments/docs/tests to reference the
new name, and ensure the implementation and idempotency checks
(hasVolumeMount/hasVolume and calls to volume.AddMemoryVolume, volume.AddSecret,
volume.AddHostPath) remain unchanged.
In `@pkg/objectupdate/volume/volume.go`:
- Around line 173-176: The parameter name sizeMiB on AddMemoryVolume is
misleading because the value is actually in bytes; rename the parameter to
sizeBytes in the function signature for AddMemoryVolume and update its use where
it calls AddEmptyDir (resource.NewQuantity should be passed sizeBytes) and
update all callers (e.g., in rte.go) to pass the byte-sized value or adjust
their argument names accordingly; ensure references to AddMemoryVolume and the
AddEmptyDir invocation are updated consistently.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 45c0487c-eb1a-4fec-9bc3-c6f5f663d32c
📒 Files selected for processing (9)
Dockerfilepkg/apply/apply.gopkg/apply/metrics.gopkg/numaresourcesscheduler/objectstate/sched/sched.gopkg/objectupdate/rte/rte.gopkg/objectupdate/rte/rte_test.gopkg/objectupdate/volume/volume.gopkg/objectupdate/volume/volume_test.gotest/e2e/serial/tests/reconcile_noop.go
✅ Files skipped from review due to trivial changes (2)
- pkg/objectupdate/rte/rte_test.go
- pkg/objectupdate/volume/volume_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/apply/apply.go
- pkg/apply/metrics.go
Collaborator
Author
|
@ffromani PTAL |
Member
|
sorry this slipped between cracks. Will try to resume this work just after the next release cut. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Usually we're using a bare manifest as the base template for all the resources we own. The operator performs the relevant modifications on those manifests and apply them on the cluster.
That usually creates unnecessary updates because the bare manifests are not containing some of the defaults which applied by the API or other controllers that present on the cluster.
To fix that, we suggest to use the existing resources as the base, so those already contains all the defaults, perform the changes we care about and then apply those on the cluster.
This is a different approach to accomplish:
#1194
Benchmarks:
Simple patch against numaresources CR (before change):
After changes:
Assisted-by: Cursor v2.6.21
AI-Attribution: AIA HAb Hin R Cursor v2.6.21 model:claude-4.6-opus-high