Owls104087 (refactored) fix an intermitent failure in ItKubernetesDomainEvents#testK8SEventsDelete #3704

doxiao · 2022-11-29T14:56:03Z

The PR fixes an intermittent issue in nightly where the ClusterDeleted event is missing when deleting a domain and cluster resources. This PR includes all in #3704, plus refactoring of DomainPresenceInfo (see the last item in the list below).

Generate ClusterCreated/Deleted/Changed events no matter if the cluster is not referenced by any domain.
When a cluster is referenced by a domain, the ClusterCreated/Deleted/Changed events are generated in a cluster-event-only MakeRightOperation followed by a regular MakeRightOperation to do the necessary domain processing.
Refactor the code to have a common ResourcePresenceInfo as the base of DomainPresenceInfo and ClusterPresenceInfo, and to have a MakeRightOperation as the base of MakeRightDomainOperation and MakeRightClusterOperation.

https://build.weblogick8s.org:8443/job/weblogic-kubernetes-operator-kind-new/14221/

…ter resources

…to complete

…operation does not wait

ankedia · 2022-11-29T21:14:55Z

operator/src/main/java/oracle/kubernetes/operator/helpers/ClusterPresenceInfo.java

+/**
+ * Operator's mapping between custom resource Domain and runtime details about that domain,
+ * including the scan and the Pods and Services for servers.
+ */


Operator's mapping between custom resource Domain and runtime details about that domain -> "Operator's mapping between custom resource cluster and runtime details about that cluster."

ankedia · 2022-11-29T21:15:23Z

operator/src/main/java/oracle/kubernetes/operator/helpers/ClusterPresenceInfo.java

+  private final ClusterResource cluster;
+
+  /**
+   * Create presence for a domain.


Create presence for a domain -> Create presence for a cluster.

ankedia · 2022-11-29T21:18:49Z

operator/src/main/java/oracle/kubernetes/operator/helpers/ResourcePresenceInfo.java

+/**
+ * Operator's mapping between custom resource Domain and runtime details about that domain,
+ * including the scan and the Pods and Services for servers.
+ */


This can either be mapping between the Domain or Cluster custom resource.

ankedia · 2022-11-29T21:19:12Z

operator/src/main/java/oracle/kubernetes/operator/helpers/ResourcePresenceInfo.java

+  final String namespace;
+
+  /**
+   * Create presence for a domain.


This can also be a cluster.

ankedia · 2022-11-29T21:21:22Z

operator/src/main/java/oracle/kubernetes/operator/logging/ThreadLoggingContext.java

@@ -27,6 +28,10 @@ public ThreadLoggingContext presenceInfo(DomainPresenceInfo info) {
    return namespace(info.getNamespace()).domainUid(info.getDomainUid());
  }

+  public ThreadLoggingContext presenceInfo(ClusterPresenceInfo info) {
+    return namespace(info.getNamespace());


Will this be confusing when there are multiple clusters (potential with same name) in the same namespace?

I don't think we can have multiple same-named clusters in the same namespace; K8S disallows it.

ok. As we discussed, I meant the name of the cluster itself and not the name of the cluster resource. Should we also include the cluster resource name is the ThreadLoggingContext?

Good point. We probably should if we also want the cluster resource name to be in the logging headers. But we need to do a thorough investigation to identify the log messages that need to have the cluster resource name as a header. I would suggest we do that as a separate task.

ankedia · 2022-11-29T21:27:32Z

operator/src/main/java/oracle/kubernetes/operator/makeright/MakeRightOperationImpl.java

+import oracle.kubernetes.operator.MakeRightOperation;
+
+/**
+ * A factory which creates and executes steps to align the cached domain status with the value read from Kubernetes.


cached domain status -> cached domain and cluster status.

ankedia · 2022-11-29T21:28:09Z

operator/src/main/java/oracle/kubernetes/operator/makeright/MakeRightClusterOperationImpl.java

+import oracle.kubernetes.operator.work.Step;
+
+/**
+ * A factory which creates and executes steps to align the cached domain status with the value read from Kubernetes.


cached domain status -> cached cluster status.

All comments in javadoc have been addressed. thanks.

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

alai8 · 2022-11-29T22:24:37Z

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

@@ -337,7 +341,7 @@ public static Step bringAdminServerUp(DomainPresenceInfo info, PodAwaiterStepFac

  @Override
  public void runMakeRight(MakeRightDomainOperation operation) {
-    final DomainPresenceInfo liveInfo = operation.getPresenceInfo();
+    final DomainPresenceInfo liveInfo = (DomainPresenceInfo) operation.getPresenceInfo();


Consider adding generics to MakeRightOperation to remove the need of this cast. Please see suggested changes at 54122bb

Good suggestion. will do.

ankedia

LGTM

sonarqubecloud · 2022-11-30T16:39:05Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

92.9% Coverage
0.0% Duplication

…ainEvents#testK8SEventsDelete (oracle#3704) * Generate cluster Created/Deleted/Changed events for unreferenced cluster resources Co-authored-by: Anthony Lai <anthony.lai@oracle.com>

…ainEvents#testK8SEventsDelete (#3704) * Generate cluster Created/Deleted/Changed events for unreferenced cluster resources Co-authored-by: Anthony Lai <anthony.lai@oracle.com>

doxiao added 16 commits November 16, 2022 12:00

Generate cluster Created/Deleted/Changed events for unreferenced clus…

8e8c833

…ter resources

Refine the solution

2b26d85

Minor changes

944a9fe

Deleting adomain wait for the current in-flight make right operation …

fef631d

…to complete

Let clusterEventOnly MakeRightOperation interrupt

2cb3a46

Cleanup

af3ca18

Use cluster event only MakeRightOperation for all cases and deleting …

c757e4d

…operation does not wait

Minor javadoc fix

d9a2468

Cluster events do not care about the domain object any more

a165997

Minor adjustments

592f0a3

Separate domain and cluster make right

d4fbe2d

fix javadoc

7b78a04

Test changes

3a51dbc

Fixing invovled object

6b04d85

Fix the missing namespace on the events

d365277

cleanup

805c52f

oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Nov 29, 2022

code cleanup

72ac182

doxiao requested review from rjeberhard, lennyphan, alai8 and ankedia November 29, 2022 19:27

doxiao marked this pull request as ready for review November 29, 2022 19:29

Merge remote-tracking branch 'origin/main' into owls104087-fix-new2

4f64c4d

ankedia reviewed Nov 29, 2022

View reviewed changes

alai8 reviewed Nov 29, 2022

View reviewed changes

doxiao and others added 4 commits November 29, 2022 17:50

Fix javadoc

0bf365f

introduce generics to MakeRightOperation

c0700ed

More code change for generics

1603107

Cleanup

45d813c

doxiao added 3 commits November 29, 2022 22:31

More cleanup

fa12017

More javadoc changes

80979d6

Cleanup to javadoc

26144be

ankedia approved these changes Nov 30, 2022

View reviewed changes

rjeberhard approved these changes Nov 30, 2022

View reviewed changes

alai8 approved these changes Nov 30, 2022

View reviewed changes

rjeberhard merged commit 31b8677 into main Dec 1, 2022

rjeberhard deleted the owls104087-fix-new2 branch December 1, 2022 12:27

Owls104087 (refactored) fix an intermitent failure in ItKubernetesDomainEvents#testK8SEventsDelete #3704

Owls104087 (refactored) fix an intermitent failure in ItKubernetesDomainEvents#testK8SEventsDelete #3704

Uh oh!

Conversation

doxiao commented Nov 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

doxiao Nov 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ankedia left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Nov 30, 2022

Uh oh!

Uh oh!

doxiao commented Nov 29, 2022 •

edited

Loading

doxiao Nov 30, 2022 •

edited

Loading