Retry behavior for synchronous calls during initialization #2118
Conversation
| try { | ||
| result = call.call(); | ||
| complete = true; | ||
| } catch (RuntimeException re) { |
There was a problem hiding this comment.
What happened if it is a RuntimeException but not an ApiException? Do we want to limit the number of retries or make it retry forever (hopefully the condition can be resolved)?
There was a problem hiding this comment.
I'll think about other exceptions... I didn't want to limit the number of retries here because the operator cannot go on until it can connect to the master and the operator will eventually be killed by the liveness probe.
| result = call.call(); | ||
| complete = true; | ||
| } catch (RuntimeException re) { | ||
| Throwable cause = re.getCause(); |
There was a problem hiding this comment.
Does the ApiException ever get nested further down? Is there ever a case where you need to loop through the causes looking for ApiException?
| LOGGER.warning(MessageKeys.EXCEPTION, cause); | ||
| } | ||
| } catch (Throwable t) { | ||
| LOGGER.warning(MessageKeys.EXCEPTION, t); |
There was a problem hiding this comment.
Please add a comment here that we expect the liveness probe to cancel this process if it "retries forever". The next coder may not understand the assumption/expectation.
As you know, we've been seeing fairly regular integration test failures for the ItIstio* series of tests. The tests fail because the operator fails to start being unable to connect to the Kubernetes master. In each case I analyzed, the operator failed just before the Istio Envoy proxy finished initialization.
Most operator calls are done using the async pattern, which has built-in delay and retry; however, the synchronous calls done during operator initialization do not have this functionality. Therefore, I've added a wrapper method that can do this retry. I've put it down at the CallBuilder level because this gave the easiest location to add the unit-test.
Creating this as draft so that you can take a look. I think I need to use some configured value for the retry delay rather than hardcoding 5 seconds.
Istio tests are clean with this change: https://build.weblogick8s.org:8443/job/weblogic-kubernetes-operator-kind-new/3454/console