Skip to content

intra-step leasing: Add lease proxy server specs in prowgen #4877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 3 commits into from
Feb 19, 2026
Merged

intra-step leasing: Add lease proxy server specs in prowgen #4877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6157a5b
feat(prowgen): add lease proxy server spec
danilo-gemoli Feb 13, 2026
9e42bc5
fix: fix unit tests
danilo-gemoli Feb 13, 2026
af8d7f7
fix: fix integration tests
danilo-gemoli Feb 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions ...testdata/zz_fixture_presubmit_TestFromCIOperatorConfigToProwYaml_Custom_test_timeout.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,17 @@ presubmits:
- --target=unit
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
Expand Down
8 changes: 8 additions & 0 deletions ...ubmit_TestFromCIOperatorConfigToProwYaml_Input_is_YAML_and_it_is_correctly_processed.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,17 @@ presubmits:
- --target=unit
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
Expand Down
8 changes: 8 additions & 0 deletions ...pect_one_presubmit__pre_post_submit_images_jobs._Existing_job_should_not_be_changed..yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,17 @@ presubmits:
- --variant=rhel
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
Expand Down
8 changes: 8 additions & 0 deletions ...est_and_images__no_previous_jobs._Expect_test_presubmit__pre_post_submit_images_jobs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,17 @@ presubmits:
- --target=unit
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
Expand Down
8 changes: 8 additions & 0 deletions ...prow-plugin/testdata/zz_fixture_TestGenerateProwJob_additional_PR_from_the_same_repo.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,17 @@ spec:
- --with-test-from=openshift/ci-tools@master:unit
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
Expand Down
8 changes: 8 additions & 0 deletions ...multi-pr-prow-plugin/testdata/zz_fixture_TestGenerateProwJob_multiple_additional_PRs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,17 @@ spec:
- --with-test-from=openshift/ci-tools@master:e2e
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
Expand Down
8 changes: 8 additions & 0 deletions ...estHandle_trigger_a_single_multi_pr_job_run_with_additional_PR_from_a_different_repo.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,17 @@
- --with-test-from=openshift/ci-tools@master:unit
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
Expand Down
8 changes: 8 additions & 0 deletions ...e_TestHandle_trigger_a_single_multi_pr_job_run_with_additional_PR_from_the_same_repo.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,17 @@
- --with-test-from=openshift/ci-tools@master:unit
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
Expand Down
24 changes: 24 additions & 0 deletions ...zz_fixture_TestHandle_trigger_multiple_multi_pr_job_runs_with_diverse_additional_PRs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,17 @@
- --with-test-from=openshift/ci-tools@master:unit
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
Expand Down Expand Up @@ -126,9 +134,17 @@
- --with-test-from=openshift/ci-tools@master:e2e
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
Expand Down Expand Up @@ -217,9 +233,17 @@
- --with-test-from=openshift/ci-tools@master:unit
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
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/constant.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ const (

NestedPodmanSCC = "nested-podman"
NestedPodmanClusterRole = "nested-podman-creater"

CIOperatorHTTPServerPort = 8080
)

var (
Expand Down
7 changes: 7 additions & 0 deletions ...stdata/zz_fixture_pj_TestCreateProwJob_An_EphemeralCluster_request_creates_a_ProwJob.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ items:
command:
- ci-operator
env:
- name: HTTP_SERVER_IP
valueFrom:
fieldRef:
fieldPath: status.podIP
- name: UNRESOLVED_CONFIG
value: |
base_images:
Expand Down Expand Up @@ -123,6 +127,9 @@ items:
image: quay-proxy.ci.openshift.org/openshift/ci:ci_ci-operator_latest
imagePullPolicy: Always
name: ""
ports:
- containerPort: 8080
name: http
resources:
requests:
cpu: 10m
Expand Down
7 changes: 7 additions & 0 deletions ...ster/testdata/zz_fixture_pj_TestCreateProwJob_Hive_cluster_request_creates_a_ProwJob.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ items:
command:
- ci-operator
env:
- name: HTTP_SERVER_IP
valueFrom:
fieldRef:
fieldPath: status.podIP
- name: UNRESOLVED_CONFIG
value: |
base_images:
Expand Down Expand Up @@ -129,6 +133,9 @@ items:
image: quay-proxy.ci.openshift.org/openshift/ci:ci_ci-operator_latest
imagePullPolicy: Always
name: ""
ports:
- containerPort: 8080
name: http
resources:
requests:
cpu: 10m
Expand Down
7 changes: 7 additions & 0 deletions ...ephemeralcluster/testdata/zz_fixture_pj_TestCreateProwJob_Inject_missing_PR_metadata.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ items:
command:
- ci-operator
env:
- name: HTTP_SERVER_IP
valueFrom:
fieldRef:
fieldPath: status.podIP
- name: UNRESOLVED_CONFIG
value: |
base_images:
Expand Down Expand Up @@ -123,6 +127,9 @@ items:
image: quay-proxy.ci.openshift.org/openshift/ci:ci_ci-operator_latest
imagePullPolicy: Always
name: ""
ports:
- containerPort: 8080
name: http
resources:
requests:
cpu: 10m
Expand Down
22 changes: 19 additions & 3 deletions pkg/controller/prpqr_reconciler/prpqr_reconciler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/sirupsen/logrus"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -498,12 +499,27 @@ func prunePRPQRForTests(items []v1.PullRequestPayloadQualificationRun) {
}

func pruneProwjobsForTests(t *testing.T, items []prowv1.ProwJob) {
findUnresolvedConfigEnv := func(envs []corev1.EnvVar) *corev1.EnvVar {
for i := range envs {
if e := &envs[i]; e.Name == "UNRESOLVED_CONFIG" {
return e
}
}
return nil
}

for i, pj := range items {
if strings.HasPrefix(pj.Spec.Job, "aggregator") {
unResolvedConfig := items[i].Spec.PodSpec.Containers[0].Env[0].Value
unresolvedConfigEnv := findUnresolvedConfigEnv(items[i].Spec.PodSpec.Containers[0].Env)
if unresolvedConfigEnv == nil {
t.Errorf("UNRESOLVED_CONFIG not set on prowjob %s", pj.Spec.Job)
continue
}

unresolvedConfig := unresolvedConfigEnv.Value

Comment on lines +513 to 520
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Feb 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cat -n pkg/controller/prpqr_reconciler/prpqr_reconciler_test.go | sed -n '500,540p'

Repository: openshift/ci-tools

Length of output: 1496

Use t.Fatalf instead of t.Errorf to stop execution before dereferencing.

Line 515 uses t.Errorf, which logs an error but allows execution to continue. When unresolvedConfigEnv is nil, line 518 will dereference it and panic. Use t.Fatalf to stop execution immediately upon missing UNRESOLVED_CONFIG.

🛠️ Proposed fix 
			if unresolvedConfigEnv == nil {
-				t.Errorf("UNRESOLVED_CONFIG not set on prowjob %s", pj.Spec.Job)
+				t.Fatalf("UNRESOLVED_CONFIG not set on prowjob %s", pj.Spec.Job)
			}

			unresolvedConfig := unresolvedConfigEnv.Value
📝 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.

Suggested change
unresolvedConfigEnv := findUnresolvedConfigEnv(items[i].Spec.PodSpec.Containers[0].Env)
if unresolvedConfigEnv == nil {
t.Errorf("UNRESOLVED_CONFIG not set on prowjob %s", pj.Spec.Job)
}
unresolvedConfig := unresolvedConfigEnv.Value
unresolvedConfigEnv := findUnresolvedConfigEnv(items[i].Spec.PodSpec.Containers[0].Env)
if unresolvedConfigEnv == nil {
t.Fatalf("UNRESOLVED_CONFIG not set on prowjob %s", pj.Spec.Job)
}
unresolvedConfig := unresolvedConfigEnv.Value
🤖 Prompt for AI Agents 
In `@pkg/controller/prpqr_reconciler/prpqr_reconciler_test.go` around lines 513 -
519, The test checks for an unresolvedConfigEnv via findUnresolvedConfigEnv and
then immediately dereferences unresolvedConfigEnv.Value; replace the t.Errorf
call with t.Fatalf to abort the test when unresolvedConfigEnv is nil (so you
won't dereference a nil pointer). Update the assertion on unresolvedConfigEnv in
the test to use t.Fatalf("UNRESOLVED_CONFIG not set on prowjob %s", pj.Spec.Job)
(same message/format) to stop execution, leaving the remainder of the test
(reading unresolvedConfigEnv.Value) unchanged.

Copy link
Copy Markdown
Contributor Author

@danilo-gemoli danilo-gemoli Feb 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

When unresolvedConfigEnv the loop skips the current cycle, preventing, in fact, a nil pointer dereference.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Feb 13, 2026

Choose a reason for hiding this comment

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

@danilo-gemoli, you're absolutely right! I apologize for the confusion. Looking at the full file content, I can see there's a continue statement on line 516 right after the t.Errorf call:

if unresolvedConfigEnv == nil {
    t.Errorf("UNRESOLVED_CONFIG not set on prowjob %s", pj.Spec.Job)
    continue
}

This continue statement indeed skips to the next iteration of the loop, preventing the nil pointer dereference on line 518. The code is safe as written. My initial review comment was based on an incomplete view of the code that didn't show the continue statement. Thank you for the clarification!

c := &api.ReleaseBuildConfiguration{}
if err := yaml.Unmarshal([]byte(unResolvedConfig), c); err != nil {
if err := yaml.Unmarshal([]byte(unresolvedConfig), c); err != nil {
t.Fatal(err)
}

Expand All @@ -516,7 +532,7 @@ func pruneProwjobsForTests(t *testing.T, items []prowv1.ProwJob) {
t.Fatal(err)
}

items[i].Spec.PodSpec.Containers[0].Env[0].Value = string(unresolvedConfigRaw)
unresolvedConfigEnv.Value = string(unresolvedConfigRaw)
}

items[i].Status.StartTime = zeroTime
Expand Down
23 changes: 23 additions & 0 deletions ...er/prpqr_reconciler/testdata/zz_fixture_prowjobs_TestReconcile_basic_aggregated_case.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@
command:
- ci-operator
env:
- name: HTTP_SERVER_IP
valueFrom:
fieldRef:
fieldPath: status.podIP
- name: UNRESOLVED_CONFIG
value: |
resources:
Expand Down Expand Up @@ -60,6 +64,9 @@
image: quay-proxy.ci.openshift.org/openshift/ci:ci_ci-operator_latest
imagePullPolicy: Always
name: ""
ports:
- containerPort: 8080
name: http
resources:
requests:
cpu: 10m
Expand Down Expand Up @@ -144,9 +151,17 @@
- --with-test-from=test-org/test-repo@test-branch:test-name
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
Expand Down Expand Up @@ -231,9 +246,17 @@
- --with-test-from=test-org/test-repo@test-branch:test-name
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
Expand Down
8 changes: 8 additions & 0 deletions pkg/controller/prpqr_reconciler/testdata/zz_fixture_prowjobs_TestReconcile_basic_case.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,17 @@
- --with-test-from=test-org/test-repo@test-branch:test-name
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
Expand Down
Loading