You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "rodcloutier (via GitHub)" <gi...@apache.org> on 2024/01/25 13:32:11 UTC

[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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Builder annotation support [camel-k]

Posted by "squakez (via GitHub)" <gi...@apache.org>.
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Builder annotation support [camel-k]

Posted by "rodcloutier (via GitHub)" <gi...@apache.org>.
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Builder annotation support [camel-k]

Posted by "rodcloutier (via GitHub)" <gi...@apache.org>.
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Builder annotation support [camel-k]

Posted by "rodcloutier (via GitHub)" <gi...@apache.org>.
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Builder annotation support [camel-k]

Posted by "rodcloutier (via GitHub)" <gi...@apache.org>.
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Builder annotation support [camel-k]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Builder annotation support [camel-k]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Builder annotation support [camel-k]

Posted by "rodcloutier (via GitHub)" <gi...@apache.org>.
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Builder annotation support [camel-k]

Posted by "squakez (via GitHub)" <gi...@apache.org>.
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Builder annotation support [camel-k]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org