[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(&impl.buildStrategy, "build-strategy", "", "Set the build strategy") cmd.Flags().StringVar(&impl.buildTimeout, "build-timeout", "", "Set how long the build process can last") cmd.Flags().StringVar(&impl.traitProfile, "trait-profile", "", "The profile to use for traits") + cmd.Flags().BoolVar(&impl.cache, "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