infra/ansible

Fork 57

[flatpak-indexer] Separate quay.io instance #3453

Merged

zlopez merged 1 commit from zlopez/ansible:11543 into main

2026-06-30 11:20:38 +00:00

zlopez commented

2026-06-29 15:12:15 +00:00

Owner

This is another try to separate the quay.io instance for
flatpak-indexer.
See more info in infra/tickets#11543

Signed-off-by: Michal Konecny mkonecny@redhat.com

This is another try to separate the quay.io instance for flatpak-indexer. See more info in https://forge.fedoraproject.org/infra/tickets/issues/11543 Signed-off-by: Michal Konecny <mkonecny@redhat.com>

zlopez added 1 commit

2026-06-29 15:12:16 +00:00

[flatpak-indexer] Separate quay.io instance

Linter / yamllint (pull_request) Successful in 29s

Details

Linter / ansible-lint (pull_request) Successful in 1m14s

Details

AI Code Review / ai-review (pull_request_target) Successful in 36s

Details

b9a7675c47

This is another try to separate the quay.io instance for
flatpak-indexer.
See more info in infra/tickets#11543

Signed-off-by: Michal Konecny <mkonecny@redhat.com>

zlopez added the

ai-review-please

label

2026-06-29 15:16:38 +00:00

forgejo-actions commented

2026-06-29 15:17:15 +00:00

AI Code Review

Detailed Code Review

The changes introduce dedicated deployments and configurations for the quay.io instance of flatpak-indexer, achieving the intended separation. The configuration and structure align well with typical OpenShift deployment patterns.

However, there are critical issues in the OpenShift deployment definitions regarding automatic image triggers. The container names specified in the image.openshift.io/triggers annotations do not match the newly defined container names, which will silently break OpenShift ImageStream auto-deployments for these new workloads. Additionally, there is a risk regarding how the new YAML configuration file is included in the ConfigMap, as it relies on Jinja syntax that may not evaluate correctly depending on how load_file operates within the environment.

📂 File Reviews

📄 `roles/openshift-apps/flatpak-indexer/templates/deployment.yml.j2` - Adds two new deployments for `flatpak-quay-indexer` and `flatpak-indexer-quay-differ` with their corresponding volumes and mounts.

Critical [Bug]: The container name specified in the image.openshift.io/triggers annotation (flatpak-indexer) does not match the actual container name (flatpak-quay-indexer). This mismatch will cause OpenShift to fail to automatically update the container image when a new tag is pushed.
- Suggestion: Update the annotation field path to reference the correct container name.

  annotations:
    image.openshift.io/triggers: '[{"from":{"kind":"ImageStreamTag","name":"flatpak-indexer:latest"},"fieldPath":"spec.template.spec.containers[?(@.name==\"flatpak-quay-indexer\")].image"}]'

Critical [Bug]: Similar to the indexer deployment, the differ deployment annotation references flatpak-indexer-differ but the container is named flatpak-indexer-quay-differ. The image trigger will fail to locate the container.
- Suggestion: Update the annotation field path to reference the correct differ container name.

      annotations:
        image.openshift.io/triggers: '[{"from":{"kind":"ImageStreamTag","name":"flatpak-indexer:latest"},"fieldPath":"spec.template.spec.containers[?(@.name==\"flatpak-indexer-quay-differ\")].image"}]'

📄 `roles/openshift-apps/flatpak-indexer/templates/configmap.yml.j2` - Introduces a new ConfigMap `indexer-quay-config` using content from `config.quay.yaml`.

Major [Logic]: The config file is included using load_file(). If load_file() only reads the raw file content, the Jinja variables inside config.quay.yaml (such as {% if env == 'staging' %} and loops) will NOT be processed and will be injected directly as text, corrupting the YAML. Ensure that the inclusion method evaluates Jinja inside the imported file.
- Suggestion: If load_file does not evaluate Jinja, use Ansible's standard lookup('template', ...) to render it before inclusion.

  config.yaml: |-
    {{ lookup('template', 'config.quay.yaml') | indent(4) }}

Questions:

❓ Does load_file() in your Ansible environment evaluate Jinja expressions inside the file being read?

✅ Summary

Overall Assessment: The architectural approach is sound, but critical bugs in the OpenShift image trigger annotations and a potential templating logic issue must be resolved before merging.
Priority Issues:
- Fix container name mismatches in the image.openshift.io/triggers annotations for both new Deployments.
- Verify that the inclusion method for config.quay.yaml correctly processes Jinja templates rather than importing them as raw strings.

🤖 AI Code Review | Generated with ai-code-review | Model: gemini-3.1-pro-preview

⚠️ AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.

## AI Code Review ### Detailed Code Review The changes introduce dedicated deployments and configurations for the quay.io instance of `flatpak-indexer`, achieving the intended separation. The configuration and structure align well with typical OpenShift deployment patterns. However, there are critical issues in the OpenShift deployment definitions regarding automatic image triggers. The container names specified in the `image.openshift.io/triggers` annotations do not match the newly defined container names, which will silently break OpenShift ImageStream auto-deployments for these new workloads. Additionally, there is a risk regarding how the new YAML configuration file is included in the ConfigMap, as it relies on Jinja syntax that may not evaluate correctly depending on how `load_file` operates within the environment. #### 📂 File Reviews <details> <summary><strong>📄 `roles/openshift-apps/flatpak-indexer/templates/deployment.yml.j2`</strong> - Adds two new deployments for `flatpak-quay-indexer` and `flatpak-indexer-quay-differ` with their corresponding volumes and mounts.</summary> - **Critical** [Bug]: The container name specified in the `image.openshift.io/triggers` annotation (`flatpak-indexer`) does not match the actual container name (`flatpak-quay-indexer`). This mismatch will cause OpenShift to fail to automatically update the container image when a new tag is pushed. - *Suggestion:* Update the annotation field path to reference the correct container name. ```` annotations: image.openshift.io/triggers: '[{"from":{"kind":"ImageStreamTag","name":"flatpak-indexer:latest"},"fieldPath":"spec.template.spec.containers[?(@.name==\"flatpak-quay-indexer\")].image"}]' ```` - **Critical** [Bug]: Similar to the indexer deployment, the differ deployment annotation references `flatpak-indexer-differ` but the container is named `flatpak-indexer-quay-differ`. The image trigger will fail to locate the container. - *Suggestion:* Update the annotation field path to reference the correct differ container name. ```` annotations: image.openshift.io/triggers: '[{"from":{"kind":"ImageStreamTag","name":"flatpak-indexer:latest"},"fieldPath":"spec.template.spec.containers[?(@.name==\"flatpak-indexer-quay-differ\")].image"}]' ```` </details> <details> <summary><strong>📄 `roles/openshift-apps/flatpak-indexer/templates/configmap.yml.j2`</strong> - Introduces a new ConfigMap `indexer-quay-config` using content from `config.quay.yaml`.</summary> - **Major** [Logic]: The config file is included using `load_file()`. If `load_file()` only reads the raw file content, the Jinja variables inside `config.quay.yaml` (such as `{% if env == 'staging' %}` and loops) will NOT be processed and will be injected directly as text, corrupting the YAML. Ensure that the inclusion method evaluates Jinja inside the imported file. - *Suggestion:* If `load_file` does not evaluate Jinja, use Ansible's standard `lookup('template', ...)` to render it before inclusion. ```` config.yaml: |- {{ lookup('template', 'config.quay.yaml') | indent(4) }} ```` **Questions:** - ❓ Does `load_file()` in your Ansible environment evaluate Jinja expressions inside the file being read? </details> ### ✅ Summary - **Overall Assessment:** The architectural approach is sound, but critical bugs in the OpenShift image trigger annotations and a potential templating logic issue must be resolved before merging. - **Priority Issues:** - Fix container name mismatches in the `image.openshift.io/triggers` annotations for both new Deployments. - Verify that the inclusion method for `config.quay.yaml` correctly processes Jinja templates rather than importing them as raw strings. --- 🤖 **AI Code Review** | Generated with [ai-code-review](https://gitlab.com/redhat/edge/ci-cd/ai-code-review) | **Model:** `gemini-3.1-pro-preview` ⚠️ *AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.*

zlopez force-pushed 11543 from b9a7675c47

Linter / yamllint (pull_request) Successful in 29s

Details

Linter / ansible-lint (pull_request) Successful in 1m14s

Details

AI Code Review / ai-review (pull_request_target) Successful in 36s

Details

to 3f29b12df6

Linter / yamllint (pull_request) Successful in 28s

Details

Linter / ansible-lint (pull_request) Successful in 1m32s

Details

2026-06-29 15:31:01 +00:00

Compare

zlopez merged commit f594173ae1 into main

2026-06-30 11:20:38 +00:00

No reviewers

No milestone

No project

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

infra/ansible!3453

No description provided.

Rows
Columns