Log the detailed exception when reconciliation

[zuston]

When implementing the controller like TomcatReconciler and throw exception when accepting k8s event, the log will not throw the exception stacktrace. Due to this limitation, I spent lot of time to dig operator framework and found the fundamental failure.

So i hope this PR to solve this problem.

[zuston]

@csviri @metacosm Please review it.

[csviri]

LGTM

[csviri]

actually this would be better to catch in ControllerExecution. @zuston could you change it pls ? or I can issue a new PR

[metacosm]

LGTM

[csviri]

Or rather here:

  public PostExecutionControl<R> handleExecution(ExecutionScope<R> executionScope) {
    try {
      return handleDispatch(executionScope);
    } catch (KubernetesClientException e) {
      log.info(
          "Kubernetes exception {} {} during event processing, {} failed",
          e.getCode(),
          e.getMessage(),
          executionScope);
      return PostExecutionControl.exceptionDuringExecution(e);
    } catch (RuntimeException e) {
      log.error("Error during event processing {} failed.", executionScope, e);
      return PostExecutionControl.exceptionDuringExecution(e);
    }
  }

Wondering why this does not work.

[metacosm]

actually this would be better to catch in ControllerExecution. @zuston could you change it pls ? or I can issue a new PR

I was wondering about that, indeed, and thought that we could do that in a subsequent PR.

[csviri]

Probably just this should be an error level:

   } catch (KubernetesClientException e) {
      log.info(
          "Kubernetes exception {} {} during event processing, {} failed",
          e.getCode(),
          e.getMessage(),
          executionScope);
      return PostExecutionControl.exceptionDuringExecution(e);

Or just removed and handle all the runtime exceptions as it is now without this part.

[zuston]

Thanks for you quick reply @csviri @metacosm .
Maybe this also should be added in cleanup and other methods exposed for users.

[csviri]

Thanks for you quick reply @csviri @metacosm . Maybe this also should be added in cleanup and other methods exposed for users.

The thing is, for me it seems that the only problem is we make different logging for client exception, as you can see we catch all the exceptions also for cleanup in the quoted code. So I would just remove that special case for the client.

[csviri]

created this PR: #936
would rather do it this way and just close this.

But thx @zuston to pointing this out!

[zuston]

created this PR: #936 would rather do it this way and just close this.

But thx @zuston to pointing this out!

I like this general fix. Thanks @csviri.

Could you let me join into slack group, now I use this project to develop our elastic-yarn operator.

[csviri]

Could you let me join into slack group, now I use this project to develop our elastic-yarn operator.

We don't have a slack group, there is a Discord server, use this to join: https://discord.gg/DacEhAy
Or join the generic Kuberentes slack, usually watching this channel: https://kubernetes.slack.com/archives/CAW0GV7A5