Skip to content

Commit c821ce8

Browse files
doxiaodoxiao
authored
Owls106230 fix NPEs caused by null domain object in the DomainPresenceInfo (#3942)
* Fix NPEs caused by null domain object in the DomainPresenceInfo
1 parent 5710c66 commit c821ce8

21 files changed

+713
-100
lines changed

‎operator/src/main/java/oracle/kubernetes/operator/DomainProcessor.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import oracle.kubernetes.operator.helpers.ClusterPresenceInfo;
1919
import oracle.kubernetes.operator.helpers.DomainPresenceInfo;
2020
import oracle.kubernetes.operator.helpers.EventHelper.EventItem;
21+
import oracle.kubernetes.operator.work.FiberGate;
2122
import oracle.kubernetes.weblogic.domain.model.ClusterResource;
2223
import oracle.kubernetes.weblogic.domain.model.DomainResource;
2324

@@ -131,4 +132,8 @@ default Map<String, Map<String,DomainPresenceInfo>> getDomainPresenceInfoMap()
131132
default Map<String, Map<String, ClusterPresenceInfo>> getClusterPresenceInfoMap() {
132133
return new ConcurrentHashMap<>();
133134
}
135+
136+
Map<String, FiberGate> getMakeRightFiberGateMap();
137+
138+
DomainPresenceInfo getExistingDomainPresenceInfo(String namespace, String domainUid);
134139
}

‎operator/src/main/java/oracle/kubernetes/operator/DomainProcessorImpl.java

Lines changed: 49 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ public class DomainProcessorImpl implements DomainProcessor, MakeRightExecutor {
106106
private static Map<String, Map<String, ScheduledFuture<?>>> statusUpdaters = new ConcurrentHashMap<>();
107107

108108
// List of clusters in a namespace.
109-
private static Map<String, Map<String, ClusterPresenceInfo>> clusters = new ConcurrentHashMap<>();
109+
private static final Map<String, Map<String, ClusterPresenceInfo>> clusters = new ConcurrentHashMap<>();
110110

111111
private final DomainProcessorDelegate delegate;
112112
private final SemanticVersion productVersion;
@@ -132,11 +132,12 @@ public DomainProcessorImpl(DomainProcessorDelegate delegate, SemanticVersion pro
132132
this.productVersion = productVersion;
133133
}
134134

135-
private static DomainPresenceInfo getExistingDomainPresenceInfo(String ns, String domainUid) {
135+
@Override
136+
public DomainPresenceInfo getExistingDomainPresenceInfo(String ns, String domainUid) {
136137
return domains.computeIfAbsent(ns, k -> new ConcurrentHashMap<>()).get(domainUid);
137138
}
138139

139-
private static DomainPresenceInfo getExistingDomainPresenceInfo(DomainPresenceInfo newPresence) {
140+
private DomainPresenceInfo getExistingDomainPresenceInfo(DomainPresenceInfo newPresence) {
140141
return getExistingDomainPresenceInfo(newPresence.getNamespace(), newPresence.getDomainUid());
141142
}
142143

@@ -158,6 +159,11 @@ public Map<String, Map<String,ClusterPresenceInfo>> getClusterPresenceInfoMap()
158159
return clusters;
159160
}
160161

162+
@Override
163+
public Map<String, FiberGate> getMakeRightFiberGateMap() {
164+
return makeRightFiberGates;
165+
}
166+
161167
private static List<DomainPresenceInfo> getExistingDomainPresenceInfoForCluster(String ns, String cluster) {
162168
List<DomainPresenceInfo> referencingDomains = new ArrayList<>();
163169
Optional.ofNullable(domains.get(ns)).ifPresent(d -> d.values().stream()
@@ -244,21 +250,10 @@ private static void deleteEventK8SObjects(CoreV1Event event) {
244250
getEventK8SObjects(event).remove(event);
245251
}
246252

247-
private static void onCreateModifyEvent(CoreV1Event event) {
248-
V1ObjectReference ref = event.getInvolvedObject();
249-
250-
if (ref == null || ref.getName() == null) {
251-
return;
252-
}
253-
254-
String kind = ref.getKind();
255-
if (kind == null) {
256-
return;
257-
}
258-
253+
private static void onCreateModifyEvent(@Nonnull String kind, @Nonnull String name, CoreV1Event event) {
259254
switch (kind) {
260255
case EventConstants.EVENT_KIND_POD:
261-
processPodEvent(event);
256+
processPodEvent(name, event);
262257
break;
263258
case EventConstants.EVENT_KIND_DOMAIN:
264259
case EventConstants.EVENT_KIND_NAMESPACE:
@@ -272,13 +267,8 @@ private static void onCreateModifyEvent(CoreV1Event event) {
272267
}
273268
}
274269

275-
private static void processPodEvent(CoreV1Event event) {
276-
V1ObjectReference ref = event.getInvolvedObject();
277-
278-
if (ref == null || ref.getName() == null) {
279-
return;
280-
}
281-
if (ref.getName().equals(NamespaceHelper.getOperatorPodName())) {
270+
private static void processPodEvent(@Nonnull String name, CoreV1Event event) {
271+
if (name.equals(NamespaceHelper.getOperatorPodName())) {
282272
updateEventK8SObjects(event);
283273
} else {
284274
processServerEvent(event);
@@ -313,30 +303,19 @@ public static List<DomainResource> getDomains(String ns) {
313303
}
314304

315305
private static void addToList(List<DomainResource> list, DomainPresenceInfo info) {
316-
if (info.isNotDeleting()) {
306+
if (isNotDeleting(info)) {
317307
list.add(info.getDomain());
318308
}
319309
}
320310

321-
private void onDeleteEvent(CoreV1Event event) {
322-
V1ObjectReference ref = event.getInvolvedObject();
323-
324-
if (ref == null || ref.getName() == null) {
325-
return;
326-
}
327-
328-
String kind = ref.getKind();
329-
if (kind == null) {
330-
return;
331-
}
332-
311+
private void onDeleteEvent(@Nonnull String kind, @Nonnull String name, CoreV1Event event) {
333312
switch (kind) {
334313
case EventConstants.EVENT_KIND_DOMAIN:
335314
case EventConstants.EVENT_KIND_NAMESPACE:
336315
deleteEventK8SObjects(event);
337316
break;
338317
case EventConstants.EVENT_KIND_POD:
339-
if (ref.getName().equals(NamespaceHelper.getOperatorPodName())) {
318+
if (name.equals(NamespaceHelper.getOperatorPodName())) {
340319
deleteEventK8SObjects(event);
341320
}
342321
break;
@@ -432,7 +411,11 @@ private boolean findClusterPresenceInfo(String namespace, String clusterName) {
432411
}
433412

434413
private boolean hasDeletedClusterEventData(MakeRightClusterOperation operation) {
435-
return operation.getEventData() != null && operation.getEventData().getItem().name().equals("CLUSTER_DELETED");
414+
return EventItem.CLUSTER_DELETED == getEventItem(operation);
415+
}
416+
417+
private EventItem getEventItem(MakeRightClusterOperation operation) {
418+
return Optional.ofNullable(operation.getEventData()).map(EventData::getItem).orElse(null);
436419
}
437420

438421
private void logStartingDomain(DomainPresenceInfo presenceInfo) {
@@ -477,6 +460,10 @@ public void registerClusterPresenceInfo(ClusterPresenceInfo info) {
477460
@Override
478461
public void unregisterDomainPresenceInfo(DomainPresenceInfo info) {
479462
unregisterPresenceInfo(info.getNamespace(), info.getDomainUid());
463+
}
464+
465+
@Override
466+
public void unregisterDomainEventK8SObjects(DomainPresenceInfo info) {
480467
unregisterEventK8SObject(info.getNamespace(), info.getDomainUid());
481468
}
482469

@@ -595,7 +582,7 @@ private void processServerPodWatch(V1Pod pod, String watchType) {
595582
break;
596583
case DELETED:
597584
boolean removed = info.deleteServerPodFromEvent(serverName, pod);
598-
if (removed && info.isNotDeleting() && Boolean.FALSE.equals(info.isServerPodBeingDeleted(serverName))) {
585+
if (removed && isNotDeleting(info) && Boolean.FALSE.equals(info.isServerPodBeingDeleted(serverName))) {
599586
LOGGER.info(MessageKeys.POD_DELETED, domainUid, getPodNamespace(pod), serverName);
600587
createMakeRightOperation(info).interrupt().withExplicitRecheck().execute();
601588
}
@@ -670,7 +657,7 @@ public void dispatchServiceWatch(Watch.Response<V1Service> item) {
670657
break;
671658
case DELETED:
672659
boolean removed = ServiceHelper.deleteFromEvent(info, item.object);
673-
if (removed && info.isNotDeleting()) {
660+
if (removed && isNotDeleting(info)) {
674661
createMakeRightOperation(info).interrupt().withExplicitRecheck().execute();
675662
}
676663
break;
@@ -702,14 +689,18 @@ public void dispatchPodDisruptionBudgetWatch(Watch.Response<V1PodDisruptionBudge
702689
break;
703690
case DELETED:
704691
boolean removed = PodDisruptionBudgetHelper.deleteFromEvent(info, item.object);
705-
if (removed && info.isNotDeleting()) {
692+
if (removed && isNotDeleting(info)) {
706693
createMakeRightOperation(info).interrupt().withExplicitRecheck().execute();
707694
}
708695
break;
709696
default:
710697
}
711698
}
712699

700+
private static boolean isNotDeleting(DomainPresenceInfo info) {
701+
return info.isNotDeleting() && info.getDomain() != null;
702+
}
703+
713704
private String getPDBNamespace(V1PodDisruptionBudget pdb) {
714705
return Optional.ofNullable(pdb).map(V1PodDisruptionBudget::getMetadata)
715706
.map(V1ObjectMeta::getNamespace).orElse(null);
@@ -721,7 +712,7 @@ private String getPDBNamespace(V1PodDisruptionBudget pdb) {
721712
*/
722713
public void dispatchConfigMapWatch(Watch.Response<V1ConfigMap> item) {
723714
V1ConfigMap c = item.object;
724-
if (c != null && c.getMetadata() != null) {
715+
if (c.getMetadata() != null) {
725716
switch (item.type) {
726717
case MODIFIED:
727718
case DELETED:
@@ -742,18 +733,22 @@ public void dispatchConfigMapWatch(Watch.Response<V1ConfigMap> item) {
742733
*/
743734
public void dispatchEventWatch(Watch.Response<CoreV1Event> item) {
744735
CoreV1Event e = item.object;
745-
if (e != null) {
746-
switch (item.type) {
747-
case ADDED:
748-
case MODIFIED:
749-
onCreateModifyEvent(e);
750-
break;
751-
case DELETED:
752-
onDeleteEvent(e);
753-
break;
754-
case ERROR:
755-
default:
756-
}
736+
V1ObjectReference ref = e.getInvolvedObject();
737+
738+
if (ref == null || ref.getName() == null || ref.getKind() == null) {
739+
return;
740+
}
741+
742+
switch (item.type) {
743+
case ADDED:
744+
case MODIFIED:
745+
onCreateModifyEvent(ref.getKind(), ref.getName(), e);
746+
break;
747+
case DELETED:
748+
onDeleteEvent(ref.getKind(), ref.getName(), e);
749+
break;
750+
case ERROR:
751+
default:
757752
}
758753
}
759754

‎operator/src/main/java/oracle/kubernetes/operator/DomainResourcesValidation.java

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ private boolean isForDomain(ClusterResource clusterResource, DomainPresenceInfo
116116
}
117117

118118
private void addPodList(V1PodList list) {
119-
getDomainPresenceInfoMap().values().stream().forEach(dpi -> removeDeletedPodsFromDPI(list, dpi));
119+
getDomainPresenceInfoMap().values().forEach(dpi -> removeDeletedPodsFromDPI(list, dpi));
120120
list.getItems().forEach(this::addPod);
121121
}
122122

@@ -140,14 +140,22 @@ private void addPod(V1Pod pod) {
140140
String domainUid = PodHelper.getPodDomainUid(pod);
141141
String serverName = PodHelper.getPodServerName(pod);
142142
if (domainUid != null && serverName != null) {
143-
getDomainPresenceInfo(domainUid).setServerPodFromEvent(serverName, pod);
143+
setServerPodFromEvent(getExistingDomainPresenceInfo(domainUid), serverName, pod);
144144
}
145145
}
146146

147-
private DomainPresenceInfo getDomainPresenceInfo(String domainUid) {
147+
private void setServerPodFromEvent(DomainPresenceInfo info, String serverName, V1Pod pod) {
148+
Optional.ofNullable(info).ifPresent(i -> i.setServerPodFromEvent(serverName, pod));
149+
}
150+
151+
private DomainPresenceInfo getOrComputeDomainPresenceInfo(String domainUid) {
148152
return getDomainPresenceInfoMap().computeIfAbsent(domainUid, k -> new DomainPresenceInfo(namespace, domainUid));
149153
}
150154

155+
private DomainPresenceInfo getExistingDomainPresenceInfo(String domainUid) {
156+
return getDomainPresenceInfoMap().get(domainUid);
157+
}
158+
151159
private Map<String, DomainPresenceInfo> getDomainPresenceInfoMap() {
152160
return processor.getDomainPresenceInfoMap().computeIfAbsent(namespace, k -> new ConcurrentHashMap<>());
153161
}
@@ -163,7 +171,7 @@ private void addServiceList(V1ServiceList list) {
163171
private void addService(V1Service service) {
164172
String domainUid = ServiceHelper.getServiceDomainUid(service);
165173
if (domainUid != null) {
166-
ServiceHelper.addToPresence(getDomainPresenceInfo(domainUid), service);
174+
ServiceHelper.addToPresence(getExistingDomainPresenceInfo(domainUid), service);
167175
}
168176
}
169177

@@ -174,12 +182,12 @@ private void addPodDisruptionBudgetList(V1PodDisruptionBudgetList list) {
174182
private void addPodDisruptionBudget(V1PodDisruptionBudget pdb) {
175183
String domainUid = PodDisruptionBudgetHelper.getDomainUid(pdb);
176184
if (domainUid != null) {
177-
PodDisruptionBudgetHelper.addToPresence(getDomainPresenceInfo(domainUid), pdb);
185+
PodDisruptionBudgetHelper.addToPresence(getExistingDomainPresenceInfo(domainUid), pdb);
178186
}
179187
}
180188

181189
private void addDomainList(DomainList list) {
182-
getDomainPresenceInfoMap().values().stream().forEach(dpi -> updateDeletedDomainsinDPI(list));
190+
getDomainPresenceInfoMap().values().forEach(dpi -> updateDeletedDomainsinDPI(list));
183191
list.getItems().forEach(this::addDomain);
184192
}
185193

@@ -188,16 +196,27 @@ private void updateDeletedDomainsinDPI(DomainList list) {
188196
.map(DomainResource::getDomainUid).collect(Collectors.toList());
189197

190198
getDomainPresenceInfoMap().values().stream()
191-
.filter(dpi -> !domainNamesFromList.contains(dpi.getDomainUid())).collect(Collectors.toList())
192-
.forEach(i -> getDomainPresenceInfo(i.getDomainUid()).setDomain(null));
199+
.filter(dpi -> !domainNamesFromList.contains(dpi.getDomainUid()))
200+
.filter(dpi -> isNotBeingProcessed(dpi.getNamespace(), dpi.getDomainUid()))
201+
.collect(Collectors.toList())
202+
.forEach(i -> i.setDomain(null));
203+
}
204+
205+
private boolean isNotBeingProcessed(String namespace, String domainUid) {
206+
return processor.getMakeRightFiberGateMap().get(namespace).getCurrentFibers().get(domainUid) == null;
193207
}
194208

195209
private void addDomain(DomainResource domain) {
196-
getDomainPresenceInfo(domain.getDomainUid()).setDomain(domain);
210+
getOrComputeDomainPresenceInfo(domain.getDomainUid()).setDomain(domain);
197211
}
198212

199213
private void addClusterList(ClusterList list) {
200214
activeClusterResources = list;
215+
list.getItems().forEach(this::addCluster);
216+
}
217+
218+
private void addCluster(ClusterResource cluster) {
219+
getClusterPresenceInfoMap().put(cluster.getClusterName(), new ClusterPresenceInfo(cluster.getNamespace(), cluster));
201220
}
202221

203222
private Stream<DomainPresenceInfo> getStrandedDomainPresenceInfos(DomainProcessor dp) {
@@ -229,7 +248,7 @@ private static void activateDomain(DomainProcessor dp, DomainPresenceInfo info)
229248
info.setPopulated(true);
230249
MakeRightDomainOperation makeRight = dp.createMakeRightOperation(info).withExplicitRecheck();
231250
if (info.getDomain().getStatus() == null) {
232-
makeRight = makeRight.withEventData(new EventData(DOMAIN_CREATED)).interrupt();
251+
makeRight.withEventData(new EventData(DOMAIN_CREATED)).interrupt();
233252
}
234253
makeRight.execute();
235254
}

‎operator/src/main/java/oracle/kubernetes/operator/DomainStatusUpdater.java

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) 2018, 2022, Oracle and/or its affiliates.
1+
// Copyright (c) 2018, 2023, Oracle and/or its affiliates.
22
// Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.
33

44
package oracle.kubernetes.operator;
@@ -395,7 +395,9 @@ private Step createDomainRefreshStep(DomainStatusUpdaterContext context) {
395395
static class DomainUpdateStep extends ResponseStep<DomainResource> {
396396
@Override
397397
public NextAction onSuccess(Packet packet, CallResponse<DomainResource> callResponse) {
398-
packet.getSpi(DomainPresenceInfo.class).setDomain(callResponse.getResult());
398+
if (callResponse.getResult() != null) {
399+
packet.getSpi(DomainPresenceInfo.class).setDomain(callResponse.getResult());
400+
}
399401
return doNext(packet);
400402
}
401403

@@ -480,9 +482,8 @@ DomainStatus cloneStatus() {
480482

481483
private Step createDomainStatusReplaceStep() {
482484
LOGGER.fine(MessageKeys.DOMAIN_STATUS, getDomainUid(), getNewStatus());
483-
if (LOGGER.isFinerEnabled()) {
484-
LOGGER.finer("status change: " + createPatchString());
485-
}
485+
LOGGER.finer("status change: " + createPatchString());
486+
486487
DomainResource oldDomain = getDomain();
487488
DomainStatus status = getNewStatus();
488489
if (isMakeRight) {
@@ -621,14 +622,12 @@ DomainStatusUpdaterContext createContext(Packet packet) {
621622
return new StatusUpdateContext(packet, this);
622623
}
623624

624-
@Override
625-
void modifyStatus(DomainStatus domainStatus) { // no-op; modification happens in the context itself.
626-
}
627-
628625
@Override
629626
public NextAction apply(Packet packet) {
630-
packet.put(ProcessingConstants.SKIP_STATUS_UPDATE,
631-
Boolean.valueOf(shouldSkipDomainStatusUpdate(packet)));
627+
if (isDomainNotPresent(packet)) {
628+
return doNext(packet);
629+
}
630+
packet.put(ProcessingConstants.SKIP_STATUS_UPDATE, shouldSkipDomainStatusUpdate(packet));
632631
if (endOfProcessing) {
633632
packet.put(ProcessingConstants.END_OF_PROCESSING, Boolean.TRUE);
634633
}
@@ -642,6 +641,11 @@ private boolean shouldSkipDomainStatusUpdate(Packet packet) {
642641
&& !endOfProcessing;
643642
}
644643

644+
private boolean isDomainNotPresent(Packet packet) {
645+
DomainPresenceInfo info = packet.getSpi(DomainPresenceInfo.class);
646+
return info == null || info.getDomain() == null;
647+
}
648+
645649
static class StatusUpdateContext extends DomainStatusUpdaterContext {
646650
private final WlsDomainConfig config;
647651
private final Set<String> expectedRunningServers;

‎operator/src/main/java/oracle/kubernetes/operator/MakeRightExecutor.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,4 +76,6 @@ default Step createNamespacedResourceSteps(Processors processors, DomainPresence
7676
* @param info the presence info which encapsulates the domain
7777
*/
7878
void unregisterClusterPresenceInfo(ClusterPresenceInfo info);
79+
80+
void unregisterDomainEventK8SObjects(DomainPresenceInfo info);
7981
}

0 commit comments

Comments
 (0)