kfctl icon indicating copy to clipboard operation
kfctl copied to clipboard

kfctl set-image-name command is not considering newName to update priv registry

Open hemantha-kumara opened this issue 5 years ago • 6 comments

I tried to update one of the component latest container image using manifests kustomization.yaml newName field and executed kfctl set-image-name to update registry field with private registry, set-image-name is not considering newName for udpating registry

Steps are as follow

  1. Updated kustomization.yaml as below(used argo kustomization here)
images:
- name: argoproj/argoui
  newName: argoproj/test/argoui
  1. Executed kfctl set-image-name test.io
  • Actual kustomization.yaml is as below
images:
- name: argoproj/argoui
  newName: test.io/argoproj/argoui
  • Expected kustomization.yaml is as below
images:
- name: argoproj/argoui
  newName: test.io/argoproj/test/argoui

Problem could be with this line where image.Name(instead of NewName) is passed to get newName https://github.com/kubeflow/kfctl/blob/0d5351e897684b02ec79b64ca8ac6f351e2b93d5/cmd/kfctl/cmd/set-image-name.go#L90

https://github.com/kubeflow/kfctl/blob/0d5351e897684b02ec79b64ca8ac6f351e2b93d5/cmd/kfctl/cmd/set-image-name.go#L161-L185

Due to this problem, now need to update kustomization.yaml name & newName fields and also corresponding deployment/statefulset with update container name

hemantha-kumara avatar Feb 13 '20 09:02 hemantha-kumara

Issue-Label Bot is automatically applying the labels:

Label Probability
kind/bug 0.73

Please mark this comment with :thumbsup: or :thumbsdown: to give our bot feedback! Links: app homepage, dashboard and code for this bot.

issue-label-bot[bot] avatar Feb 13 '20 09:02 issue-label-bot[bot]

@xaniasd Can you please check? Due to this issue, we are suppressing the flexibility of kustomization's name/newName fields

hemantha-kumara avatar Feb 13 '20 09:02 hemantha-kumara

/cc @yanniszark

hemantha-kumara avatar Feb 13 '20 09:02 hemantha-kumara

Here are the changes i tested, seems it is working(pls note for testing changes, I have used 0.7.1 code base from kubeflow/kubeflow repo - same changes should work even with kfctl repo code base)

-bash-4.2$ git diff set-image-name.go
diff --git a/bootstrap/cmd/kfctl/cmd/set-image-name.go b/bootstrap/cmd/kfctl/cmd/set-image-name.go
index b384f9f7..b6b13a9f 100644
--- a/bootstrap/cmd/kfctl/cmd/set-image-name.go
+++ b/bootstrap/cmd/kfctl/cmd/set-image-name.go
@@ -82,13 +82,13 @@ The flatten flag discards both registry and name components except for the last
                                if _, err := os.Stat(kustomizationFilePath); err == nil {
                                        kustomization := kustomize.GetKustomization(absPath)
                                        for i, image := range kustomization.Images {
-                                               if !strings.HasPrefix(image.Name, filter) {
-                                                       log.Infof("No filter match for %s, skipping\n", image.Name)
+                                               if !strings.HasPrefix(image.NewName, filter) {
+                                                       log.Infof("No filter match for %s, skipping\n", image.NewName)
                                                        continue
                                                }

-                                               newName := setImageName(image.Name, newNameComponents)
-                                               log.Infof("Replacing image name from %s to %s", image.Name, newName)
+                                               newName := setImageName(image.NewName, newNameComponents)
+                                               log.Infof("Replacing image name from %s to %s", image.NewName, newName)
                                                kustomization.Images[i].NewName = newName
                                                fmt.Printf("%s=%s\n", imageToString(image), imageToString(kustomization.Images[i]))
                                        }

hemantha-kumara avatar Feb 13 '20 12:02 hemantha-kumara

/priority p1

jtfogarty avatar Apr 17 '20 21:04 jtfogarty

The same issue https://github.com/kubeflow/manifests/issues/1850 I wasn't able to install kubeflow directly in air-gapped environment, because --set-image-name doesn't work correctly. I faced three types of issue:

  1. It doesn't handle image names written in configmaps and other custom resources
  2. When it creates kustomize manifests, they usually work as intended, but I found several images that's names were not modified during install. I believe it is connected to how kustomize handles newTag, newImage keys and probably the full digests of images (like gcr.io/knative-releases/knative.dev/net-istio/cmd/webhook@sha256:e6b142c0f82e0e0b8cb670c11eb4eef6ded827f98761bbf4bea7bdb777b80092) breaks this functionality
  3. There are several images names written in params.env files and they are not processed with --set-image-name flag.

So... what resolution do I see? I see that we need to standardize all source manifests in a way that changing of images names will be possible and easy.

Right now I resolved my issue by monkey-patching of the source manifests, but it is not scalable and/or maintainable solution

gecube avatar Apr 17 '21 09:04 gecube