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

Support for JIB(Java Image Builder) mode in dmp #1277

Merged
merged 3 commits into from
Sep 13, 2020

Conversation

rohanKanojia
Copy link
Member

@rohanKanojia rohanKanojia commented Oct 12, 2019

This ports Jib support added in Eclipse JKube in eclipse-jkube/jkube#257

Copy link
Collaborator

@rhuss rhuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot, @rohanKanojia ! That's awesome ;-)

Unfortunately I'm on the road this week, so probably can't review it this week.

One thing I remarked is that docker:build with JIB pushes to a registry like docker:push would do. I know that's how JIB works normally, but maybe we can still keep that phases seperate ? I.e. docker:build would create the tar file only and docker:push would move it to the registry ?

@rohanKanojia
Copy link
Member Author

@rhuss : Yes, I think this is possible and I would also like to have behavior like this. JIB has an offline mode; we can force offline mode during build phase and regular build during push phase.

@rhuss
Copy link
Collaborator

rhuss commented Oct 16, 2019

@rohanKanojia Thanks a lot, looks good ! Let me integrate it until the end of the week, when I'm back from the conference.

@rohanKanojia rohanKanojia added the WIP Work in Progress label Oct 17, 2019
@rohanKanojia
Copy link
Member Author

This has some problems, marking as WIP until update.

@rhuss
Copy link
Collaborator

rhuss commented Oct 18, 2019

@rohanKanojia ok, let me know when its ready for review.

@rohanKanojia
Copy link
Member Author

Actually, we faced some issue on fmp for this jib, jibservice wasn't considering some parameters of ImageConfiguration . I have refactored code a bit to handle those cases(for ex, when you want to push a registry other than docker hub). But you're the expert here, you can put more light on what other use cases are still missing during code review.

@rhuss
Copy link
Collaborator

rhuss commented Jan 8, 2020

I will review this PR after the next release. But let's get out a version of dmp first, as it already contains a tons of new stuff.

@rhuss
Copy link
Collaborator

rhuss commented Jun 4, 2020

@rohanKanojia sorry for the delay, but could you rebase this and fix the conflicts ? Let's get this in for the next release ...

Copy link
Collaborator

@rhuss rhuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks ! Looks good to me on a quick review. Let's merge the PR now and we always can refine it later if needed.

@rhuss rhuss merged commit 4817b64 into fabric8io:master Sep 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants