feat(crier): switch to upstream manifests#5130
Conversation
Signed-off-by: Denis Ollier <dollierp@redhat.com>
There was a problem hiding this comment.
Code Review
This pull request refactors the Prow deployment to use upstream crier manifests instead of local ones, updating secret handling for the Slack token and adjusting Kustomize configurations and patches accordingly. A review comment suggests changing the hardcoded namespace kubevirt-prow-jobs to test-pods in the new crier-rbac.yaml file to ensure consistency and support dynamic namespace patching.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| kind: Role | ||
| apiVersion: rbac.authorization.k8s.io/v1 | ||
| metadata: | ||
| namespace: kubevirt-prow-jobs |
There was a problem hiding this comment.
To maintain consistency with other roles in the overlay (such as deck and prow-controller-manager) and to allow dynamic namespace patching via prow-jobs-namespace.yaml, please use the placeholder namespace test-pods instead of the hardcoded kubevirt-prow-jobs namespace.
kind: Role
apiVersion: rbac.authorization.k8s.io/v1
metadata:
namespace: test-podsThere was a problem hiding this comment.
@dollierp is there something to that? I don't remember we even have this placeholder namespace? Is that part of the new manifests and I missed it?
There was a problem hiding this comment.
Yet another AI hallucination, all github/ci/prow-deploy/kustom/overlays/kubevirt-prow-control-plane/resources/*-rbac.yaml files are using kubevirt-prow-jobs namespace.
| kind: Role | ||
| apiVersion: rbac.authorization.k8s.io/v1 | ||
| metadata: | ||
| namespace: kubevirt-prow-jobs |
There was a problem hiding this comment.
@dollierp is there something to that? I don't remember we even have this placeholder namespace? Is that part of the new manifests and I missed it?
| @@ -1,4 +1,9 @@ | |||
| resources: | |||
| - manifests/upstream/crier_deployment.yaml | |||
| - manifests/upstream/crier_rolebinding_test-pods.yaml | |||
There was a problem hiding this comment.
i thought we weren't using test-pods?
There was a problem hiding this comment.
Upstream manifests use test-pods while we are using kubevirt-prow-jobs, kustomize rules are doing the replacement for rolebindings here: https://github.com/kubevirt/project-infra/blob/main/github/ci/prow-deploy/kustom/overlays/kubevirt-prow-control-plane/kustomization.yaml#L194-L198.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dhiller 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 |
|
@dollierp: Updated the
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 kubernetes-sigs/prow repository. |
|
Looks like it worked as expected, |
What this PR does / why we need it:
Make crier deployment use new upstream manifests instead of old k8s-test-infra manifests.
Changes in manifests generated by kustomize:
Special notes for your reviewer:
/cc @dhiller