Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to properly run out of cluster #5109

Closed

Conversation

rodcloutier
Copy link
Contributor

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

NONE

We must be able to find the current namespace. If not is specified/detected we cannot keep going on

When running out of cluster, we have no image thus we use the default image in that case, when requested
Copy link
Contributor

@squakez squakez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first part looks good, it anticipates the exit that would eventually happen.

@@ -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
Copy link
Contributor

@squakez squakez Jan 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

buildConfig.ToolImage = platform.OperatorImage

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@apache apache deleted a comment from github-actions bot Feb 6, 2024
Copy link
Contributor

github-actions bot commented May 7, 2024

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants