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

DeploymentConfig ImageChange trigger seems to be wrong for custom images #904

Closed
1 of 2 tasks
manusa opened this issue Sep 10, 2021 · 3 comments · Fixed by #911
Closed
1 of 2 tasks

DeploymentConfig ImageChange trigger seems to be wrong for custom images #904

manusa opened this issue Sep 10, 2021 · 3 comments · Fixed by #911
Assignees
Labels
bug Something isn't working
Projects
Milestone

Comments

@manusa
Copy link
Member

manusa commented Sep 10, 2021

Description

The following statements sets the namespace of the change trigger to the image user. This seems to be an issue for custom images which include the repository/user/group information.

https://github.com/eclipse/jkube/blob/682cc6f25ccf3e95e20832d1396fa3890f3a4e26/jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/openshift/ImageChangeTriggerEnricher.java#L98

You can easily reproduce this with quickstarts/maven/quarkus-customized-image.

  • Check if issue is reproducible in Fabric8 Maven Plugin
  • [...]
@manusa manusa added the bug Something isn't working label Sep 10, 2021
@manusa manusa added this to the 1.5.0 milestone Sep 10, 2021
@manusa manusa added this to Planned in Sprint #207 Sep 10, 2021
@rohanKanojia rohanKanojia self-assigned this Sep 13, 2021
@rohanKanojia rohanKanojia moved this from Planned to In progress in Sprint #207 Sep 13, 2021
@rohanKanojia
Copy link
Member

When I was trying to reproduce this issue, Due to image name being set to ${project.groupId}/${project.artifactId} I bumped into #908. I modified image name to %g/${project.artifactId} But noticed a difference in behavior between Fabric8 Maven Plugin and Eclipse JKube.

In Fabric8 Maven Plugin, %g was getting replaced by namespace which was currently configured or picked from kube config. But in case of Eclipse JKube, it was always set to ${project.groupId}.

In ResourceMojo, there is a lateInit method which seems to be setting docker.image.user property when OpenShift platform is detected:

https://github.com/eclipse/jkube/blob/e2aa310a834322d5eca84f802e5830d706c47d21/kubernetes-maven-plugin/plugin/src/main/java/org/eclipse/jkube/maven/plugin/mojo/build/ResourceMojo.java#L81

https://github.com/eclipse/jkube/blob/e2aa310a834322d5eca84f802e5830d706c47d21/kubernetes-maven-plugin/plugin/src/main/java/org/eclipse/jkube/maven/plugin/mojo/build/ResourceMojo.java#L299

This property seems to be read from ImageNameFormatter, note that what's being read is jkube.image.user:

https://github.com/eclipse/jkube/blob/e2aa310a834322d5eca84f802e5830d706c47d21/jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/helper/ImageNameFormatter.java#L93-L100

I think we have a typo in ResourceMojo, we should update this property to match the one being read in ImageNameFormatter. I'll create a PR to fix this.

@rohanKanojia
Copy link
Member

rohanKanojia commented Sep 14, 2021

According to OpenShift documentation, from.namespace field is optional. If we don't set this namespace is assumed to be namespace where DeploymentConfig gets applied.

Screenshot from 2021-09-14 18-27-31

If I remove this namespace set line from ImageChangeTriggerEnricher, I can see that in target/classes/META-INF/jkube/openshift.yml ImageChangeTrigger's from.namespace is no longer set:

    triggers:
    - type: ConfigChange
    - imageChangeParams:
        automatic: true
        containerNames:
        - orgeclipsejkubequickstartsmaven-quarkus-customized-image
        from:
          kind: ImageStreamTag
          name: quarkus-customized-image:latest
      type: ImageChange

But after issuing mvn oc:apply, I can see it in applied DeploymentConfig.

    triggers:
    - type: ConfigChange
    - imageChangeParams:
        automatic: true
        containerNames:
        - orgeclipsejkubequickstartsmaven-quarkus-customized-image
        from:
          kind: ImageStreamTag
          name: quarkus-customized-image:latest
          namespace: rokumar-dev
        lastTriggeredImage: image-registry.openshift-image-registry.svc:5000/rokumar-dev/quarkus-customized-image@sha256:a080942a695f6c29ebe4d2b98d294bdd41124000d97ea7066ff3078c4fe07645
      type: ImageChange

I have tested with both %g as image user and also ${project.groupId}(with a fix to #908) and it seems to be working well.

rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Sep 14, 2021
Right now `%g` placeholder which corresponds to default image user picks
`jkube.image.user` property first, if not then project groupId.

Due to a typo in ResourceMojo for DOCKER_IMAGE_USER property, wrong
property was being set in mojo.

Relates to eclipse-jkube#904

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Sep 14, 2021
…be wrong for custom images

Setting from.namespace field from image.getUser() can be problematic if
it's not equal to the namespace. If it's set to `%g`, it will work since
JKube will evaluate to current configured namespace but if user defines
image name to have a different user then ImageChangeTrigger would be
configured with a non-existent namespace.

Don't set from.namespace field for ImageChangeTrigger while adding
ImageChangeTrigger to DeploymentConfig

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
@manusa manusa moved this from In progress to Review in Sprint #207 Sep 15, 2021
@manusa
Copy link
Member Author

manusa commented Sep 15, 2021

OK, let's remove it then.

If there's a regression we can always set it back since the correct behavior is fixed in #910 (as long as the user is providing %g in the image name repository part).

Updated documentation for 4.8 https://docs.openshift.com/container-platform/4.8/rest_api/workloads_apis/deploymentconfig-apps-openshift-io-v1.html

manusa pushed a commit that referenced this issue Sep 15, 2021
Right now `%g` placeholder which corresponds to default image user picks
`jkube.image.user` property first, if not then project groupId.

Due to a typo in ResourceMojo for DOCKER_IMAGE_USER property, wrong
property was being set in mojo.

Relates to #904

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
Sprint #207 automation moved this from Review to Done Sep 15, 2021
manusa pushed a commit that referenced this issue Sep 15, 2021
…custom images

Setting from.namespace field from image.getUser() can be problematic if
it's not equal to the namespace. If it's set to `%g`, it will work since
JKube will evaluate to current configured namespace but if user defines
image name to have a different user then ImageChangeTrigger would be
configured with a non-existent namespace.

Don't set from.namespace field for ImageChangeTrigger while adding
ImageChangeTrigger to DeploymentConfig

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Sprint #207
  
Done
2 participants