carvel-dev / kapp-controller

Continuous delivery and package management for Kubernetes.
https://carvel.dev/kapp-controller
Apache License 2.0
262 stars 101 forks source link

Support image pull cache for App CR to archieve 'imagePullPolicy: IfNotPresent' #1411

Open jessehu opened 7 months ago

jessehu commented 7 months ago

Describe the problem/challenge you have Hello, is there an option similar to IfNotPresent in K8s for the image url used in fetch ? https://carvel.dev/kapp-controller/docs/v0.48.x/packaging/

  # App template used to create the underlying App CR.
  # See 'App CR Spec' docs for more info
  template:
    spec:
      fetch:
      - imgpkgBundle:
          image: registry.tkg.vmware.run/tkg-fluent-bit@sha256:...

The reason for this feature is:

This is discussed in https://kubernetes.slack.com/archives/CH8KCCKA5/p1700836690956389

Describe the solution you'd like

vendir v0.36.0 supports Only run sync for stable versions, if vendir.yaml has changed since last sync · Issue #278 · carvel-dev/vendir. kapp-controller can utilize it.

Anything else you would like to add: [Additional information that will assist in solving the issue.]


Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible" 👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

github-actions[bot] commented 6 months ago

This issue is being marked as stale due to a long period of inactivity and will be closed in 5 days if there is no response.

praveenrewar commented 6 months ago

@jessehu Are you trying to pull a bundle or an image? We already have caching available for images and bundles, although, I think it doesn't work if the bundle is not colocated. cc @joaopapereira

jessehu commented 6 months ago

I'm pulling an image. It's not cached because kapp-controller reports not able to pull the image when the image registry server goes down.

renuy commented 6 months ago

If the image size is huge, we would prefer not to cache it in kapp-controller as it will increase the size of the kc container. Explore how vendir's functionality can be used here.

100mik commented 6 months ago

We can configure the max size by setting an env in the pod (ref). However, a higher cache max size is not recommended because of the reasons that Renu mentioned.

To make this easier we could start making this config more accessible via Kapp-controllers config, but we should definitely be conscious of the impact this has on the footprint.

jessehu commented 6 months ago

Thank you. The images I need to pull is very small (n KB).

joaopapereira commented 6 months ago

What version of kapp-controller are you using? Starting on version 0.43 kapp-controller by default caches all images as long as they are referenced by a SHA, the size is smaller than 1MB and in case of a bundle if it is colocated or not. Are all the above assumptions true in your case?

jessehu commented 6 months ago

hi @joaopapereira I'm using kapp-controller 0.44.6. Here is the Package CR yaml which uses image tag name not SHA.

apiVersion: data.packaging.carvel.dev/v1alpha1
kind: Package
metadata:
  name: calico.sks.mydomain.com.3.26.3
  namespace: sks-system
spec:
  refName: calico.sks.mydomain.com
  version: 3.26.3
  releaseNotes: https://docs.tigera.io/calico/latest/release-notes/#v3.26.3
  licenses:
  - Apache 2.0
  template:
    spec:
      fetch:
      - image:
          url: registry.mydomain.com/kubesmart/packages/calico:3.26.3
          subPath: manifests
      template:
      - helmTemplate:
          path: charts/tigera-operator
          valuesFrom:
          - path: values.yaml
          - secretRef:
              name: calico-global-data-values
      - ytt:
          paths:
            # "-" is used for getting the output of 'helmTemplate' step as the input for 'ytt' step.
            - "-"
            - ytt
          valuesFrom:
            - path: values.yaml
            - secretRef:
                name: calico-global-data-values
            - secretRef:
                name: calico-data-values
      deploy:
      - kapp:
          rawOptions:
          - --wait-timeout=60s
joaopapereira commented 6 months ago

Since you are using a tag vendir cannot guarantee that the image will be the same, so it will not cache it.

jessehu commented 6 months ago

Got it. Is it reasonable to let vendir cache a image with tag as well (in addition to sha256) ? A tag is immutable.

jessehu commented 6 months ago

Found this PR but it also requires image SHA not tag: Activate caching of images and bundles by joaopapereira · Pull Request #909 · carvel-dev/kapp-controller

joaopapereira commented 6 months ago

The problem is that your particular tag might be immutable, but we cannot guarantee that all tags used in kapp-controller are. Because of this, we need to safeguard the kapp-controller from the generic case. An excellent example is the latest tag all images have in dockerhub.

jessehu commented 6 months ago

Yup, latest tag is not immutable but others should be especially in product env (e.g. docker hub). Or could we have an vendir or kapp-controller option to cache images with tag ?

jessehu commented 5 months ago

Thank you all for the guidance. I managed to make the image fetch cache work by switching to image sha256 and optionally adding the following env var VENDIR_MAX_CACHE_SIZE for kapp-controller-sidecarexec pod:

          - args:
            - --sidecarexec
            env:
            - name: KAPPCTRL_SIDECAREXEC_SOCK
              value: /etc/kappctrl-mem-tmp/sidecarexec.sock
            - name: IMGPKG_ACTIVE_KEYCHAINS
              value: gke,aks,ecr
            - name: VENDIR_MAX_CACHE_SIZE
              value: 5Mi
            image: registry.example.com/project/kapp-controller:v0.44.6
            name: kapp-controller-sidecarexec