Skip to content

ci: Simplify the test job by removing the matrix strategy#277

Merged
rm3l merged 1 commit intoredhat-developer:mainfrom
rm3l:ci/remove_matrix_strategy_and_test_the_default_tag_from_the_branch
Dec 5, 2025
Merged

ci: Simplify the test job by removing the matrix strategy#277
rm3l merged 1 commit intoredhat-developer:mainfrom
rm3l:ci/remove_matrix_strategy_and_test_the_default_tag_from_the_branch

Conversation

@rm3l
Copy link
Copy Markdown
Member

@rm3l rm3l commented Dec 5, 2025

Description of the change

We should test the default image tag that is set in the backstage Chart's values file

Which issue(s) does this PR fix or relate to

How to test changes / Special notes to the reviewer

Checklist

  • For each Chart updated, version bumped in the corresponding Chart.yaml according to Semantic Versioning.
  • For each Chart updated, variables are documented in the values.yaml and added to the corresponding README.md. The pre-commit utility can be used to generate the necessary content. Use pre-commit run -a to apply changes. The pre-commit Workflow will do this automatically for you if needed.
  • JSON Schema template updated and re-generated the raw schema via the pre-commit hook.
  • Tests pass using the Chart Testing tool and the ct lint command.
  • If you updated the orchestrator-infra chart, make sure the versions of the Knative CRDs are aligned with the versions of the CRDs installed by the OpenShift Serverless operators declared in the values.yaml file. See Installing Knative Eventing and Knative Serving CRDs for more details.

We should test the default image tag that is set in the backstage Chart's values file
@rhdh-qodo-merge
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Naming Consistency

The job name was hardcoded to 'Test Latest Release'. Confirm this still matches external CI expectations that previously relied on matrix-based dynamic names and that any dashboards or filters depending on version-specific names are updated accordingly.

name: Test Latest Release
runs-on: ubuntu-latest
📚 Focus areas based on broader codebase context

Test Image Tag

The workflow no longer sets upstream.backstage.image.tag, relying on the chart’s default. Verify that CI indeed uses the desired test image tag and not latest unintentionally. If CI should differ from defaults, reintroduce an override or a CI-specific values file. (Ref 1, Ref 2)

- name: Run chart-testing (install)
  if: steps.list-changed.outputs.changed == 'true'
  run: |
    ct install \
      --debug \
      --config ct-install.yaml \
      --upgrade \
      --target-branch "${{ github.event.pull_request.base.ref }}" \
      --helm-extra-set-args="--set upstream.ingress.enabled=true --set global.host=rhdh.127.0.0.1.sslip.io"

Reference reasoning: The chart’s default test image tag is defined in values as latest, while CI previously overrode tags explicitly and there exists a CI values file that sets a specific test image/tag. This indicates CI often relies on explicit image/tag settings; removing the override may change behavior to the default.

📄 References
  1. redhat-developer/rhdh-chart/charts/backstage/values.yaml [339-357]
  2. redhat-developer/rhdh-chart/charts/backstage/ci/with-custom-image-for-test-pod-values.yaml [1-15]
  3. redhat-developer/rhdh-chart/charts/backstage/ci/with-test-pod-disabled-values.yaml [1-12]
  4. redhat-developer/rhdh-chart/charts/backstage/ci/with-orchestrator-and-dynamic-plugins-npmrc-values.yaml [1-24]
  5. redhat-developer/rhdh-chart/charts/orchestrator-infra/values.yaml [37-41]
  6. redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/values.yaml [141-163]
  7. redhat-developer/rhdh-chart/charts/backstage/ci/with-orchestrator-values.yaml [1-21]
  8. redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/ci/upstream-values.yaml [1-27]

@rhdh-qodo-merge rhdh-qodo-merge Bot added the enhancement New feature or request label Dec 5, 2025
@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge Bot commented Dec 5, 2025

PR Type

(Describe updated until commit f74c142)

Enhancement


Description

  • Removes matrix strategy from test job, simplifying CI workflow

  • Tests only default image tag from backstage Chart values

  • Eliminates separate testing for 'latest' and 'next' versions

  • Removes experimental flag and continue-on-error configuration


File Walkthrough

Relevant files
Configuration changes
test.yaml
Remove matrix strategy and use default image tag                 

.github/workflows/test.yaml

  • Removed strategy matrix that tested multiple image versions ('latest'
    and 'next')
  • Simplified job name from dynamic matrix-based to static "Test Latest
    Release"
  • Removed continue-on-error flag that was tied to experimental matrix
    variable
  • Removed --set upstream.backstage.image.tag=${{ matrix.version }} from
    helm-extra-set-args to use default tag from values
+2/-16   

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Dec 5, 2025

@rhdh-qodo-merge
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Explicitly set the image tag

The upstream.backstage.image.tag is no longer being set due to the removal of
the matrix strategy. Explicitly set this tag to latest in the
--helm-extra-set-args to ensure the CI job tests the intended version.

.github/workflows/test.yaml [150]

---helm-extra-set-args="--set upstream.ingress.enabled=true --set global.host=rhdh.127.0.0.1.sslip.io"
+--helm-extra-set-args="--set upstream.backstage.image.tag=latest --set upstream.ingress.enabled=true --set global.host=rhdh.127.0.0.1.sslip.io"
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that removing the matrix strategy also removed the upstream.backstage.image.tag setting, which would cause the test to run against a default or incorrect version, defeating the purpose of the "Test Latest Release" job.

High
  • More

@rm3l rm3l marked this pull request as ready for review December 5, 2025 11:25
@openshift-ci openshift-ci Bot requested review from gazarenkov and zdrapela December 5, 2025 11:25
@rm3l rm3l changed the title chore(ci): Simplify the test job by removing the matrix strategy ci: Simplify the test job by removing the matrix strategy Dec 5, 2025
@rhdh-qodo-merge
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Behavior Change

Removing the matrix drops testing against the 'next' image and experimental path. Confirm that coverage is still sufficient and that losing the 'continue-on-error' behavior for experimental runs is intentional.

test-chart:
  # Aligning job name with the OpenShift CI config: https://github.com/openshift/release/blob/master/core-services/prow/02_config/redhat-developer/rhdh-chart/_prowconfig.yaml#L18
  name: Test Latest Release
  runs-on: ubuntu-latest

  steps:
    - name: Checkout
📚 Focus areas based on broader codebase context

Test Image Tag Source

The workflow now removes the matrix that set upstream.backstage.image.tag and uses the default tag from chart values. Verify the default test image/tag aligns with CI’s intended image, especially for the test pod and backstage image. If CI requires a specific tag, pass it explicitly to avoid drifting from the values file. (Ref 1, Ref 2)

name: Test Latest Release
runs-on: ubuntu-latest

Reference reasoning: The charts define default test pod images/tags in values files, e.g., charts/backstage/values.yaml sets test.image.tag: latest, while CI overrides exist in charts/backstage/ci/with-custom-image-for-test-pod-values.yaml to pin a specific tag. Removing explicit tag injection in the workflow risks using an unintended default rather than the CI-pinned versions.

📄 References
  1. redhat-developer/rhdh-chart/charts/backstage/values.yaml [339-357]
  2. redhat-developer/rhdh-chart/charts/backstage/ci/with-custom-image-for-test-pod-values.yaml [1-15]
  3. redhat-developer/rhdh-chart/charts/backstage/ci/with-test-pod-disabled-values.yaml [1-12]
  4. redhat-developer/rhdh-chart/charts/orchestrator-infra/values.yaml [37-41]
  5. redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/values.yaml [141-163]
  6. redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/ci/upstream-values.yaml [1-27]
  7. redhat-developer/rhdh-chart/charts/backstage/ci/with-orchestrator-values.yaml [1-21]
  8. redhat-developer/rhdh-chart/charts/orchestrator-software-templates/templates/tekton/tekton-tasks.yaml [117-151]

@rm3l rm3l merged commit d4f58d7 into redhat-developer:main Dec 5, 2025
6 of 7 checks passed
@rm3l rm3l deleted the ci/remove_matrix_strategy_and_test_the_default_tag_from_the_branch branch December 5, 2025 11:26
@rhdh-qodo-merge
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Explicitly set the Helm image tag

Explicitly set the upstream.backstage.image.tag to latest in the ct install
command. This ensures the test validates the correct image, as this setting was
removed along with the matrix strategy.

.github/workflows/test.yaml [150]

---helm-extra-set-args="--set upstream.ingress.enabled=true --set global.host=rhdh.127.0.0.1.sslip.io"
+--helm-extra-set-args="--set upstream.backstage.image.tag=latest --set upstream.ingress.enabled=true --set global.host=rhdh.127.0.0.1.sslip.io"
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that removing the matrix strategy also removed the setting of upstream.backstage.image.tag, which could cause the test to run against an unintended image version, thus compromising the test's validity.

High
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant