Jkube: OpenShift Maven Plugin doesn't add ImageChange triggers when merging from a deployment fragment

[rohanKanojia]

Description

Eclipse JKube is not adding ImageChange triggers to DeploymentConfig due to which applications deployed in openshift are failing with ImagePullErr. This seems to happen in a case when project has some deployment fragment in src/main/jkube directory. I have to explicitly add jkube.openshift.enrichAllWithImageChangeTrigger property in order to add ImageChange triggers:

    triggers:
    - type: ConfigChange
    - imageChangeParams:
        automatic: true
        containerNames:
        - spring-boot
        from:
          kind: ImageStreamTag
          name: spring-boot-camel:latest
      type: ImageChange

Same project works fine without any extra configuration with Fabric8 Maven Plugin

Info

• Eclipse JKube version : 1.0.0-rc-1
• Maven version (mvn -v) :

Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
Maven home: /opt/apache-maven
Java version: 1.8.0_251, vendor: Oracle Corporation, runtime: /usr/java/jdk1.8.0_251-amd64/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "5.6.19-300.fc32.x86_64", arch: "amd64", family: "unix"

• Kubernetes / Red Hat OpenShift setup and version : OpenShift 4.5.1

• If it's a bug, how to reproduce :

• Clone https://github.com/rohanKanojia/spring-boot-camel
• Disable jkube.openshift.enrichAllWithImageChangeTrigger property
• deploy application using mvn clean install -DskipTests oc:deploy -Popenshift

• application would fail to deploy with ImagePullErr

• If it's a feature request, what is your use case :

• Sample Reproducer Project : https://github.com/rohanKanojia/spring-boot-camel

[manusa]

Relates to: #312

[manusa]

The behavior in fabric8-maven-plugin is the opposite to the one in JKube as defined in the docs.

Fabric8:

JKube:

In a nutshell, JKube behaves as expected and this is not an issue. Maybe we need to rethink the current default behavior, or at least find out why was this changed.

[rohanKanojia]

I didn't realize I was testing this on Fabric8 Maven Plugin 4.1.0, this behavior is the same as in Fabric8 Maven Plugin since 4.2.0. It's broken in both FMP and JKube.

[manusa]

So what changed?

[rohanKanojia]

I think my refactor on fmp related to sidecars caused this issue: https://github.com/fabric8io/fabric8-maven-plugin/commit/947f32b849438e61c1e908297485229a59a4c0cf

[manusa]

Can't really see where:

Apparently that line should evaluate to false by default before and after.

[rohanKanojia]

In order to add ImageChange triggers we check IMAGECHANGE_TRIGGER processing instruction which is set during resource goal by controller enrichers.
https://github.com/eclipse/jkube/blob/89f36498ab7e2dcc3d7d233e0ae919f4f05fe451/jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/ControllerViaPluginConfigurationEnricher.java#L123-L128

This is set only when defaultApplicationContainerName which is returned after merging resource fragment with JKube's opinionated PodSpec. However, it is being returned as null due to which IMAGECHANGE_TRIGGER has nothing inside it. This is why isImageChangeTriggerNeeded() method returns false:

https://github.com/eclipse/jkube/blob/89f36498ab7e2dcc3d7d233e0ae919f4f05fe451/jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/openshift/ImageChangeTriggerEnricher.java#L121-L128

This seems to be due to KubernetesResourceUtil expecting container name to be present inside resource fragment:
https://github.com/eclipse/jkube/blob/89f36498ab7e2dcc3d7d233e0ae919f4f05fe451/jkube-kit/enricher/api/src/main/java/org/eclipse/jkube/kit/enricher/api/util/KubernetesResourceUtil.java#L677-L682

[manusa]

Gosh I love pasta but really hate spaghetti.

So if you set a container name in the deployment fragment, the change ImageChangeTrigger is added?

[rohanKanojia]

Yes, you're right. If container name is present there. It works. If we modify code in KubernetesResourceUtil to something like this(adding a case when fragment container name is null), it works:

                        if (defaultApplicationContainerName == null) {
                            if (container.getName() != null) { // Pick from fragment
                                defaultApplicationContainerName = container.getName();
                            } else if (defaultName != null) { // Pick from defaultName
                                defaultApplicationContainerName = defaultName;
                            }
                        } 

[manusa]

That class needs a deep refactor.
Please proceed as you may, but at least create 3 tests (1 of them must be failing) before implementing your fix.
We really need to revisit that class. Cyclomatic complexity is terrible (77) right now, probably will raise to 83 after the fix...

[rmarting]

I am getting this issue with the latest published openshift-maven-plugin-1.0.0-rc-1 version. It is seems that this issue was fixed but I don't know if there is available a new version. How could I get it to fix my local environment?

[manusa]

No new version has been released.

Release 1.0.0 is scheduled for 2020-09-09.

In the meantime, you can either build master yourself or use jitpack:

    <repositories>
        <repository>
            <id>jitpack.io</id>
            <url>https://jitpack.io</url>
        </repository>
    </repositories>

    <dependency>
        <groupId>com.github.eclipse.jkube</groupId>
        <artifactId>kubernetes-maven-plugin</artifactId>
        <version>master</version>
    </dependency>