Skip to content

kms: remove TODO comment about resource requests in kms-preflight pod manifest - #2357

Open
bertinatto wants to merge 1 commit into
openshift:masterfrom
bertinatto:resources-requests-preflight
Open

kms: remove TODO comment about resource requests in kms-preflight pod manifest#2357
bertinatto wants to merge 1 commit into
openshift:masterfrom
bertinatto:resources-requests-preflight

Conversation

@bertinatto

@bertinatto bertinatto commented Jul 9, 2026

Copy link
Copy Markdown
Member

The current values should be good enough for a one shot pod. These values are also used for pods that use the same binary, but different commands in KASO.

Summary by CodeRabbit

  • Chores
    • Removed an outdated note from the preflight pod template. No functional behavior or user-facing configuration changed.

… manifest

The current values should be good enough for a one shot pod.
These values are also used for pods that use the same binary,
but different commands in KASO.
@openshift-ci
openshift-ci Bot requested review from dgrisonnet and p0lyn0mial July 9, 2026 17:53
@openshift-ci

openshift-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bertinatto

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 9, 2026
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b672924c-597e-4e24-81a0-63a2828fb0a3

📥 Commits

Reviewing files that changed from the base of the PR and between 64c3ee5 and c543409.

📒 Files selected for processing (1)
  • pkg/operator/encryption/kms/preflight/assets/kms-preflight-pod.yaml
💤 Files with no reviewable changes (1)
  • pkg/operator/encryption/kms/preflight/assets/kms-preflight-pod.yaml

Walkthrough

A TODO comment preceding the resources section in the KMS preflight pod YAML template was removed. No functional or structural changes were made to the pod specification.

Changes

Preflight Pod YAML Cleanup

Layer / File(s) Summary
Remove TODO comment
pkg/operator/encryption/kms/preflight/assets/kms-preflight-pod.yaml
Deleted a TODO comment about resource request values that preceded the resources section; the resources section itself remains unchanged.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested labels: lgtm

Suggested reviewers: ardaguclu, p0lyn0mial

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the only change: removing a TODO comment about resource requests from the kms-preflight pod manifest.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed Only a YAML manifest comment changed; no Ginkgo tests or test titles were added or edited.
Test Structure And Quality ✅ Passed PR only removes a YAML TODO comment; no Ginkgo tests or test behavior changed, so the test-structure check is not applicable.
Microshift Test Compatibility ✅ Passed Only a YAML comment changed; no new Ginkgo tests or MicroShift-relevant APIs were added.
Single Node Openshift (Sno) Test Compatibility ✅ Passed Only a YAML comment was removed in kms-preflight-pod.yaml; no Ginkgo/e2e tests or SNO-sensitive code were added.
Topology-Aware Scheduling Compatibility ✅ Passed This PR removes only a TODO comment from the pod manifest; no new scheduling constraints are introduced. The nodeSelector targeting master nodes was pre-existing and outside this PR's scope.
Ote Binary Stdout Contract ✅ Passed Only a YAML TODO comment was removed; no process-level code or stdout writes were added.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed Only a YAML comment was removed; no new Ginkgo e2e tests or network-facing code were added.
No-Weak-Crypto ✅ Passed Only a TODO comment was removed from a YAML pod manifest; no crypto code or weak algorithms were added or changed.
Container-Privileges ✅ Passed Only a TODO comment was removed; no privileged, hostPID/IPC, SYS_ADMIN, or allowPrivilegeEscalation changes were introduced.
No-Sensitive-Data-In-Logs ✅ Passed Only a TODO comment was removed from a pod manifest; no logs or sensitive fields were added or changed.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@openshift-ci

openshift-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

@bertinatto: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@p0lyn0mial

Copy link
Copy Markdown
Contributor

@bertinatto the pod will also have plugins attached. I think you've already evaluated the resource requirements for each plugin. Do you think the resource requirements we've defined today are good enough?

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants