Skip to content
This repository has been archived by the owner on Jun 19, 2024. It is now read-only.

JibBuildConfiguration ignores BuildImageConfiguration.registry #1738

Closed
erikgb opened this issue Oct 17, 2019 · 18 comments · Fixed by #1740
Closed

JibBuildConfiguration ignores BuildImageConfiguration.registry #1738

erikgb opened this issue Oct 17, 2019 · 18 comments · Fixed by #1740
Assignees

Comments

@erikgb
Copy link
Contributor

erikgb commented Oct 17, 2019

Description

JibBuildConfiguration MUST take into account all relevant properties from BuildImageConfiguration.

We do not have access to Docker Hub (registry-1.docker.io) in our CI pipelines, but have to go through a corporate Docker image "proxy". Normally this should be doable with the correct registry configuration in f-m-p. But currently this is not possible for a Jib build because the JibBuildConfiguration created from BuildImageConfiguration seems to ignore a lot of the image configuration in this method:

public static JibBuildService.JibBuildConfiguration getJibBuildConfiguration(BuildService.BuildServiceConfig config, BuildImageConfiguration buildImageConfiguration, String fullImageName, Logger log) throws MojoExecutionException {

This is a blocker that needs to be fixed.

@rohanKanojia @dev-gaur A general (critical) question: Why are you accepting pull request with no, or very little test coverage? In the long run this will definitely discourage developers from using f-m-p. 😏
I feel a bit sad now, as I was looking forward to use the new Java Image Builder support in f-m-p, just realizing that it seems to be full of basic bugs....

@rohanKanojia
Copy link
Member

😞 It was a part of Google Summer of Code. Yeah, we can't just blame that for that much bugs.

@erikgb
Copy link
Contributor Author

erikgb commented Oct 17, 2019

@rohanKanojia : Please do understand me correctly: The Java Image Builder feature is an extremely nice addition to f-m-p. I just think the quality should be better. 😪

@rohanKanojia
Copy link
Member

@erikgb : I really apologize for the inconvenience caused. I agree it is fault on my side. Let me fix all jib issues before cutting release tomorrow. I think we only thought of a basic use case. We didn't consider other use cases 😢 .

@rohanKanojia
Copy link
Member

Could you please provide some more feedback on how this can be made better(for now, for you use case)? I can try to provide a better version of this support in 4.3.1

@erikgb
Copy link
Contributor Author

erikgb commented Oct 17, 2019

@rohanKanojia : Good, thanks! I just hope there isn't more bugs "hiding" down the line. Do you still want me to prepare a PR for #1737, or are you fixing that too now? We risk Git conflicts modifying the same code.....

@rohanKanojia
Copy link
Member

I haven't started working on these yet. But I can try in a hour or two. If you have time, please do 👍 . Otherwise I would fix both of these issues by tonight.

@erikgb
Copy link
Contributor Author

erikgb commented Oct 17, 2019

Of course @rohanKanojia , I can add some general ideas/inputs:

  • There must be some test coverage of this functionality, even if it is just for the basic stuff. At least an integration test?
  • The unit test (JibBuildServiceTest) must be improved to verify some of the more "complex" setups.
  • The code maps ImageConfiguration to another representation (JibBuildService.JibBuildConfiguration), which I think is really scary. Why? JibBuildServiceUtil#buildImage could as well take ImageConfiguration as a parameter. I think the ImageConfiguration instance should be kept until the call into Jib in JibBuildServiceUtil.

@rohanKanojia
Copy link
Member

rohanKanojia commented Oct 17, 2019

I have raised a PR to fix this issue. I tried pushing to quay.io using this, but seems like quay doesn't support container image manifest pushed by current version of jib. But at least, it attempted to push at the correct registry.
This was my plugin configuration:

      <plugin>
        <groupId>io.fabric8</groupId>
        <artifactId>fabric8-maven-plugin</artifactId>
        <version>4.3-SNAPSHOT</version>
        <configuration>
          <images>
            <image>
              <registry>quay.io</registry>
              <name>rohankanojia/helloworld-java:${project.version}</name>
              <alias>hello-world</alias>
              <build>
                <from>openjdk:latest</from>
                <assembly>
                  <descriptorRef>artifact</descriptorRef>
                </assembly>
                <cmd>java -jar maven/${project.name}-${project.version}.jar</cmd>
              </build>
              <run>
                <wait>
                  <log>Hello World!</log>
                </wait>
              </run>
            </image>
          </images>
          <authConfig>
            <username>rohankanojia</username>
            <password>xxxxxxxxxx</password>
          </authConfig>
        </configuration>
      </plugin>

~/work/repos/jib-helloworld-sample : $ mvn fabric8:build -Dfabric8.build.jib=true 
[INFO] Scanning for projects...
[INFO] 
[INFO] ------------< io.fabric8.dmp.samples:dmp-sample-helloworld >------------
[INFO] Building dmp-sample-helloworld 1.1-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- fabric8-maven-plugin:4.3-SNAPSHOT:build (default-cli) @ dmp-sample-helloworld ---
[WARNING] F8: Cannot access cluster for detecting mode: No route to host (Host unreachable)
[INFO] F8: Running in Kubernetes mode
[INFO] F8: Building Container image with JIB(Java Image Builder) in Kubernetes mode
[WARNING] F8: Cannot access cluster for detecting mode: No route to host (Host unreachable)
[WARNING] F8: Cannot access cluster for detecting mode: No route to host (Host unreachable)
[WARNING] F8: Cannot access cluster for detecting mode: No route to host (Host unreachable)
[WARNING] F8: Cannot access cluster for detecting mode: No route to host (Host unreachable)
[INFO] F8: Image tagging successful!
[INFO] F8: The base image requires auth. Trying again for openjdk...
[INFO] F8: Container program arguments set to [jshell] (inherited from base image)
[INFO] F8: Executing tasks:
[INFO] F8: [==============================] 100.0% complete
[WARNING] F8: Registry Exception occurred : Tried to push image manifest for quay.io/rohankanojia/helloworld-java:1.1-SNAPSHOT but failed because: Registry ma
y not support pushing OCI Manifest or Docker Image Manifest Version 2, Schema 2 | If this is a bug, please file an issue at https:/GoogleContainerTools/jib/issues/new
[INFO] F8: Building Image Tarball at rohankanojia/helloworld-java.tar.
[INFO] F8:  /home/rohaan/work/repos/jib-helloworld-sample/target/docker/rohankanojia/helloworld-java.tar successfully built.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  44.672 s
[INFO] Finished at: 2019-10-18T00:24:18+05:30
[INFO] ------------------------------------------------------------------------

I've skipped adding/improving tests for now since I'm not sure how would we verify whether image has been pushed to the registry or not. We might need to hit some API related to that registry.

JIB also has an offline mode(which just builds a tarball of image). But that's right now being used as a fallback option(in case registry is inaccessible). We can maybe do offline builds and verify tarball(not sure about this either).

rohanKanojia added a commit to rohanKanojia/fabric8-maven-plugin that referenced this issue Oct 18, 2019
@rohanKanojia rohanKanojia reopened this Oct 18, 2019
@rohanKanojia
Copy link
Member

I'm keeping this open until we get your use case resolved.

@erikgb
Copy link
Contributor Author

erikgb commented Oct 21, 2019

@rohanKanojia : F-m-p 4.3.1 does not appear to solve my issues on our CI server:

[ERROR] Failed to execute goal io.fabric8:fabric8-maven-plugin:4.3.1:build (fabric8) on project hrls-organization-producer-adapter: Failed to execute the build: java.lang.UnsupportedOperationException: java.lang.IllegalStateException: java.util.concurrent.ExecutionException: java.io.IOException: Cannot run Jib in offline mode; fabric8/java-centos-openjdk11-jdk:1.6.3 not found in local Jib cache -> [Help 1]

I will try to look more closely at these issues later this week. But I still feel that that the Jib feature in f-m-p was released too early, and needs a lot of care (work) to be really usable - except in very basic setups....

@rohanKanojia
Copy link
Member

@erikgb : ah, It seems to be pulling from dockerhub only atm. Let me also try to find some option to configure pull registry.

@erikgb
Copy link
Contributor Author

erikgb commented Oct 21, 2019

Yes, indeed, and that has been the problem since I filed this issue. A user (like me) would expect all the f-m-p registry settings to apply for the Jib feature as well. Currently it doesn't. 😉

@rohanKanojia
Copy link
Member

@erikgb : Do setting up these environment variables help?
https:/GoogleContainerTools/jib/blob/master/docs/faq.md#how-do-i-configure-a-proxy

@erikgb
Copy link
Contributor Author

erikgb commented Oct 21, 2019

@rohanKanojia : It is not a simple proxy that sits between our CI servers and Docker Hub. It is a Docker Repository (smart) proxy: Artifactory. I probably need to set absolute docker images for now. The f-m-p registry settings does not currently work for Jib builds.

@rohanKanojia
Copy link
Member

ah, ok 😞

@erikgb
Copy link
Contributor Author

erikgb commented Oct 26, 2019

@rohanKanojia : This week became a bit more busy than anticipated, but I had a closer look at the newly added Jib feature today. As I see it, the main issue is that the fabric8:build goal does both image build and image push. The fabric8:push goal should push the image. So I opened a Jib issue; GoogleContainerTools/jib#2104, and got immediate response. I will give the suggestion a try, and plan to submit a PR to improve the Jib feature in f-m-p.

@rohanKanojia
Copy link
Member

rohanKanojia commented Oct 27, 2019

I also had the same concern while reviewing this feature(see my comment on #1675) . Unfortunately, I was unable to convince @dev-gaur and @theexplorist to get adapt as per my idea. Right now tarball is built as a fallback option in case the push fails.

While porting this to docker-maven plugin, I separated build and push steps (fabric8io/docker-maven-plugin#1277 (comment))

I would like to separate these two steps in fmp also. Code is already there to build tarball, but that's just not the default way. Happy to modify this if @dev-gaur agrees.

@devang-gaur
Copy link
Contributor

Of course @rohanKanojia , I can add some general ideas/inputs:

  • There must be some test coverage of this functionality, even if it is just for the basic stuff. At least an integration test?
  • The unit test (JibBuildServiceTest) must be improved to verify some of the more "complex" setups.

Totally agree, we need to improve test coverage.

  • The code maps ImageConfiguration to another representation (JibBuildService.JibBuildConfiguration), which I think is really scary. Why? JibBuildServiceUtil#buildImage could as well take ImageConfiguration as a parameter. I think the ImageConfiguration instance should be kept until the call into Jib in JibBuildServiceUtil.

I created a seperate object model for Jib image build to help with abstractions. Even though docker and jib does the same thing here (build images), but the set of parameters they require have a slight differences in names, etc. JibBuildConfiguration object helps in mapping DMP specific parameters to Jib specific parameter set. Its just to ease developer's understanding of things.

And the conversion from ImageConfiguration objects to JibBuildConfiguration objects is confined within JibBuildServiceUtil class.

devang-gaur pushed a commit to devang-gaur/fabric8-maven-plugin that referenced this issue Dec 3, 2019
@rohanKanojia rohanKanojia removed their assignment Dec 18, 2019
devang-gaur pushed a commit to devang-gaur/fabric8-maven-plugin that referenced this issue Jan 30, 2020
devang-gaur pushed a commit to devang-gaur/fabric8-maven-plugin that referenced this issue Feb 5, 2020
devang-gaur pushed a commit to devang-gaur/fabric8-maven-plugin that referenced this issue Feb 14, 2020
devang-gaur pushed a commit to devang-gaur/fabric8-maven-plugin that referenced this issue Feb 18, 2020
devang-gaur pushed a commit to devang-gaur/fabric8-maven-plugin that referenced this issue Feb 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants