ci-operator: Expose IP_POOL_AVAILABLE only when the step ip_pool has run

[danilo-gemoli]

Up until now, the IP_POOL_AVAILABLE env var has been exposed only for the tests that uses the aws cluster profile.
This logic is fragile and problematic when someone (like me) add a new cluster profile that supports the BYOIP feature and forget to update the code that exposes the variable.

This PR changes the code such that the env variable only when the ip_pool step run, regardless of the cluster profile in use.

ci-operator: Expose IP_POOL_AVAILABLE only when the ip_pool step has run

This PR changes ci-operator so the IP_POOL_AVAILABLE environment variable is published based on whether the ip_pool step actually ran, instead of being tied to specific cluster profiles.

What changed and why:

• ip_pool step:
  • IPPoolStep now tracks if the wrapped step has started using an atomic flag. Provides() only returns the IP pool lease value after the step has run and confirmed a lease is being managed.
  • This prevents exposing IP_POOL_AVAILABLE prematurely when the ip_pool step never executed.
• Cluster profile logic:
  • ClusterProfile.IPPoolLeaseType was narrowed so only ClusterProfileAWSUSEast1 returns the aws-ip-pools lease type; ClusterProfileAWS no longer reports that type. This reduces fragile profile-based assumptions.
• multi_stage executor:
  • The environment assembly now attempts to include DEFAULT_IP_POOL_LEASE for any non-empty profile value, but only appends it when the ip_pool-provided value is non-empty — effectively making exposure runtime-driven rather than profile-driven.
• Tests:
  • Unit tests updated to exercise the new atomic step-run flag: they assert the env var is empty when the wrapped step never ran and contains lease information only after the step runs.

Practical impact for CI users/operators:

• IP_POOL_AVAILABLE will only appear in test jobs when the ip_pool step actually acquired or managed a lease, avoiding incorrect environment exposure for profiles that don't execute ip_pool.
• Adding new cluster profiles that support BYOIP will no longer require code changes to environment-exposure logic; behavior is determined at runtime by whether ip_pool runs.
• Safer, less fragile behavior across different cluster profiles and multi-stage executions.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: automatic mode

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 73257eaa-5714-4025-9b71-463a3c1b3541

📥 Commits

Reviewing files that changed from the base of the PR and between eb02750 and cddfbd4.

📒 Files selected for processing (5)

• pkg/api/clusterprofile.go
• pkg/api/leases_test.go
• pkg/steps/ip_pool.go
• pkg/steps/ip_pool_test.go
• pkg/steps/multi_stage/multi_stage.go

💤 Files with no reviewable changes (1)

• pkg/api/clusterprofile.go

---

📝 Walkthrough

Walkthrough

ClusterProfile mapping narrowed to us-east-1 for aws-ip-pools; IPPoolStep defers DEFAULT_IP_POOL_LEASE env emission until it actually runs; multi-stage environment now looks up the gated var for any non-empty profile; tests updated.

Changes

IP Pool Lease Type Mapping and Deferred Env Var Emission

Layer / File(s)	Summary
Cluster profile lease mapping pkg/api/clusterprofile.go, pkg/api/leases_test.go	ClusterProfile.IPPoolLeaseType() returns "aws-ip-pools" only for ClusterProfileAWSUSEast1; tests updated to use that profile.
IP pool step runtime tracking pkg/steps/ip_pool.go	ipPoolStep adds stepRun *atomic.Bool; constructor initializes it; Provides() returns an empty DefaultIPPoolLeaseEnv until stepRun is true; run() sets stepRun after confirming a lease.
IP pool step tests and helper pkg/steps/ip_pool_test.go	Add atomicBool() helper; update TestProvides and TestRun to initialize or set stepRun; add test asserting empty env when wrapped step did not run.
Multi-stage environment integration pkg/steps/multi_stage/multi_stage.go	environment() now looks up api.DefaultIPPoolLeaseEnv for any non-empty s.profile and appends it only when non-empty (not limited to "aws").

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• openshift/ci-tools#5169: Modifies IPPoolStep lease acquisition flow and how lease inputs are derived from profile and env values.
• openshift/ci-tools#4983: Changes in multi-stage environment and cluster profile handling that touch the same environment wiring.

Suggested labels

tide/merge-method-squash, lgtm

Suggested reviewers

• droslean
• bear-redhat
• jmguzik

🚥 Pre-merge checks | ✅ 11 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Coverage For New Features	⚠️ Warning	Multi-stage.go's environment() method change lacks test coverage. No test case sets profile field or verifies DefaultIPPoolLeaseEnv exposure when s.profile != "".	Add test case to multi_stage_test.go TestEnvironment verifying DefaultIPPoolLeaseEnv is exposed when profile is set and non-empty value exists.
Test Structure And Quality	❓ Inconclusive	The check requires reviewing Ginkgo test code, but PR contains only standard Go tests using testing package (func Test*, t.Run). Check instructions are not applicable.	Clarify if check applies to Ginkgo only or all tests. PR has standard Go tests, not Ginkgo BDD tests with It/BeforeEach patterns.

✅ Passed checks (11 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: making IP_POOL_AVAILABLE exposure conditional on the ip_pool step actually running, which is the core objective of the PR.
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.
Go Error Handling	✅ Passed	PR properly handles Go errors: errors wrapped with %w, nil checks before dereferences, no ignored errors in new code, panic only in test mocks. Environment changes safely check params.Get() results.
Stable And Deterministic Test Names	✅ Passed	Modified test files use standard Go testing with static test case names—no Ginkgo tests found. All test names are deterministic (e.g., "leases acquired", "aws, with 4.16 branch").
Microshift Test Compatibility	✅ Passed	No Ginkgo e2e tests are added in this PR. All modified test files are standard Go unit tests using testing.T, not cluster-facing e2e tests subject to MicroShift compatibility requirements.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PR contains only unit tests (golang testing package), not Ginkgo e2e tests. No new e2e tests were added that would require SNO compatibility checks.
Topology-Aware Scheduling Compatibility	✅ Passed	PR modifies only CI/CD tooling code (Go source files in openshift/ci-tools), not deployment manifests or operator code. No scheduling constraints found.
Ote Binary Stdout Contract	✅ Passed	No process-level functions (main, init, TestMain, BeforeSuite, etc.) exist in modified files, and no stdout writes are present. Code uses only logrus for logging, which writes to stderr.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No new Ginkgo e2e tests added. Changes contain only standard Go unit tests and API/logic modifications.

_{✏️ 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.}

[danilo-gemoli]

/test e2e

[danilo-gemoli]

/test e2e

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danilo-gemoli, Prucek

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 [Prucek,danilo-gemoli]

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

[openshift-merge-bot]

Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage.

[danilo-gemoli]

/override ci/prow/e2e

[openshift-ci]

@danilo-gemoli: Overrode contexts on behalf of danilo-gemoli: ci/prow/e2e

Details

In response to this:

/override ci/prow/e2e

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.

[openshift-ci]

@danilo-gemoli: 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.