Skip to content

Comments

acm helm chart to oci image refactor#4185

Open
sclarkso wants to merge 1 commit intomainfrom
acm-helm-image-refactor
Open

acm helm chart to oci image refactor#4185
sclarkso wants to merge 1 commit intomainfrom
acm-helm-image-refactor

Conversation

@sclarkso
Copy link
Collaborator

@sclarkso sclarkso commented Feb 24, 2026

What

Updates acm makefile and update-policy-chart.sh to helm package charts and helm push them to acr repo.
Updates acm pipeline.yaml to consume oci images instead of local charts
Updates the Helm step handler in templatize's pipeline engine to support oci:// chart references.
Adds the 3 chart digests to config.yaml and the necessary additions to config.schema
Updates image-updater with the downstream oci image locatiion in the ACR.
Also had to modify the acr.go to be able to deal with the fact that OCI images don't have arch or OS metadata.

Why

On main today, there's one copy of each chart checked into deploy/helm/ and every environment deploys that same version. If the CI job re-renders charts from a newer operator bundle, everyone gets the new version on the next deploy -- there's no way to pin an environment to a specific chart version.
With the OCI approach, each chart version is a distinct tagged image in ACR with its own digest. Each environment's config.yaml section can reference a different digest, so you could have pers running the latest charts while cspr stays pinned to a known-good version. The same mechanism that already works for container images (per-environment digest overrides in config.yaml) now works for Helm charts too.

Special notes for your reviewer

I have tested in personal-dev. These changes still require the versions of the chart images specified in config.yaml to have been manually pushed to the acr. Image-updater changes in this PR will only grab the latest downstream images that exist, so there shouldn't be a scenario where the images specified in config.yaml don't exist. The plan is to update the github action to add build-and-push for acm, and to remove the manual step from prow ci that currently updates the acm helm charts.

@openshift-ci
Copy link

openshift-ci bot commented Feb 24, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sclarkso
Once this PR has been reviewed and has the lgtm label, please assign janboll for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

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

@openshift-ci
Copy link

openshift-ci bot commented Feb 24, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@sclarkso
Copy link
Collaborator Author

/test e2e-parallel

@sclarkso
Copy link
Collaborator Author

/test all

@sclarkso
Copy link
Collaborator Author

/test e2e-parallel

@sclarkso sclarkso marked this pull request as ready for review February 24, 2026 06:05
@openshift-ci openshift-ci bot requested review from bennerv and geoberle February 24, 2026 06:05
resourceGroup: '{{ .mgmt.rg }}'
subscription: '{{ .mgmt.subscription.key }}'
steps:
- name: deploy-mce-crds
Copy link
Collaborator

Choose a reason for hiding this comment

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

the deploy steps need to depend on the respective image mirror step

releaseName: 'mce-crds'
releaseNamespace: 'multicluster-engine'
chartDir: ./deploy/helm/multicluster-engine-crds
chartDir: oci://{{ .acr.svc.name }}.{{ .acrDNSSuffix }}/helm/multicluster-engine-crds@{{ .acm.chartCrds.digest }}
Copy link
Contributor

Choose a reason for hiding this comment

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

This can't work in Ev2, sorry. You should keep the filesystem contents.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We might be scrapping this anyway, but Ev2 won't use my changes to the templatize helm code to generate the artifact? chartDir has to be a local path?

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.

3 participants