Skip to content

Add terminationMessagePolicy for OCP 4.21 conformance#505

Open
dustman9000 wants to merge 1 commit intoopenshift:masterfrom
dustman9000:termination-message-policy
Open

Add terminationMessagePolicy for OCP 4.21 conformance#505
dustman9000 wants to merge 1 commit intoopenshift:masterfrom
dustman9000:termination-message-policy

Conversation

@dustman9000
Copy link
Copy Markdown
Member

@dustman9000 dustman9000 commented Apr 16, 2026

Summary

  • Adds terminationMessagePolicy: FallbackToLogsOnError to the operator container spec in both OLM (deploy/) and PKO (deploy_pko/) deployment manifests

The required-scc annotation was already present in both files. The terminationMessagePolicy ensures container termination messages capture log output on error, which is required for OCP 4.21 conformance.

Test plan

  • Verify YAML is valid and properly indented
  • Deploy to integration and confirm operator starts correctly
  • Confirm terminationMessagePolicy appears in pod spec via oc get pod -o yaml

Summary by CodeRabbit

  • Chores
    • Updated deployment configurations to enhance error message diagnostics during container termination events.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 16, 2026

Walkthrough

Added terminationMessagePolicy: FallbackToLogsOnError configuration to the configure-alertmanager-operator container specification in two Kubernetes deployment manifest files, altering how container termination messages are populated on error scenarios.

Changes

Cohort / File(s) Summary
Kubernetes Deployment Configuration
deploy/04_operator.yaml, deploy_pko/Deployment-configure-alertmanager-operator.yaml.gotmpl
Added terminationMessagePolicy: FallbackToLogsOnError field to the configure-alertmanager-operator container spec to control termination message behavior.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 10
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately describes the main change: adding terminationMessagePolicy configuration for OCP 4.21 conformance, which is precisely what both deployment files were updated with.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Stable And Deterministic Test Names ✅ Passed This custom check is not applicable to this pull request. The PR exclusively modifies YAML deployment configuration files to add the terminationMessagePolicy field. The check requires assessing Ginkgo test names, but no test files were modified.
Test Structure And Quality ✅ Passed PR modifies only Kubernetes deployment YAML manifests without any Ginkgo test code changes.
Microshift Test Compatibility ✅ Passed This pull request does not introduce any new Ginkgo e2e tests; changes are limited to modifying Kubernetes deployment manifest files to add terminationMessagePolicy field.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests for SNO compatibility are being introduced in this PR; only YAML configuration files are modified.
Topology-Aware Scheduling Compatibility ✅ Passed The PR only adds terminationMessagePolicy: FallbackToLogsOnError to container specs, which controls error message handling and does not impact scheduling or pod placement.
Ote Binary Stdout Contract ✅ Passed PR modifies only Kubernetes manifest YAML files to add terminationMessagePolicy field; no Go source code changes that would affect OTE stdout contract.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PR does not add new Ginkgo e2e tests, only modifies Kubernetes deployment manifests with terminationMessagePolicy field.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 16, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
deploy/04_operator.yaml (1)

31-54: Add explicit container securityContext to both Deployment manifests for compliance scanning clarity.

The current manifests rely on the restricted-v2 SCC annotation but lack explicit container-level security settings. Adding securityContext will resolve CKV_K8S_20 and CKV_K8S_23 findings and improve portability across environments.

Apply this configuration to both deploy/04_operator.yaml and deploy_pko/Deployment-configure-alertmanager-operator.yaml.gotmpl:

Hardening patch
       containers:
         - name: configure-alertmanager-operator
           image: quay.io/redhat-services-prod/camo-hcm-tenant/configure-alertmanager-operator-master/configure-alertmanager-operator-master@sha256:06210e55ea90935f8e1ccea48b1b3db2694de6bac4564596d306dd94333933b9
+          securityContext:
+            allowPrivilegeEscalation: false
+            runAsNonRoot: true
+            capabilities:
+              drop:
+                - ALL
+            seccompProfile:
+              type: RuntimeDefault
           command:
           - configure-alertmanager-operator
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@deploy/04_operator.yaml` around lines 31 - 54, Add an explicit
container-level securityContext to the configure-alertmanager-operator container
in both Deployment manifests (the one shown and the corresponding deploy_pko
template) to satisfy CKV_K8S_20/23: set runAsNonRoot: true and a non-zero
runAsUser (e.g., 1000), set allowPrivilegeEscalation: false, set
readOnlyRootFilesystem: true, and drop all Linux capabilities (capabilities:
drop: ["ALL"]); apply this block under the container spec for the container
named configure-alertmanager-operator in both manifests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@deploy/04_operator.yaml`:
- Around line 31-54: Add an explicit container-level securityContext to the
configure-alertmanager-operator container in both Deployment manifests (the one
shown and the corresponding deploy_pko template) to satisfy CKV_K8S_20/23: set
runAsNonRoot: true and a non-zero runAsUser (e.g., 1000), set
allowPrivilegeEscalation: false, set readOnlyRootFilesystem: true, and drop all
Linux capabilities (capabilities: drop: ["ALL"]); apply this block under the
container spec for the container named configure-alertmanager-operator in both
manifests.

ℹ️ Review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro Plus

Run ID: 12a8437a-5563-4bf0-be94-88168d6c9842

📥 Commits

Reviewing files that changed from the base of the PR and between a71dd18 and 0a89ca3.

📒 Files selected for processing (2)
  • deploy/04_operator.yaml
  • deploy_pko/Deployment-configure-alertmanager-operator.yaml.gotmpl

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.26%. Comparing base (a71dd18) to head (0a89ca3).
⚠️ Report is 9 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #505   +/-   ##
=======================================
  Coverage   68.26%   68.26%           
=======================================
  Files           8        8           
  Lines        1087     1087           
=======================================
  Hits          742      742           
  Misses        315      315           
  Partials       30       30           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nephomaniac
Copy link
Copy Markdown
Contributor

/retest

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Apr 23, 2026

@dustman9000: 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.

@nephomaniac
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Apr 23, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Apr 23, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dustman9000, nephomaniac

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:
  • OWNERS [dustman9000,nephomaniac]

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

@nephomaniac
Copy link
Copy Markdown
Contributor

@dustman9000, re the failing jobs, this may need to be rebased to pick up the konflux updates/fixes(?)

nephomaniac added a commit to nephomaniac/configure-alertmanager-operator that referenced this pull request Apr 24, 2026
Adds terminationMessagePolicy: FallbackToLogsOnError to the operator
container spec in both OLM (deploy/) and PKO (deploy_pko/) deployment
manifests, and regenerates PKO test fixtures.

Replaces openshift#505 which could not be rebased.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants