Add CI configuration for openshift/oc-tnf#80511
Conversation
|
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:
WalkthroughFive new YAML files onboard the Changesopenshift/oc-tnf CI Infrastructure Onboarding
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Neilhamza 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@ci-operator/config/openshift/oc-tnf/openshift-oc-tnf-main.yaml`:
- Line 21: The test command in the `commands` field on line 21 does not include
race detection, which means the tests are not checking for race conditions. Add
the `-race` flag to the GOFLAGS variable in the command string so that race
detection is enabled when running the make test target. The fix involves
modifying the GOFLAGS string from "-mod=readonly" to include the -race flag
alongside the existing -mod=readonly flag.
In `@core-services/prow/02_config/openshift/oc-tnf/_pluginconfig.yaml`:
- Around line 10-13: The lgtm plugin is configured with repo-scoped settings but
is not registered in the plugins list for the openshift/oc-tnf repository. Since
tide requires the lgtm label to function properly, this missing plugin can block
normal label workflows. To fix this, add lgtm to the plugins list under
plugins.openshift/oc-tnf.plugins alongside the existing approve entry so the
plugin is properly enabled for the repository.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c215a18a-866f-4c5a-9bea-d30db8ca19e5
📒 Files selected for processing (3)
ci-operator/config/openshift/oc-tnf/openshift-oc-tnf-main.yamlcore-services/prow/02_config/openshift/oc-tnf/_pluginconfig.yamlcore-services/prow/02_config/openshift/oc-tnf/_prowconfig.yaml
| memory: 200Mi | ||
| tests: | ||
| - as: unit | ||
| commands: GOFLAGS="-mod=readonly" make test |
There was a problem hiding this comment.
Enable race detection in the unit test command.
The current command runs make test without --race, so it does not meet the intended race-check coverage for this onboarding CI path.
Suggested patch
- commands: GOFLAGS="-mod=readonly" make test
+ commands: GOFLAGS="-mod=readonly" make test --race📝 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.
| commands: GOFLAGS="-mod=readonly" make test | |
| commands: GOFLAGS="-mod=readonly -race" make test |
🤖 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 `@ci-operator/config/openshift/oc-tnf/openshift-oc-tnf-main.yaml` at line 21,
The test command in the `commands` field on line 21 does not include race
detection, which means the tests are not checking for race conditions. Add the
`-race` flag to the GOFLAGS variable in the command string so that race
detection is enabled when running the make test target. The fix involves
modifying the GOFLAGS string from "-mod=readonly" to include the -race flag
alongside the existing -mod=readonly flag.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@ci-operator/jobs/openshift/oc-tnf/openshift-oc-tnf-main-presubmits.yaml`:
- Around line 20-55: The container spec for the ci-operator target is missing
required security hardening settings. Add a securityContext field to the
container specification that includes runAsNonRoot set to true,
readOnlyRootFilesystem set to true, allowPrivilegeEscalation set to false, and
capabilities with drop set to ALL. These settings should be added at the same
level as the args, command, env, image, and other container properties to ensure
the generated job inherits the proper security hardening.
- Around line 39-41: The presubmit containers are missing required resource
specifications: they only define requests.cpu but lack memory requests, memory
limits, and CPU limits. Rather than editing the generated yaml files directly in
ci-operator/jobs/, locate the source job configuration in
ci-operator/config/openshift/oc-tnf/ and add the missing resource specifications
(memory requests, memory limits, and CPU limits) to each of the four container
specs. After making these changes to the source configuration, regenerate the
job manifests by running make update to properly update the ci-operator/jobs/
output files.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: adae36e0-16d3-4b9f-a236-0311f9109c00
📒 Files selected for processing (3)
ci-operator/config/openshift/oc-tnf/OWNERSci-operator/jobs/openshift/oc-tnf/openshift-oc-tnf-main-presubmits.yamlcore-services/prow/02_config/openshift/oc-tnf/_pluginconfig.yaml
✅ Files skipped from review due to trivial changes (1)
- ci-operator/config/openshift/oc-tnf/OWNERS
🚧 Files skipped from review as they are similar to previous changes (1)
- core-services/prow/02_config/openshift/oc-tnf/_pluginconfig.yaml
| containers: | ||
| - args: | ||
| - --gcs-upload-secret=/secrets/gcs/service-account.json | ||
| - --image-import-pull-secret=/etc/pull-secret/.dockerconfigjson | ||
| - --report-credentials-file=/etc/report/credentials | ||
| - --target=golint | ||
| command: | ||
| - ci-operator | ||
| env: | ||
| - name: HTTP_SERVER_IP | ||
| valueFrom: | ||
| fieldRef: | ||
| fieldPath: status.podIP | ||
| image: quay-proxy.ci.openshift.org/openshift/ci:ci_ci-operator_latest | ||
| imagePullPolicy: Always | ||
| name: "" | ||
| ports: | ||
| - containerPort: 8080 | ||
| name: http | ||
| resources: | ||
| requests: | ||
| cpu: 10m | ||
| volumeMounts: | ||
| - mountPath: /secrets/gcs | ||
| name: gcs-credentials | ||
| readOnly: true | ||
| - mountPath: /secrets/manifest-tool | ||
| name: manifest-tool-local-pusher | ||
| readOnly: true | ||
| - mountPath: /etc/pull-secret | ||
| name: pull-secret | ||
| readOnly: true | ||
| - mountPath: /etc/report | ||
| name: result-aggregator | ||
| readOnly: true | ||
| serviceAccountName: ci-operator |
There was a problem hiding this comment.
Harden container securityContext for all generated presubmit pod specs.
The container specs do not set required hardening (runAsNonRoot, readOnlyRootFilesystem, allowPrivilegeEscalation: false, and dropping ALL capabilities). Add these in the source ci-operator config so generated jobs inherit them.
As per coding guidelines, manifest containers should set securityContext with runAsNonRoot, readOnlyRootFilesystem, allowPrivilegeEscalation: false, and “Drop ALL capabilities, add only what is required.”
🤖 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 `@ci-operator/jobs/openshift/oc-tnf/openshift-oc-tnf-main-presubmits.yaml`
around lines 20 - 55, The container spec for the ci-operator target is missing
required security hardening settings. Add a securityContext field to the
container specification that includes runAsNonRoot set to true,
readOnlyRootFilesystem set to true, allowPrivilegeEscalation set to false, and
capabilities with drop set to ALL. These settings should be added at the same
level as the args, command, env, image, and other container properties to ensure
the generated job inherits the proper security hardening.
Source: Coding guidelines
| resources: | ||
| requests: | ||
| cpu: 10m |
There was a problem hiding this comment.
Add memory/cpu limits (and memory requests) to each presubmit container spec.
All four job containers only set requests.cpu and omit memory requests/limits and cpu limits. This breaks the manifest baseline and risks unstable scheduling/resource contention. Apply this in the source ci-operator/config/... and regenerate jobs with make update instead of editing generated output directly.
As per coding guidelines, Kubernetes/OpenShift manifests must define “Resource limits (cpu, memory) on every container,” and ci-operator/jobs/**/*.yaml must be regenerated from ci-operator/config/ via make update.
🤖 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 `@ci-operator/jobs/openshift/oc-tnf/openshift-oc-tnf-main-presubmits.yaml`
around lines 39 - 41, The presubmit containers are missing required resource
specifications: they only define requests.cpu but lack memory requests, memory
limits, and CPU limits. Rather than editing the generated yaml files directly in
ci-operator/jobs/, locate the source job configuration in
ci-operator/config/openshift/oc-tnf/ and add the missing resource specifications
(memory requests, memory limits, and CPU limits) to each of the four container
specs. After making these changes to the source configuration, regenerate the
job manifests by running make update to properly update the ci-operator/jobs/
output files.
Source: Coding guidelines
10d4cab to
828ae00
Compare
Register Prow CI jobs for the new oc-tnf plugin repo: - Unit tests, golangci-lint, go mod tidy, dependency verification - Tide merge rules requiring lgtm + approved labels - Approve plugin configuration - OWNERS synced from openshift/oc-tnf repo - Generated presubmit jobs via ci-operator-prowgen Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
f5d05ce to
d7aaef8
Compare
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@Neilhamza: all tests passed! 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. |
Summary
Register Prow CI jobs and merge configuration for the new
openshift/oc-tnfplugin repository.oc-tnfis a standaloneocplugin for Two Node with Fencing (TNF) cluster operations, distributed via krew custom index.Files added
ci-operator/config/openshift/oc-tnf/openshift-oc-tnf-main.yamlmake testwith--race)make golangci-lint)go mod tidyverificationgo-verify-deps)core-services/prow/02_config/openshift/oc-tnf/_prowconfig.yamllgtm+approvedlabelscore-services/prow/02_config/openshift/oc-tnf/_pluginconfig.yaml/approveand/lgtmcommandsContext
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Chores
oc-tnfproject, including refreshed Go linter/tooling and CI container settings.Tests
golint,modtidy,unit, andverify-deps.