Skip to content

adding log message for projects that chose undistributed, so it's cle…#968

Closed
acornett21 wants to merge 1 commit into
redhat-openshift-ecosystem:mainfrom
acornett21:update_finally_for_undistrubited
Closed

adding log message for projects that chose undistributed, so it's cle…#968
acornett21 wants to merge 1 commit into
redhat-openshift-ecosystem:mainfrom
acornett21:update_finally_for_undistrubited

Conversation

@acornett21

Copy link
Copy Markdown
Contributor

…arer when their operator isn't released to catalogs

Merge Request Checklists

  • Development is done in feature branches
  • Code changes are submitted as pull request into a primary branch [Provide reason for non-primary branch submissions]
  • Code changes are covered with unit and integration tests.
  • Code passes all automated code tests:
    • Linting
    • Code formatter - Black
    • Security scanners
    • Unit tests
    • Integration tests
  • Code is reviewed by at least 1 team member
  • Pull request is tagged with "risk/good-to-go" label for minor changes

…arer when their operator isn't released to catalogs

Signed-off-by: Adam D. Cornett <adc@redhat.com>
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add finally-step log when operator distribution is 'undistributed'
✨ Enhancement ⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

Description

• Add a conditional finally-step to log when lease acquisition was skipped.
• Surface the operator_distribution value from Pyxis for easier release troubleshooting.
• Clarify that undistributed operators are not released to any catalogs/indexes.
Diagram

graph TD
  A["operator-release-pipeline.yml"] --> B["get-pyxis-data"] --> C["acquire-lease"] --> D["log skip notice"] --> E["release-leases"]

  subgraph Legend
    direction LR
    _file["Pipeline YAML"] ~~~ _task(["Tekton task"])
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Log inside acquire-lease when it decides to skip
  • ➕ Keeps the decision and its explanation in the same task
  • ➕ Avoids coupling the log condition to task status values like "None"
  • ➖ Requires changing the acquire-lease task implementation (potentially shared/reused)
  • ➖ Harder to add without impacting existing task behavior/output
2. Gate the log step directly on operator_distribution == 'undistributed'
  • ➕ More explicit and stable condition than relying on task status
  • ➕ Better communicates the actual business rule being surfaced
  • ➖ May miss other scenarios where acquire-lease is skipped for different reasons
  • ➖ Requires ensuring the distribution result is always present and normalized

Recommendation: The current approach is reasonable for a low-risk observability improvement because it lives entirely in the pipeline template and does not alter release behavior. If this pattern expands, prefer gating on operator_distribution directly (or emitting a structured skip-reason/result from acquire-lease) to avoid relying on task status string semantics.

Files changed (1) +25 / -0

Enhancement (1) +25 / -0
operator-release-pipeline.ymlAdd conditional finally-step to log undistributed release skip +25/-0

Add conditional finally-step to log undistributed release skip

• Adds a new finally task (log-distribution-skip-notice) that runs when the acquire-lease task status indicates it was skipped. The task prints the operator_distribution value from Pyxis and a message stating the operator will not be released to any indexes/catalogs.

ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-release-pipeline.yml

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Misleading undistributed notice 🐞 Bug ≡ Correctness
Description
The finally task logs “Operator will not be released…” whenever $(tasks.acquire-lease.status) is
None, but acquire-lease can be unset when it never ran due to upstream failures, not only when
distribution is undistributed. This can mislead debugging by attributing a pipeline’s non-release
behavior to distribution choice when the real cause is earlier task failure.
Code

ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-release-pipeline.yml[R688-706]

+    # Log when acquire lease was skipped because the partner chose 'undistributed' in pyxis
+    - name: log-distribution-skip-notice
+      when:
+        - input: "$(tasks.acquire-lease.status)"
+          operator: in
+          values:
+            - "None"
+      taskSpec:
+        params:
+          - name: pipeline_image
+            type: string
+          - name: operator-distribution
+            type: string
+        steps:
+          - name: log
+            image: "$(params.pipeline_image)"
+            script: |
+              echo "Operator distribution value: $(params.operator-distribution)"
+              echo "Operator will not be released to any indexes/catalogs"
Evidence
acquire-lease is skipped based on operator_distribution != undistributed, but it also depends on
upstream tasks via runAfter, meaning it may not run for reasons unrelated to distribution. The
repository itself notes that failures prevent dependent tasks from executing, so using “task status
is None” as a proxy for “undistributed” is unreliable.

ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-release-pipeline.yml[469-489]
ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-release-pipeline.yml[688-706]
ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-release-pipeline.yml[723-725]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `log-distribution-skip-notice` finally task currently runs when `$(tasks.acquire-lease.status)` is `None`, but that condition can also be true when `acquire-lease` never executed due to upstream failures. This makes the emitted message incorrect/misleading.

## Issue Context
`acquire-lease` is skipped specifically when `operator_distribution` is `undistributed` (via the `&notUndistributed` when expression). The final log should be tied to that business condition rather than a broad “task didn’t run” state.

## Fix Focus Areas
- ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-release-pipeline.yml[688-712]

## Suggested change
Update the `when:` for `log-distribution-skip-notice` to check the actual distribution value (and optionally `get-pyxis-certification-data` status), e.g.:
- `input: "$(tasks.get-pyxis-certification-data.results.operator_distribution)"`
- `operator: in`
- `values: ["undistributed"]`
This aligns the log trigger with the reason `acquire-lease` is skipped.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Finally result dependency 🐞 Bug ☼ Reliability
Description
The new finally task depends on
$(tasks.get-pyxis-certification-data.results.operator_distribution), so the notice relies on an
upstream task result being available at finally-time. This reduces finally-block robustness,
conflicting with the pipeline’s own guidance that dependencies on other task results can prevent
final tasks from executing cleanly when earlier tasks fail.
Code

ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-release-pipeline.yml[R707-711]

+      params:
+        - name: pipeline_image
+          value: "$(params.pipeline_image)"
+        - name: operator-distribution
+          value: "$(tasks.get-pyxis-certification-data.results.operator_distribution)"
Evidence
The finally task passes a value sourced from get-pyxis-certification-data results, and the
pipeline itself documents that dependencies on other task results can prevent finally tasks from
executing when earlier tasks fail; this new dependency therefore makes the finally behavior less
robust.

ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-release-pipeline.yml[688-711]
ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-release-pipeline.yml[723-725]
ansible/roles/operator-pipeline/templates/openshift/tasks/get-pyxis-certification-data.yml[19-30]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`log-distribution-skip-notice` references `$(tasks.get-pyxis-certification-data.results.operator_distribution)` from a finally task. If that upstream task doesn’t complete successfully, this finally task’s message can’t reliably be formed/executed.

## Issue Context
The same finally section explicitly warns that dependencies on other task results can prevent final tasks from running when earlier tasks fail.

## Fix Focus Areas
- ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-release-pipeline.yml[688-712]

## Suggested change
Make the finally log task resilient to missing upstream results by:
- Adding a `when` guard requiring `get-pyxis-certification-data` to have completed successfully before consuming its results (and/or gating on the result being `undistributed`).
- Or removing the dynamic echo of `operator_distribution` from this finally task and emitting a simpler notice that does not require upstream results.
This keeps the finally section robust even when earlier tasks fail.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Qodo Logo

@acornett21

Copy link
Copy Markdown
Contributor Author

I forgot to push this to the upstream branch, I'll take comments here and then when I'm back from PTO on Monday, I'll fix and push upstream and raise a new pr.

@acornett21

Copy link
Copy Markdown
Contributor Author

Closing this forked PR in favor of the upstream.

@acornett21 acornett21 closed this Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant