OWLS-89106 - Potential fix for pod startup issue in GBU CNE environment after node drain/repave operation #2398
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| import java.util.Map; | ||
| import java.util.Optional; | ||
| import java.util.concurrent.ThreadFactory; | ||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
| import java.util.function.Consumer; | ||
| import javax.annotation.Nonnull; | ||
|
|
@@ -28,16 +29,24 @@ | |
| import io.kubernetes.client.util.Watchable; | ||
| import oracle.kubernetes.operator.TuningParameters.WatchTuning; | ||
| import oracle.kubernetes.operator.builders.WatchBuilder; | ||
| import oracle.kubernetes.operator.calls.CallResponse; | ||
| import oracle.kubernetes.operator.helpers.CallBuilder; | ||
| import oracle.kubernetes.operator.helpers.DomainPresenceInfo; | ||
| import oracle.kubernetes.operator.helpers.KubernetesUtils; | ||
| import oracle.kubernetes.operator.helpers.LegalNames; | ||
| import oracle.kubernetes.operator.helpers.PodHelper; | ||
| import oracle.kubernetes.operator.helpers.ResponseStep; | ||
| import oracle.kubernetes.operator.logging.LoggingFacade; | ||
| import oracle.kubernetes.operator.logging.LoggingFactory; | ||
| import oracle.kubernetes.operator.logging.MessageKeys; | ||
| import oracle.kubernetes.operator.steps.DefaultResponseStep; | ||
| import oracle.kubernetes.operator.watcher.WatchListener; | ||
| import oracle.kubernetes.operator.work.NextAction; | ||
| import oracle.kubernetes.operator.work.Packet; | ||
| import oracle.kubernetes.operator.work.Step; | ||
| import oracle.kubernetes.weblogic.domain.model.Domain; | ||
|
|
||
| import static oracle.kubernetes.operator.ProcessingConstants.MAKE_RIGHT_DOMAIN_OPERATION; | ||
|
|
||
| /** | ||
| * Watches for changes to pods. | ||
|
|
@@ -322,6 +331,55 @@ V1ObjectMeta getMetadata(V1Pod pod) { | |
| Step createReadAsyncStep(String name, String namespace, String domainUid, ResponseStep<V1Pod> responseStep) { | ||
| return new CallBuilder().readPodAsync(name, namespace, domainUid, responseStep); | ||
| } | ||
|
|
||
| @Override | ||
| protected DefaultResponseStep<V1Pod> resumeIfReady(Callback callback) { | ||
| return new DefaultResponseStep<>(null) { | ||
| @Override | ||
| public NextAction onSuccess(Packet packet, CallResponse<V1Pod> callResponse) { | ||
| DomainPresenceInfo info = packet.getSpi(DomainPresenceInfo.class); | ||
| Optional.ofNullable(callResponse.getResult()).ifPresent(pod -> setServerPodFromEvent(info, pod)); | ||
| if (isReady(callResponse.getResult())) { | ||
| resetWatchBackstopRecheckCount(info); | ||
| return proceedFromWait(packet, callResponse); | ||
| } | ||
| if (shouldWait(info)) { | ||
| // Watch backstop recheck count is less than or equal to the configured recheck count, delay. | ||
| return doDelay(createReadAndIfReadyCheckStep(callback), packet, | ||
| getWatchBackstopRecheckDelaySeconds(), TimeUnit.SECONDS); | ||
| } else { | ||
| // Watch backstop recheck count is more than configured recheck count, proceed to make-right step. | ||
| return doNext(new CallBuilder().readDomainAsync(info.getDomainUid(), | ||
| info.getNamespace(), new MakeRightDomainStep(null)), packet); | ||
| } | ||
| } | ||
|
|
||
| private void resetWatchBackstopRecheckCount(DomainPresenceInfo info) { | ||
| Optional.ofNullable(info).ifPresent(DomainPresenceInfo::resetWatchBackstopRecheckCount); | ||
| } | ||
|
|
||
| private void setServerPodFromEvent(DomainPresenceInfo info, V1Pod pod) { | ||
| Optional.ofNullable(info).ifPresent(i -> i.setServerPodFromEvent(getPodLabel(pod), pod)); | ||
| } | ||
|
|
||
| private NextAction proceedFromWait(Packet packet, CallResponse<V1Pod> callResponse) { | ||
| callback.proceedFromWait(callResponse.getResult()); | ||
| return doNext(packet); | ||
| } | ||
|
|
||
| private boolean shouldWait(DomainPresenceInfo info) { | ||
| return info == null || info.incrementAndGetWatchBackstopRecheckCount() <= getWatchBackstopRecheckCount(); | ||
| } | ||
|
|
||
| private String getPodLabel(V1Pod pod) { | ||
| return Optional.ofNullable(pod) | ||
| .map(V1Pod::getMetadata) | ||
| .map(V1ObjectMeta::getLabels) | ||
| .map(m -> m.get(LabelConstants.SERVERNAME_LABEL)) | ||
| .orElse(null); | ||
| } | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| private class WaitForPodReadyStep extends WaitForPodStatusStep { | ||
|
|
@@ -360,6 +418,7 @@ protected void removeCallback(String podName, Consumer<V1Pod> callback) { | |
| protected void logWaiting(String name) { | ||
| LOGGER.fine(MessageKeys.WAITING_FOR_POD_READY, name); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| private class WaitForPodDeleteStep extends WaitForPodStatusStep { | ||
|
|
@@ -383,4 +442,20 @@ protected void removeCallback(String podName, Consumer<V1Pod> callback) { | |
| removeOnDeleteCallback(podName, callback); | ||
| } | ||
| } | ||
|
|
||
| private static class MakeRightDomainStep extends DefaultResponseStep { | ||
| MakeRightDomainStep(Step next) { | ||
| super(next); | ||
| } | ||
|
|
||
| @Override | ||
| public NextAction onSuccess(Packet packet, CallResponse callResponse) { | ||
| MakeRightDomainOperation makeRightDomainOperation = | ||
|
There was a problem hiding this comment. I am not sure if it is always correct for all cases to use the existing MakeRightDomainOperation object. For example, if the original object has an EventData, we may not know if the event has been generated already or not in the previous attempt. There was a problem hiding this comment. Thanks, Dongbo. That's a good point. In the latest integration test run, I'm seeing some new failures in ItKubernetesEvent and ItPodsRestart related to event generation. It looks like these failures are intermittent and timing-dependent. I'm not sure if I can create a new MakeRightDomainOperation object in this step. I'll continue looking into this tomorrow. There was a problem hiding this comment. @doxiao, I'm not sure but I wouldn't be surprised if there are some bug in our flow. The intended design is that the operator could die at any code point and have a new operator start and have to recover. Therefore, it's a bug if any of our code paths depend on in-memory state. Since updating the Domain status and generated Events is not transactional then there may be some gaps that are impossible to close, but we should for instance be able to start a rolling, have the operator die, be restarted, and then have the new operator complete the roll and properly generate the rolling completed event. I see these timeouts as being analogous. If a wait timesout and the make-right loop goes back to the "top" then it ought to complete similarly. There was a problem hiding this comment. Ryan, I agree with you that the operator's processing flow should not depend on in-memory states. We could review the flow to make sure that is the case. But the issue here is a little different, I think - we are reusing a DomainMakeRightOperation object to do a new execution. I don't think the operator does this prior to this PR. EventData is just one piece of the data in the MakeRightDomainOperation object. We are probably better off creating a new MakeRightDomainOperation; the new code here already overrides some of the data in the existing object prior to calling execute(). There was a problem hiding this comment. I'm not quite sure what you are proposing. What Anil and I originally discussed was to, on timeout, cancel the current fiber and start a new fiber for a new make-right operation. He found that he could instead take the current Fiber back to the top step using this pattern. That sounds reasonable. Is there just some other state in the current make right operation and/or Fiber that he needs to clear. There was a problem hiding this comment. My suggestion is to create a new MakeRightDomainOperation passing in the new DomainPresenceInfo, instead of reusing the current make right domain operation. This suggestion does not change the logic/spirit of the current changes. I discussed with Anil offline, and found out that apparently he had problem in creating a new make right domain operation instance within PodWatcher's static context. We could add a static method somewhere for this. For example, we could refactor the Main class a little and add a static factory method there for PodWatcher to use to create a new make right domain operation instance. What I like about using a new instance is that the code here would have full control of what it needs to do. Alternatively, we could have a clear() method on MakeRightDomainOperation to start clean if we know for sure there is no other fiber running using this make right domain operation instance. Then we have to keep the clear() method up-to-date when we add new variables into it in the future. I personally prefers the first option. There was a problem hiding this comment. I have implemented the clear() method on MakeRightDomainOperation to reset its state before executing the new make-right operation in |
||
| (MakeRightDomainOperation)packet.get(MAKE_RIGHT_DOMAIN_OPERATION); | ||
| makeRightDomainOperation.setLiveInfo(new DomainPresenceInfo((Domain)callResponse.getResult())); | ||
| makeRightDomainOperation.withExplicitRecheck().interrupt().execute(); | ||
| return super.onSuccess(packet, callResponse); | ||
| } | ||
| } | ||
|
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you reread the domain, but don't we also need to reread the pod and/or other services?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was following the approach in
handleModifiedDomainwhere we call the make-right domain with just the domain object received in the watch event.