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 16:06:00 UTC
[PR] Add support to properly run out of cluster [camel-k]
rodcloutier opened a new pull request, #5109:
URL: https://github.com/apache/camel-k/pull/5109
This PR intends to solve some issue encountered while running the operator out of cluster.
## Current namespace requirement
We must be able to find the current namespace. If not is specified/detected we cannot keep going on as we will eventually require the namespace to be defined to create the integration plaform objects.
The proposed solution is to not allow both NAMESPACE and WATCH_NAMESPACE to be empty as it creates an issue.
## Current operator image detection
The other issue, it that when running out of cluster, we inherently cannot, when requested, detect the current image as we are not in a pod.
The proposed solution is to simply return the default operator image and never return an empty image as it is used later in the operator execution.
**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
Re: [PR] Add support to properly run out of cluster [camel-k]
Posted by "rodcloutier (via GitHub)" <gi...@apache.org>.
rodcloutier commented on code in PR #5109:
URL: https://github.com/apache/camel-k/pull/5109#discussion_r1467611199
##########
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:
But currently if you remove this, we cannot run out of cluster as this function is used to set the global `platform.Operator` which is eventually used, at least, in pkg/controller/integrationkit/build.go
https://github.com/apache/camel-k/blob/ec9406b410bcea0b2d74bfb0dc1ec57e50394d44/pkg/controller/integrationkit/build.go#L137
This will result in a build resource that has not `ToolImage` thus it cannot be run at all.
This is actually one of the reason for the other PR #5110 which would allow to workaround this by explicitly specify a ToolImage regardless if we are running in a pod or not.
--
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] Add support to properly run out of cluster [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5109:
URL: https://github.com/apache/camel-k/pull/5109#issuecomment-1911689028
: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] Add support to properly run out of cluster [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #5109:
URL: https://github.com/apache/camel-k/pull/5109#discussion_r1467685433
##########
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:
That's the point. The local execution should not allow to run a pipeline but only execute the code that can find "locally". It's because the local operator is running a given binary (typically built from source), whilst the builder would be run in the Pod with potentially a different software (as it is packaged in a container from somewhere else). I think it's wise to keep this design as it to avoid consistency problems.
--
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] Add support to properly run out of cluster [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #5109:
URL: https://github.com/apache/camel-k/pull/5109#discussion_r1467362812
##########
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:
I think the previous assumption is correct. I mean, if we're running out of cluster, then, the software binary is not distributed in a container image. If we enable this, then, we would trick the operator to think that it is running from a container, for example, enabling the possibility to run pipeline in a pod (which would instead spin off the `kamel builder` from a distribution that is different with the running application). IMO, we must return as empty, as also claimed in the comment.
##########
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:
I think the previous assumption was correct. I mean, if we're running out of cluster, then, the software binary is not distributed in a container image. If we enable this, then, we would trick the operator to think that it is running from a container, for example, enabling the possibility to run pipeline in a pod (which would instead spin off the `kamel builder` from a distribution that is different with the running application). IMO, we must return as empty, as also claimed in the comment.
--
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] Add support to properly run out of cluster [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #5109:
URL: https://github.com/apache/camel-k/pull/5109#discussion_r1467362812
##########
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:
I think the previous assumption was correct. I mean, if we're running out of cluster, then, the software binary is not distributed in a container image. If we enable this, then, we would trick the operator to think that it is running from a container, for example, enabling the possibility to run pipeline in a pod (which would instead spin off the `kamel builder` from a distribution that is different from the running application). IMO, we must return as empty, as also claimed in the comment.
--
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] Add support to properly run out of cluster [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5109:
URL: https://github.com/apache/camel-k/pull/5109#issuecomment-2097135806
This PR has been automatically marked as stale due to 90 days of inactivity.
It will be closed if no further activity occurs within 15 days.
If you think that’s incorrect or the issue should never stale, please simply write any comment.
Thanks for your contributions!
--
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