Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for mirroring Helm charts with both operator and operand images #1030

Open
pgodowski opened this issue Jan 20, 2025 · 4 comments

Comments

@pgodowski
Copy link

pgodowski commented Jan 20, 2025

As of today, oc-mirror supports mirroring the container images referenced from Helm Charts, as the result of the Helm support initial work: #83

However, when the Helm Chart packages the operator-managed workloads, oc-mirror is mirroring only the operator-controller image, but not the operand images. The reason for it is that oc-mirror does the image reference looking within the rendered Helm YAML manifests, assuming that some of those YAMLs would represent Deployment, Pod, StatefulSet, etc.

In operator-managed workloads, what gets initially deployed via Helm chart though is the operator-controller Deployment/StatefulSet, but the operand is deployed by operator-controller dynamically, after operator's CustomResource reconcilation.

This enhancement request is to provide some convention for Helm chart authors to follow by listing the operand images and then oc-mirror to get the list of such operand images for mirroring purposes.

There are some existing community efforts in this space:

  • helm-images - which can extract image reference from both workload-controller manifests (Deployment, etc), but also from ConfigMap
  • helm-mirror - which extract image reference from within any manifest file, as long as it matches the specific regular expression (like image:).

There are some suggestions for implementation choices:

  • use ConfigMap which has the list of images
  • use of ImageSetConfigration packages within Helm chart, which has the additionalImages section populated with the list of operand images
  • use the images.txt file which was supposed to be a Helm convention, but never materialised, see Support for first-class image definition helm/helm#7754
@pgodowski
Copy link
Author

pgodowski commented Jan 27, 2025

Explored this a little bit more and came up with two tactical approaches which can be done with the existing oc-mirror v2 implementation - those are obviously stop-gap options, yet some of the concepts there could be taken into consideration in the actual design of the final solution.

Tactical option 1 - additionalImages in ImageSetConfiguration

apiVersion: mirror.openshift.io/v1alpha2
kind: ImageSetConfiguration
...
mirror:
 helm:
...
additionalImages:
- name: quay.io/acme/operator-image:1.0
- name: quay.io/acme/operand-primary-image:1.0
- name: quay.io/acme/operand-secondary-image:1.0

Tactical option 2 - additional CR in Helm, listing images

The second option which proved working was to come up with a new CRD, let's call it ImageMirrorList, which has the list of operator/operand images (i.e. ALL images) to be mirrored and the CR in YAML format to be put into Helm, templates directory. Given that the CRD structure is matching the existing JSON path expressions which oc-mirror uses, (i.e. .spec.containers[*].image), oc-mirror v2 successfully is able to add the listed images into the collection of images to mirror.

Sample CR below:

apiVersion: acme.com/v1
kind: ImageMirrorList
metadata:
    name: ibm-test-images-list
spec:
    containers:
        - name: operator-image
          image: quay.io/acme/operator-image:1.0
        - name: operand-primary-image
          image: quay.io/acme/operand-primary-image:1.0
        - name: operand-secondary-image
          image: quay.io/acme/operand-secondary-image:1.0

It has though disadvantage that it introduces the application-specific CRD, whereas if OCP provided such CRD, it would be much cleaner.

@lmzuccarelli @tlwu2013

@aguidirh
Copy link
Contributor

Hi @pgodowski,

Currently oc-mirror v2 has the field imagePaths (example below) where you can specify custom paths where oc-mirror v2 should look for helm charts images. These images will be added to the list of images to be mirrored.

repositories:
  - name: podinfo
    url: https://example.github.io/podinfo
    charts:
      - name: podinfo
        version: 5.0.0
         imagePaths:
         - "{.spec.template.spec.custom[*].image}"

Do you think using this field could be a solution ?

@pgodowski
Copy link
Author

Currently oc-mirror v2 has the field imagePaths (example below) where you can specify custom paths where oc-mirror v2 should look for helm charts images. These images will be added to the list of images to be mirrored.

Thanks for your response @aguidirh.
That is promising, e.g. we could add operand image references as ENV variables in the operator Deployment (which we have to do anyway).

So I tried adding imagePaths into ImageSetConfiguration like below:

kind: ImageSetConfiguration
...
mirror:
 helm:
  local:
    - name: test-mirror-helm
      path: /test/test-mirror-helm-0.3.0.tgz
      imagePaths:
        - "{.spec.template.spec.containers[*].env[*].value}"

and with Helm chart having templates/deployment-test.yaml file like below, oc-mirror v2 picked up the image reference found in ENV variable reference from Deployment

apiVersion: apps/v1
kind: Deployment
metadata:
    name: nginx-ingress
spec:
    replicas: 1
    selector: 
        matchLabels:
            app: nginx-test
    template:
        metadata:
            labels:
                app: nginx-test
        spec:
            containers:
                - name: nginx
                  image: quay.io/nginx/nginx-ingress:latest
                  env:
                  - name: DUMMY_IMAGE
                    value: "quay.io/prometheus/prometheus:latest"
                  ports:
                    - containerPort: 8080
                      protocol: TCP

This is very promising - thanks for sharing this as an option - I didn't find imagePaths override option in docs - do you plan to document this? Also wondering, whether the matching could be a little bit more sophisticated so that only subset of env keys are picked, but will experiment with this a little bit more.

@aguidirh
Copy link
Contributor

Hi @pgodowski ,

Yes, the team is currently adding the information about this field in openshift docs 4.18.

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

No branches or pull requests

2 participants