You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2022/02/11 12:28:16 UTC
[GitHub] [camel-k] squakez opened a new pull request #3008: fix(build): incremental build
squakez opened a new pull request #3008:
URL: https://github.com/apache/camel-k/pull/3008
* Fixed the SHA1 which were not calculated properly
* Added a check to verify that the base image != default expected
* Unit and E2E test to make sure we'll be warned if the behavior is broken again
Closes #3007
<!-- Description -->
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
NONE
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a change in pull request #3008: fix(build): incremental build
Posted by GitBox <gi...@apache.org>.
squakez commented on a change in pull request #3008:
URL: https://github.com/apache/camel-k/pull/3008#discussion_r806586674
##########
File path: pkg/builder/image.go
##########
@@ -120,7 +120,7 @@ func jvmDockerfile(ctx *builderContext) error {
}
func incrementalImageContext(ctx *builderContext) error {
- if ctx.Build.BaseImage != "" {
+ if ctx.Build.BaseImage != "" && ctx.Build.BaseImage != defaults.BaseImage() {
Review comment:
Okey, the goal of this PR is to fix the incremental build which is failing since version 1.7. I am going to remove that branch to fix in `1.8.1` and create a follow up issue to implement the suggestions you've provided.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a change in pull request #3008: fix(build): incremental build
Posted by GitBox <gi...@apache.org>.
squakez commented on a change in pull request #3008:
URL: https://github.com/apache/camel-k/pull/3008#discussion_r806586674
##########
File path: pkg/builder/image.go
##########
@@ -120,7 +120,7 @@ func jvmDockerfile(ctx *builderContext) error {
}
func incrementalImageContext(ctx *builderContext) error {
- if ctx.Build.BaseImage != "" {
+ if ctx.Build.BaseImage != "" && ctx.Build.BaseImage != defaults.BaseImage() {
Review comment:
Okey, the goal of this PR is to fix the incremental build which is failing since version 1.7. I am going to remove that branch to fix in `1.8.1` and create a follow up issue to implement the suggestions you've provided.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a change in pull request #3008: fix(build): incremental build
Posted by GitBox <gi...@apache.org>.
squakez commented on a change in pull request #3008:
URL: https://github.com/apache/camel-k/pull/3008#discussion_r804736957
##########
File path: pkg/builder/image.go
##########
@@ -120,7 +120,7 @@ func jvmDockerfile(ctx *builderContext) error {
}
func incrementalImageContext(ctx *builderContext) error {
- if ctx.Build.BaseImage != "" {
+ if ctx.Build.BaseImage != "" && ctx.Build.BaseImage != defaults.BaseImage() {
Review comment:
True. I fail to understand this check meaning then and how it worked before the last refactoring. The `ctx.Build.BaseImage` will always be `!= ""` because, at minimum it is filled with the default base image, reason why it is not working. Any help appreciated :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #3008: fix(build): incremental build
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #3008:
URL: https://github.com/apache/camel-k/pull/3008#discussion_r805913550
##########
File path: pkg/builder/image.go
##########
@@ -120,7 +120,7 @@ func jvmDockerfile(ctx *builderContext) error {
}
func incrementalImageContext(ctx *builderContext) error {
- if ctx.Build.BaseImage != "" {
+ if ctx.Build.BaseImage != "" && ctx.Build.BaseImage != defaults.BaseImage() {
Review comment:
The branch logic, to fallback to "standard" image, should be removed.
The logic to configure either "standard" or "incremental" should be done in the _builder_ trait, where a configuration parameter could be added, and the corresponding build step added. It's a bit more subtle than that, because "incremental" image build step should be deactivated for Quarkus native build, from the _quarkus_ trait, that already manipulates the build steps, but that should be quite straightforward.
There is also the following TODO that may be removed while we are at it:
https://github.com/apache/camel-k/blob/478ca56e54eadc3cfa9a10bf592935a34d37d9e7/pkg/builder/image.go#L148
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #3008: fix(build): incremental build
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #3008:
URL: https://github.com/apache/camel-k/pull/3008#discussion_r804668099
##########
File path: pkg/builder/image.go
##########
@@ -120,7 +120,7 @@ func jvmDockerfile(ctx *builderContext) error {
}
func incrementalImageContext(ctx *builderContext) error {
- if ctx.Build.BaseImage != "" {
+ if ctx.Build.BaseImage != "" && ctx.Build.BaseImage != defaults.BaseImage() {
Review comment:
I think it's still an issue when the base image set on the IntegrationPlatform that's been used to initialise the Build base image is different than `default.BaseImage`?
##########
File path: pkg/builder/image.go
##########
@@ -120,7 +120,7 @@ func jvmDockerfile(ctx *builderContext) error {
}
func incrementalImageContext(ctx *builderContext) error {
- if ctx.Build.BaseImage != "" {
+ if ctx.Build.BaseImage != "" && ctx.Build.BaseImage != defaults.BaseImage() {
Review comment:
I think it's still an issue when the base image set on the IntegrationPlatform that's been used to initialise the Build base image is different than `default.BaseImage()`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti merged pull request #3008: fix(build): incremental build
Posted by GitBox <gi...@apache.org>.
astefanutti merged pull request #3008:
URL: https://github.com/apache/camel-k/pull/3008
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #3008: fix(build): incremental build
Posted by GitBox <gi...@apache.org>.
squakez commented on pull request #3008:
URL: https://github.com/apache/camel-k/pull/3008#issuecomment-1040326870
This one looks good now. @astefanutti do you think is it okey to merge (given that we will need to come back on this topic with #3011)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #3008: fix(build): incremental build
Posted by GitBox <gi...@apache.org>.
squakez commented on pull request #3008:
URL: https://github.com/apache/camel-k/pull/3008#issuecomment-1040326870
This one looks good now. @astefanutti do you think is it okey to merge (given that we will need to come back on this topic with #3011)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti merged pull request #3008: fix(build): incremental build
Posted by GitBox <gi...@apache.org>.
astefanutti merged pull request #3008:
URL: https://github.com/apache/camel-k/pull/3008
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org