[GitHub] [camel-k] asifdxtreme commented on a change in pull request #922: fix: Add option to disable Kaniko cache warming
asifdxtreme commented on a change in pull request #922: fix: Add option to
disable Kaniko cache warming
URL: https://github.com/apache/camel-k/pull/922#discussion_r328131853
##
File path: pkg/controller/integrationplatform/initialize.go
##
@@ -137,7 +137,7 @@ func (action *initializeAction) Handle(ctx
context.Context, platform *v1alpha1.I
}
// Check if the operator is running in the same namespace
before starting the cache warmer
- if platform.Namespace == platformutil.GetOperatorNamespace() {
+ if platform.Namespace == platformutil.GetOperatorNamespace() &&
platform.Spec.Build.KanikoBuildCache {
Review comment:
Hi @astefanutti Thanks again for the suggestion and help to fix this issue,
as per your review suggestions I have updated the code.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k] asifdxtreme commented on a change in pull request #922: fix: Add option to disable Kaniko cache warming
asifdxtreme commented on a change in pull request #922: fix: Add option to
disable Kaniko cache warming
URL: https://github.com/apache/camel-k/pull/922#discussion_r327515627
##
File path: pkg/controller/integrationplatform/initialize.go
##
@@ -137,7 +137,7 @@ func (action *initializeAction) Handle(ctx
context.Context, platform *v1alpha1.I
}
// Check if the operator is running in the same namespace
before starting the cache warmer
- if platform.Namespace == platformutil.GetOperatorNamespace() {
+ if platform.Namespace == platformutil.GetOperatorNamespace() &&
platform.Spec.Build.KanikoBuildCache {
Review comment:
Thanks @astefanutti , I will look into this
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k] asifdxtreme commented on a change in pull request #922: fix: Add option to disable Kaniko cache warming
asifdxtreme commented on a change in pull request #922: fix: Add option to
disable Kaniko cache warming
URL: https://github.com/apache/camel-k/pull/922#discussion_r327479299
##
File path: pkg/controller/integrationplatform/initialize.go
##
@@ -137,7 +137,7 @@ func (action *initializeAction) Handle(ctx
context.Context, platform *v1alpha1.I
}
// Check if the operator is running in the same namespace
before starting the cache warmer
- if platform.Namespace == platformutil.GetOperatorNamespace() {
+ if platform.Namespace == platformutil.GetOperatorNamespace() &&
platform.Spec.Build.KanikoBuildCache {
Review comment:
Hi @astefanutti ,
Since we are not able to get any alternative solution for this, so can we go
ahead to merge this patch?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k] asifdxtreme commented on a change in pull request #922: fix: Add option to disable Kaniko cache warming
asifdxtreme commented on a change in pull request #922: fix: Add option to
disable Kaniko cache warming
URL: https://github.com/apache/camel-k/pull/922#discussion_r326565411
##
File path: pkg/controller/integrationplatform/initialize.go
##
@@ -137,7 +137,7 @@ func (action *initializeAction) Handle(ctx
context.Context, platform *v1alpha1.I
}
// Check if the operator is running in the same namespace
before starting the cache warmer
- if platform.Namespace == platformutil.GetOperatorNamespace() {
+ if platform.Namespace == platformutil.GetOperatorNamespace() &&
platform.Spec.Build.KanikoBuildCache {
Review comment:
seems like only way possible is to keep another flag to track from where the
installation has been initiated, but this seems too much of flags for such a
small feature.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k] asifdxtreme commented on a change in pull request #922: fix: Add option to disable Kaniko cache warming
asifdxtreme commented on a change in pull request #922: fix: Add option to
disable Kaniko cache warming
URL: https://github.com/apache/camel-k/pull/922#discussion_r324988292
##
File path: pkg/controller/integrationplatform/initialize.go
##
@@ -137,7 +137,7 @@ func (action *initializeAction) Handle(ctx
context.Context, platform *v1alpha1.I
}
// Check if the operator is running in the same namespace
before starting the cache warmer
- if platform.Namespace == platformutil.GetOperatorNamespace() {
+ if platform.Namespace == platformutil.GetOperatorNamespace() &&
platform.Spec.Build.KanikoBuildCache {
Review comment:
@astefanutti I tried some options with Cobra but could'nt find a way to
differentiate whether the value was not set or set to default values.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k] asifdxtreme commented on a change in pull request #922: fix: Add option to disable Kaniko cache warming
asifdxtreme commented on a change in pull request #922: fix: Add option to
disable Kaniko cache warming
URL: https://github.com/apache/camel-k/pull/922#discussion_r323183498
##
File path: pkg/controller/integrationplatform/initialize.go
##
@@ -137,7 +137,7 @@ func (action *initializeAction) Handle(ctx
context.Context, platform *v1alpha1.I
}
// Check if the operator is running in the same namespace
before starting the cache warmer
- if platform.Namespace == platformutil.GetOperatorNamespace() {
+ if platform.Namespace == platformutil.GetOperatorNamespace() &&
platform.Spec.Build.KanikoBuildCache {
Review comment:
@astefanutti based on your input I thought to reverse the way we are dealing
with this new variable, so now the `kamel` cli tool will take the value of
`skip-kaniko-build-cache` whose value will be by default `false`. Now in the
intialize we skip the cache warmer only if this value has been explicitly
update as `true` (`if platform.Namespace == platformutil.GetOperatorNamespace()
&& !platform.Spec.Build.SkipKanikoBuildCache {`). Hope this will cover most of
the scenarios of installation.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k] asifdxtreme commented on a change in pull request #922: fix: Add option to disable Kaniko cache warming
asifdxtreme commented on a change in pull request #922: fix: Add option to
disable Kaniko cache warming
URL: https://github.com/apache/camel-k/pull/922#discussion_r323183498
##
File path: pkg/controller/integrationplatform/initialize.go
##
@@ -137,7 +137,7 @@ func (action *initializeAction) Handle(ctx
context.Context, platform *v1alpha1.I
}
// Check if the operator is running in the same namespace
before starting the cache warmer
- if platform.Namespace == platformutil.GetOperatorNamespace() {
+ if platform.Namespace == platformutil.GetOperatorNamespace() &&
platform.Spec.Build.KanikoBuildCache {
Review comment:
@astefanutti based on your input I though to reverse the way we are dealing
with this new variable, so now the `kamel` cli tool we take the value of
`skip-kaniko-build-cache` whose value will be by default `false`. Now in the
intialize we check for cache warmer intializer only if this value has been
explicitly update as `true` (`if platform.Namespace ==
platformutil.GetOperatorNamespace() &&
!platform.Spec.Build.SkipKanikoBuildCache {`). Hope this will cover most of the
scenarios of installation.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k] asifdxtreme commented on a change in pull request #922: fix: Add option to disable Kaniko cache warming
asifdxtreme commented on a change in pull request #922: fix: Add option to
disable Kaniko cache warming
URL: https://github.com/apache/camel-k/pull/922#discussion_r323085413
##
File path: pkg/controller/integrationplatform/initialize.go
##
@@ -137,7 +137,7 @@ func (action *initializeAction) Handle(ctx
context.Context, platform *v1alpha1.I
}
// Check if the operator is running in the same namespace
before starting the cache warmer
- if platform.Namespace == platformutil.GetOperatorNamespace() {
+ if platform.Namespace == platformutil.GetOperatorNamespace() &&
platform.Spec.Build.KanikoBuildCache {
Review comment:
That makes sense, I didn't considered this scenario, I will update my code
accordingly.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k] asifdxtreme commented on a change in pull request #922: fix: Add option to disable Kaniko cache warming
asifdxtreme commented on a change in pull request #922: fix: Add option to
disable Kaniko cache warming
URL: https://github.com/apache/camel-k/pull/922#discussion_r323081347
##
File path: pkg/apis/camel/v1alpha1/integrationplatform_types.go
##
@@ -105,6 +105,7 @@ type IntegrationPlatformBuildSpec struct {
PersistentVolumeClaim string
`json:"persistentVolumeClaim,omitempty"`
Maven MavenSpec
`json:"maven,omitempty"`
HTTPProxySecret string
`json:"httpProxySecret,omitempty"`
+ Cache bool
`json:"cache,omitempty"`
Review comment:
Thanks @astefanutti for the suggestion, I have updated it now
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k] asifdxtreme commented on a change in pull request #922: fix: Add option to disable Kaniko cache warming
asifdxtreme commented on a change in pull request #922: fix: Add option to
disable Kaniko cache warming
URL: https://github.com/apache/camel-k/pull/922#discussion_r321143514
##
File path: pkg/apis/camel/v1alpha1/integrationplatform_types.go
##
@@ -105,6 +105,7 @@ type IntegrationPlatformBuildSpec struct {
PersistentVolumeClaim string
`json:"persistentVolumeClaim,omitempty"`
Maven MavenSpec
`json:"maven,omitempty"`
HTTPProxySecret string
`json:"httpProxySecret,omitempty"`
+ Cache bool
`json:"bool,omitempty"`
Review comment:
Thanks for the review, I have updated the code now
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k] asifdxtreme commented on a change in pull request #922: fix: Add option to disable Kaniko cache warming
asifdxtreme commented on a change in pull request #922: fix: Add option to
disable Kaniko cache warming
URL: https://github.com/apache/camel-k/pull/922#discussion_r320864286
##
File path: pkg/cmd/install.go
##
@@ -235,6 +237,12 @@ func (o *installCmdOptions) install(_ *cobra.Command, _
[]string) error {
platform.Spec.Build.HTTPProxySecret = o.httpProxySecret
}
+ if o.cache {
+ platform.Spec.Build.Cache = true
Review comment:
thanks for the review, I have updated it as per your suggestions, I was
trying to be defensive if someone passes non boolean values.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k] asifdxtreme commented on a change in pull request #922: fix: Add option to disable Kaniko cache warming
asifdxtreme commented on a change in pull request #922: fix: Add option to
disable Kaniko cache warming
URL: https://github.com/apache/camel-k/pull/922#discussion_r320863459
##
File path: pkg/cmd/install.go
##
@@ -74,6 +74,7 @@ func newCmdInstall(rootCmdOptions *RootCmdOptions)
*cobra.Command {
cmd.Flags().StringVar(, "build-strategy", "", "Set
the build strategy")
cmd.Flags().StringVar(, "build-timeout", "", "Set how
long the build process can last")
cmd.Flags().StringVar(, "trait-profile", "", "The
profile to use for traits")
+ cmd.Flags().BoolVar(, "cache", true, "To enable or disable
the cache in building phase")
Review comment:
thanks for the review, I have updated it as per your suggestions
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
