Re: [PR] Builder annotation support [camel-k]
squakez merged PR #5104: URL: https://github.com/apache/camel-k/pull/5104 -- 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-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Builder annotation support [camel-k]
github-actions[bot] commented on PR #5104: URL: https://github.com/apache/camel-k/pull/5104#issuecomment-1911601737 :heavy_check_mark: Unit test coverage report - coverage increased from 34.8% to 35.6% (**+0.8%**) -- 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-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Builder annotation support [camel-k]
github-actions[bot] commented on PR #5104: URL: https://github.com/apache/camel-k/pull/5104#issuecomment-1910537478 :heavy_check_mark: Unit test coverage report - coverage increased from 34.8% to 35.3% (**+0.5%**) -- 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-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Builder annotation support [camel-k]
rodcloutier commented on code in PR #5104: URL: https://github.com/apache/camel-k/pull/5104#discussion_r1466607137 ## pkg/controller/integrationkit/build.go: ## @@ -133,8 +133,10 @@ func (action *buildAction) handleBuildSubmitted(ctx context.Context, kit *v1.Int } } // The build operation, when executed as a Pod, should be executed by a container image containing the - // `kamel builder` command. Likely the same image running the operator should be fine. - buildConfig.ToolImage = platform.OperatorImage + // `kamel builder` command. If not specified, likely the same image running the operator should be fine. Review Comment: Done in #5110 -- 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-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Builder annotation support [camel-k]
rodcloutier commented on code in PR #5104:
URL: https://github.com/apache/camel-k/pull/5104#discussion_r1466599547
##
pkg/apis/camel/v1/common_types.go:
##
@@ -55,6 +55,8 @@ type BuildConfiguration struct {
LimitMemory string `property:"limit-memory"
json:"limitMemory,omitempty"`
// The node selector for the builder pod. Only used for `pod` strategy
NodeSelector map[string]string `property:"node-selector"
json:"nodeSelector,omitempty"`
+ // Annotation to use for the builder pod. Only user for `pod` strategy
Review Comment:
Updated with the new commits (changing this triggers many other changes when
generating files)
--
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-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Builder annotation support [camel-k]
rodcloutier commented on code in PR #5104:
URL: https://github.com/apache/camel-k/pull/5104#discussion_r1466598609
##
pkg/cmd/operator/operator.go:
##
@@ -295,7 +295,8 @@ func getOperatorImage(ctx context.Context, c ctrl.Reader)
(string, error) {
ns := platform.GetOperatorNamespace()
name := platform.GetOperatorPodName()
if ns == "" || name == "" {
- return "", nil
+ // We are most likely running out of cluster. Let's take a
chance and use the default value
Review Comment:
Done see #5109.
I have taken the liberty of bundling these two changes as:
* They are scope to the same issue
* They are located in the same context/file
* They are small enough
--
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-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Builder annotation support [camel-k]
rodcloutier commented on code in PR #5104:
URL: https://github.com/apache/camel-k/pull/5104#discussion_r1466597168
##
pkg/cmd/operator/operator.go:
##
@@ -165,8 +165,8 @@ func Run(healthPort, monitoringPort int32, leaderElection
bool, leaderElectionID
// in which case it's not possible to determine a namespace.
operatorNamespace = watchNamespace
if operatorNamespace == "" {
- leaderElection = false
- log.Info("unable to determine namespace for leader
election")
+ // We cannot go forward anyhow since later on we need a
namespace to create resources
Review Comment:
Done see #5109
--
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-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Builder annotation support [camel-k]
github-actions[bot] commented on PR #5104: URL: https://github.com/apache/camel-k/pull/5104#issuecomment-1910283554 :heavy_check_mark: Unit test coverage report - coverage increased from 34.8% to 35.3% (**+0.5%**) -- 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-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Builder annotation support [camel-k]
squakez commented on code in PR #5104:
URL: https://github.com/apache/camel-k/pull/5104#discussion_r1466413582
##
pkg/cmd/operator/operator.go:
##
@@ -295,7 +295,8 @@ func getOperatorImage(ctx context.Context, c ctrl.Reader)
(string, error) {
ns := platform.GetOperatorNamespace()
name := platform.GetOperatorPodName()
if ns == "" || name == "" {
- return "", nil
+ // We are most likely running out of cluster. Let's take a
chance and use the default value
Review Comment:
Ditto
##
pkg/apis/camel/v1/common_types.go:
##
@@ -55,6 +55,8 @@ type BuildConfiguration struct {
LimitMemory string `property:"limit-memory"
json:"limitMemory,omitempty"`
// The node selector for the builder pod. Only used for `pod` strategy
NodeSelector map[string]string `property:"node-selector"
json:"nodeSelector,omitempty"`
+ // Annotation to use for the builder pod. Only user for `pod` strategy
Review Comment:
```suggestion
// Annotation to use for the builder pod. Only used for `pod` strategy
```
##
pkg/cmd/operator/operator.go:
##
@@ -165,8 +165,8 @@ func Run(healthPort, monitoringPort int32, leaderElection
bool, leaderElectionID
// in which case it's not possible to determine a namespace.
operatorNamespace = watchNamespace
if operatorNamespace == "" {
- leaderElection = false
- log.Info("unable to determine namespace for leader
election")
+ // We cannot go forward anyhow since later on we need a
namespace to create resources
Review Comment:
Please, open a separate PR for this change so we can have a more focused
look on what it really involves.
##
pkg/controller/integrationkit/build.go:
##
@@ -133,8 +133,10 @@ func (action *buildAction) handleBuildSubmitted(ctx
context.Context, kit *v1.Int
}
}
// The build operation, when executed as a Pod, should be
executed by a container image containing the
- // `kamel builder` command. Likely the same image running the
operator should be fine.
- buildConfig.ToolImage = platform.OperatorImage
+ // `kamel builder` command. If not specified, likely the same
image running the operator should be fine.
Review Comment:
I think we must force the image to be the one on which the operator is
running. Please, open a separate PR to discuss this separately.
--
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-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Builder annotation support [camel-k]
rodcloutier commented on PR #5104: URL: https://github.com/apache/camel-k/pull/5104#issuecomment-1910230153 I am struggling to find any place to insert test for this enhancement. Could anyone chime in? -- 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-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Builder annotation support [camel-k]
rodcloutier opened a new pull request, #5104: URL: https://github.com/apache/camel-k/pull/5104 This PR proposes to add the possibility to configure annotation on the build pod. This was required to allow build pods to properly execute in the context of using Buildah in an AppArmor enabled Kubernetes cluster. Also included are a few tweak encountered while running the operator out of cluster. **Release Note** ```release-note Add possibility to use custom annotations on builder pods. ``` -- 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-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
