-
Notifications
You must be signed in to change notification settings - Fork 202
Fix #1738: JibBuildConfiguration ignores BuildImageConfiguration.registry #1740
Conversation
@erikgb : Could you please try it out and see if it's working for you use case ? |
@rohanKanojia : This PR does not seem to fix my use case, but it is very hard to verify it 100%. To "integration test" it, I need to push/deploy it to our corporate Maven repository.... But I know how to (Java) debug, and it seems like the registry handling is improved - compared to HEAD of master. But still not bug free. The problem is that there are so many ways to configure registry handling in f-m-p, and when transforming to Jib "dialect" you need to care for all of them. 😄 What I am trying to do at the moment is quite simple. I set the Maven property fabric8.generator.registry to our corporate Docker registry proxy. The expectation is that the base image, fabric8/java-centos-openjdk11-jdk:1.6.3 in my case, is pulled from this proxy Docker registry and not registry-1.docker.io (which is blocked). This does not currently seem to happen:
I think we need tests to do changes to this functionality. It seems like very basic functionality might work, but if you try to use more advanced functionality in f-m-p it just doesn't work. I will try to find some temporary workaround for my issues for now. Probably by avoiding use of the registry handling in f-m-p and only use absolute Docker image references.... So my suggestion is to postpone doing any more related to this, and release f-m-p 4.3.1. I will have more time next week, and can try to do some work on the Jib feature - at least add some tests. I am not comfortable at all doing changes without tests.... |
33df9a1
to
eaab0c5
Compare
@erikgb : Could you please give it a try again? I've added code to handle generator parameters as well. |
@erikgb : I'm merging this and releasing 4.3.1 . |
@rohanKanojia : Sorry for the late response. I was off for the weekend. I will check if version 4.3.1 works in my use case and let you know.... |
This pull request shall port the following PRs: fabric8io/fabric8-maven-plugin#1675 fabric8io/fabric8-maven-plugin#1734 fabric8io/fabric8-maven-plugin#1739 fabric8io/fabric8-maven-plugin#1740
This pull request shall port the following PRs: fabric8io/fabric8-maven-plugin#1675 fabric8io/fabric8-maven-plugin#1734 fabric8io/fabric8-maven-plugin#1739 fabric8io/fabric8-maven-plugin#1740
This pull request shall port the following PRs: fabric8io/fabric8-maven-plugin#1675 fabric8io/fabric8-maven-plugin#1734 fabric8io/fabric8-maven-plugin#1739 fabric8io/fabric8-maven-plugin#1740
Fix #1738