kpt icon indicating copy to clipboard operation
kpt copied to clipboard

`set-image` function is optimized for `out-of-place` and found it practically unusable for`in-place` mode

Open droot opened this issue 2 years ago • 7 comments

set-image function is optimized for out-of-place and found it practically unusable forin-place mode. With a few tweaks, UX can be enhanced significantly.

I am writing a kpt package for backstage-ui-plugin (aka CaD UI). This package contains one deployment resource (so only one resource that runs a container image). Here is what deployment looks like:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: backstage
  namespace: example
  labels:
    app.kubernetes.io/name: backstage
spec:
  replicas: 1
  template:
    spec:
      containers:
        - name: backstage
          image: gcr.io/kpt-dev/kpt-backstage-plugins/backstage-plugin-cad:v0.1.0
...
...

Note that the image is gcr.io/kpt-dev/kpt-backstage-plugins/backstage-plugin-cad:v0.1.0.

On day0, I create a deployable instance of this package and I am happy with it.

On day 2, This package is updated upstream by the CaD UI team and new image version is available, say v0.2.0. Now, as a user, I want to update the image version.

As per current docs, updating the image version requires me to run following command:

kpt fn eval -i set-image:v0.1.0 -- name=<> --newTag=<>

Recording my friction points:

  1. I initially thought name to be container name because if you look at image is specified in the container section in pod template and name means the container name.
  2. name is expected to be current image name. In order to get the current image, I had to open the deployment.yaml. If I end up opening the file resource file, then might as well edit the image version right there :) so it defeats the whole point of having that function.
  3. The camelCase command line args newTag and newName were not inline with rest of the command line argument format.

So, here is how we can make it nicer to operate in in-place mode:

Thinking from user's perspective, assume they discover or know the tag before hand ( I know there is separate issue about discovering the new version and updating the tag automatically, but that's not the point here). User just want to apply new tag to the images referenced in the package.

  1. If I have just one reference of image in workload types resources (like in my case), user should be able to update the new tag by just running following command:

kpt fn eval -i set-image:v0.1.0 -- tag=<new-tag>

  1. If user has more than one image referenced in workload types, we should use name argument to match the container they want to update the image of. For ex, backstage is the container name here, so command will look like:

# just update the tag for image specified for container name backstage
kpt fn eval set-image:v0.1.0 -- name=backstage tag=<new-tag>
  1. If user wants to update the image and tag for a given container, the command will look like:

kpt fn eval set-image:v0.1.0 -- name=backstage image=<new-image> tag=<new-tag>
  1. We have two edge cases, what if we have two containers with same name or what if container name is not specified ?. For such edge cases, function target selector can be used to narrow down to the KRM object. For example,
kpt fn eval set-image:v0.1.0 --match-name <my-workload-name> -- tag=<new-tag>

Some more notes:

  1. Running fn eval with non-existing image should tell me couldn't find image with this name.

 kpt fn eval -i set-image:v0.1.0 -- name=backstage newTag=v0.2.0
[RUNNING] "gcr.io/kpt-fn/set-image:v0.1.0"
[PASS] "gcr.io/kpt-fn/set-image:v0.1.0" in 1.1s
  Results:
    [info]: no images changed
  
  1. Latest version v0.1.1 seems to be not working correctly.

droot avatar Aug 06 '22 00:08 droot

/cc @ChristopherFry @bgrant0607

droot avatar Aug 06 '22 00:08 droot

The original issue was #2577, which lists some examples of tools that support similar operations.

I would also look at Flux and ArgoCD image updaters.

I would expect that the new tag and/or digest would be discovered by polling a registry, or would be known in a CI pipeline. Either way, once that information is known, something needs to inject to the proper location during CD without human editing.

My current thinking on promotion across environments (#3280) is to update a multi-environment blueprint, then update the deployment package for each environment. That would be similar to kustomize with a remote base and each environment kustomization.yaml pinned to a specific commit or tag.

bgrant0607 avatar Aug 06 '22 00:08 bgrant0607

In the example, is "backstage" the container name or the workload name? I would normally not recommend the container name to match the resource name, as the latter is often customized but the former generally doesn't. Also, in the case of an app with sidecar containers or a generic app blueprint the workload container may have a generic name like "app", "main", "primary", etc.

bgrant0607 avatar Aug 06 '22 00:08 bgrant0607

The original issue was https://github.com/GoogleContainerTools/kpt/issues/2577, which lists some examples of tools that support similar operations.

Ack. Will take a quick look.

Either way, once that information is known, something needs to inject to the proper location during CD without human editing.

Yes, that's what I am planning to use set-image for.

My current thinking on promotion across environments (https://github.com/GoogleContainerTools/kpt/issues/3280) is to update a multi-environment blueprint, then update the deployment package for each environment. That would be similar to kustomize with a remote base and each environment kustomization.yaml pinned to a specific commit or tag.

Yes, this is the pattern I will follow. Anything I would like to broadcast in all the environments should be done through the upstream multi-environment blueprint (sort of fits the pattern of a bulk operation on the multi-environment blueprint).

In the example, is "backstage" the container name or the workload name? I would normally not recommend the container name to match the resource name, as the latter is often customized but the former generally doesn't. Also, in the case of an app with sidecar containers or a generic app blueprint the workload container may have a generic name like "app", "main", "primary", etc.

My bad, example is using backstage for both the container and workload name [I will update the package and this example to remove the confusion]. They shouldn't be the same. --match-name filter is applied on the resource name (implemented in kpt fn) and I am proposing a --name argument for set-image function.

droot avatar Aug 08 '22 19:08 droot

I think it's a fair point that set-image should accept Container .name as the matcher besides Container .image.

"4. ...or what if container name is not specified" --> Container .name is mandatory code. "what if we have two containers with same name " I'm not sure about this. But using target selector is a good idea.

Eventually, set-image should be able to update the new 'image' from a variety of resources: I think this requires non KRM resource support(e.g. read image from CI environment or from a CI config), and having fn-config read values from dynamic input resources. Similar requests have been asked by Slack users. The related umbrella issues are #3396 #2528

yuwenma avatar Aug 08 '22 19:08 yuwenma

There's also: https://kubernetes.io/docs/reference/labels-annotations-taints/#kubectl-kubernetes-io-default-container

bgrant0607 avatar Aug 08 '22 21:08 bgrant0607

There's also: https://kubernetes.io/docs/reference/labels-annotations-taints/#kubectl-kubernetes-io-default-container

Oh..this is interesting. Didn't know such a thing existed. So the default container annotation (if exists) can also be used to select the container in case of ambiguity.

On a different note: kpt pkg analyze can propose editing suggestions such as default-container for a package because most of the real deployment will have more than one container (logging/mesh sidecar etc.) and having a default container annotation can save so much time during the kubectl logs...etc.

droot avatar Aug 08 '22 22:08 droot

A common use case is also to simply change the registry name. For example, if I use a private registry and push all my approved images there, then I want all image fields to be updated with just the new registry - or perhaps registry+directory, something like:

artifacts.bigco.com/bigco-productA/someimage:v4.2 -> my-registry.example.com/bigco/bigco-productA/someimage:v4.2
artifacts.bigco.com/bigco-productA/anotherimage:v4.2 -> my-registry.example.com/bigco/bigco-productA/anotherimage:v4.2
artifacts.bigco.com/bigco-productB/someimage:v3.1.1 -> my-registry.example.com/bigco/bigco-productB/someimage:v3.1.1

# also support implicit "docker" registry perhaps
nginx/nginx:v1 -> my-registry.example.com/docker/nginx/nginx:v1
coredns/coredns:v1.9.3 -> my-registry.example.com/docker/coredns/coredns/v1.9.3

The idea being that I should be able to change the registry without changing the image and tag.

johnbelamaric avatar Dec 21 '22 18:12 johnbelamaric

This work is partially done, then we realized some more code refactoring is needed to use the new kpt-fn-sdk.

I don't have short term plan to work on it.

yuwenma avatar Jan 26 '23 18:01 yuwenma