From e2ddba0d3ee332e4535977ef6c1edd9ddddef6af Mon Sep 17 00:00:00 2001 From: Jonathan Knight Date: Tue, 2 Sep 2025 10:49:13 +0300 Subject: [PATCH 1/2] Remove GC logging configuration --- api/v1/coherence_types.go | 7 ----- api/v1/constants.go | 1 - api/v1/create_job_jvmspec_test.go | 43 ++----------------------------- docs/jvm/040_gc.adoc | 32 +---------------------- pkg/runner/cmd_config.go | 11 -------- 5 files changed, 3 insertions(+), 91 deletions(-) diff --git a/api/v1/coherence_types.go b/api/v1/coherence_types.go index 72568802b..ccdcac925 100644 --- a/api/v1/coherence_types.go +++ b/api/v1/coherence_types.go @@ -1714,13 +1714,6 @@ func (in *JvmGarbageCollectorSpec) CreateEnvVars() []corev1.EnvVar { envVars = append(envVars, corev1.EnvVar{Name: EnvVarJvmGcCollector, Value: *in.Collector}) } - // Enable or disable GC logging - if in != nil && in.Logging != nil { - envVars = append(envVars, corev1.EnvVar{Name: EnvVarJvmGcLogging, Value: BoolPtrToString(in.Logging)}) - } else { - envVars = append(envVars, corev1.EnvVar{Name: EnvVarJvmGcLogging, Value: "false"}) - } - return envVars } diff --git a/api/v1/constants.go b/api/v1/constants.go index 1279f7dca..48eef3d75 100644 --- a/api/v1/constants.go +++ b/api/v1/constants.go @@ -291,7 +291,6 @@ const ( EnvVarJvmDebugAttach = "JVM_DEBUG_ATTACH" EnvVarJvmGcArgs = "JVM_GC_ARGS" EnvVarJvmGcCollector = "JVM_GC_COLLECTOR" - EnvVarJvmGcLogging = "JVM_GC_LOGGING" EnvVarJvmMemoryHeap = "JVM_HEAP_SIZE" EnvVarJvmMemoryInitialHeap = "JVM_INITIAL_HEAP_SIZE" EnvVarJvmMemoryMaxHeap = "JVM_MAX_HEAP_SIZE" diff --git a/api/v1/create_job_jvmspec_test.go b/api/v1/create_job_jvmspec_test.go index 474e821b1..42bb42429 100644 --- a/api/v1/create_job_jvmspec_test.go +++ b/api/v1/create_job_jvmspec_test.go @@ -7,9 +7,10 @@ package v1_test import ( + "testing" + coh "github.com/oracle/coherence-operator/api/v1" corev1 "k8s.io/api/core/v1" - "testing" ) func TestCreateJobWithEmptyJvmSpec(t *testing.T) { @@ -257,46 +258,6 @@ func TestCreateJobWithJvmSpecWithGarbageCollectorArgs(t *testing.T) { assertJobCreation(t, deployment, jobExpected) } -func TestCreateJobWithJvmSpecWithGarbageCollectorLoggingFalse(t *testing.T) { - - spec := coh.CoherenceResourceSpec{ - JVM: &coh.JVMSpec{ - Gc: &coh.JvmGarbageCollectorSpec{ - Logging: boolPtr(false), - }, - }, - } - - // Create the test deployment - deployment := createTestCoherenceJob(spec) - // Create expected Job - jobExpected := createMinimalExpectedJob(deployment) - addEnvVarsToAllJobContainers(jobExpected, corev1.EnvVar{Name: "JVM_GC_LOGGING", Value: "false"}) - - // assert that the Job is as expected - assertJobCreation(t, deployment, jobExpected) -} - -func TestCreateJobWithJvmSpecWithGarbageCollectorLoggingTrue(t *testing.T) { - - spec := coh.CoherenceResourceSpec{ - JVM: &coh.JVMSpec{ - Gc: &coh.JvmGarbageCollectorSpec{ - Logging: boolPtr(true), - }, - }, - } - - // Create the test deployment - deployment := createTestCoherenceJob(spec) - // Create expected Job - jobExpected := createMinimalExpectedJob(deployment) - addEnvVarsToAllJobContainers(jobExpected, corev1.EnvVar{Name: "JVM_GC_LOGGING", Value: "true"}) - - // assert that the Job is as expected - assertJobCreation(t, deployment, jobExpected) -} - func TestCreateJobWithJvmSpecWithDiagnosticsVolume(t *testing.T) { hostPath := &corev1.HostPathVolumeSource{Path: "/home/root/debug"} diff --git a/docs/jvm/040_gc.adoc b/docs/jvm/040_gc.adoc index da013b378..ef5e847a6 100644 --- a/docs/jvm/040_gc.adoc +++ b/docs/jvm/040_gc.adoc @@ -13,7 +13,7 @@ == Garbage Collector Settings The `Coherence` CRD has fields in the `jvm.gc` section to allow certain garbage collection parameters to be set. -These include GC logging, setting the collector to use and arbitrary GC arguments. +These setting the collector to use and arbitrary GC arguments. [IMPORTANT] ==== @@ -26,36 +26,6 @@ added in the Coherence resource spec. Alternatively specify a different garbage collector, ideally on a version of Java this old, use CMS. ==== -=== Enable GC Logging - -To enable GC logging set the `jvm.gc.logging` field to `true`. -For example: -[source,yaml] ----- -apiVersion: coherence.oracle.com/v1 -kind: Coherence -metadata: - name: storage -spec: - jvm: - gc: - logging: true ----- - -Setting the field to true adds the following JVM arguments to the JVM in the `coherence` container: ----- --verbose:gc --XX:+PrintGCDetails --XX:+PrintGCTimeStamps --XX:+PrintHeapAtGC --XX:+PrintTenuringDistribution --XX:+PrintGCApplicationStoppedTime --XX:+PrintGCApplicationConcurrentTime ----- - -If different GC logging arguments are required then the relevant JVM arguments can be added to either the -`jvm.args` field or the `jvm.gc.args` field. - === Set the Garbage Collector The garbage collector to use can be set using the `jvm.gc.collector` field. diff --git a/pkg/runner/cmd_config.go b/pkg/runner/cmd_config.go index 9239cb1cb..8fb615820 100644 --- a/pkg/runner/cmd_config.go +++ b/pkg/runner/cmd_config.go @@ -415,15 +415,4 @@ func populateServerDetails(details *run_details.RunDetails) { details.AddDiagnosticOption("-XX:+PrintCommandLineFlags") details.AddDiagnosticOption("-XX:+PrintFlagsFinal") } - - // Add GC logging parameters if required - if details.IsEnvTrue(v1.EnvVarJvmGcLogging) { - details.AddMemoryOption("-verbose:gc") - details.AddMemoryOption("-XX:+PrintGCDetails") - details.AddMemoryOption("-XX:+PrintGCTimeStamps") - details.AddMemoryOption("-XX:+PrintHeapAtGC") - details.AddMemoryOption("-XX:+PrintTenuringDistribution") - details.AddMemoryOption("-XX:+PrintGCApplicationStoppedTime") - details.AddMemoryOption("-XX:+PrintGCApplicationConcurrentTime") - } } From a5793d1521bc536e15bfdd5953379b2fb988c7ff Mon Sep 17 00:00:00 2001 From: Jonathan Knight Date: Wed, 3 Sep 2025 16:30:35 +0300 Subject: [PATCH 2/2] Fix tests --- api/v1/coherence_types.go | 6 --- api/v1/common_test.go | 13 ++--- api/v1/create_job_jvmspec_test.go | 3 +- api/v1/create_statefulset_jvmspec_test.go | 64 +---------------------- docs/about/04_coherence_spec.adoc | 1 - pkg/runner/runner_jvm_test.go | 50 +----------------- 6 files changed, 10 insertions(+), 127 deletions(-) diff --git a/api/v1/coherence_types.go b/api/v1/coherence_types.go index ccdcac925..28eb2a385 100644 --- a/api/v1/coherence_types.go +++ b/api/v1/coherence_types.go @@ -1691,12 +1691,6 @@ type JvmGarbageCollectorSpec struct { // +listType=atomic // +optional Args []string `json:"args,omitempty"` - // Enable the following GC logging args -verbose:gc -XX:+PrintGCDetails -XX:+PrintGCTimeStamps - // -XX:+PrintHeapAtGC -XX:+PrintTenuringDistribution -XX:+PrintGCApplicationStoppedTime - // -XX:+PrintGCApplicationConcurrentTime - // Default is true - // +optional - Logging *bool `json:"logging,omitempty"` } // CreateEnvVars creates the GC environment variables for the Coherence container. diff --git a/api/v1/common_test.go b/api/v1/common_test.go index e8bf465c7..25f126b6d 100644 --- a/api/v1/common_test.go +++ b/api/v1/common_test.go @@ -9,6 +9,11 @@ package v1_test import ( "encoding/json" "fmt" + "os" + "sort" + "strings" + "testing" + "github.com/go-test/deep" . "github.com/onsi/gomega" coh "github.com/oracle/coherence-operator/api/v1" @@ -20,10 +25,6 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" - "os" - "sort" - "strings" - "testing" ) const ( @@ -344,10 +345,6 @@ func createMinimalExpectedPodSpec(deployment coh.CoherenceResource) corev1.PodTe Name: "COHERENCE_WKA", Value: deployment.GetWKA(), }, - { - Name: "JVM_GC_LOGGING", - Value: "false", - }, { Name: "JVM_USE_CONTAINER_LIMITS", Value: "true", diff --git a/api/v1/create_job_jvmspec_test.go b/api/v1/create_job_jvmspec_test.go index 42bb42429..b7111b644 100644 --- a/api/v1/create_job_jvmspec_test.go +++ b/api/v1/create_job_jvmspec_test.go @@ -242,8 +242,7 @@ func TestCreateJobWithJvmSpecWithGarbageCollectorArgs(t *testing.T) { spec := coh.CoherenceResourceSpec{ JVM: &coh.JVMSpec{ Gc: &coh.JvmGarbageCollectorSpec{ - Args: []string{"-XX:GC-ArgOne", "-XX:GC-ArgTwo"}, - Logging: nil, + Args: []string{"-XX:GC-ArgOne", "-XX:GC-ArgTwo"}, }, }, } diff --git a/api/v1/create_statefulset_jvmspec_test.go b/api/v1/create_statefulset_jvmspec_test.go index 9588a35b5..2ed382924 100644 --- a/api/v1/create_statefulset_jvmspec_test.go +++ b/api/v1/create_statefulset_jvmspec_test.go @@ -7,9 +7,10 @@ package v1_test import ( + "testing" + coh "github.com/oracle/coherence-operator/api/v1" corev1 "k8s.io/api/core/v1" - "testing" ) func TestCreateStatefulSetWithEmptyJvmSpec(t *testing.T) { @@ -236,67 +237,6 @@ func TestCreateStatefulSetWithJvmSpecWithGarbageCollector(t *testing.T) { assertStatefulSetCreation(t, deployment, stsExpected) } -func TestCreateStatefulSetWithJvmSpecWithGarbageCollectorArgs(t *testing.T) { - - spec := coh.CoherenceResourceSpec{ - JVM: &coh.JVMSpec{ - Gc: &coh.JvmGarbageCollectorSpec{ - Args: []string{"-XX:GC-ArgOne", "-XX:GC-ArgTwo"}, - Logging: nil, - }, - }, - } - - // Create the test deployment - deployment := createTestDeployment(spec) - // Create expected StatefulSet - stsExpected := createMinimalExpectedStatefulSet(deployment) - addEnvVarsToAll(stsExpected, corev1.EnvVar{Name: "JVM_GC_ARGS", Value: "-XX:GC-ArgOne -XX:GC-ArgTwo"}) - - // assert that the StatefulSet is as expected - assertStatefulSetCreation(t, deployment, stsExpected) -} - -func TestCreateStatefulSetWithJvmSpecWithGarbageCollectorLoggingFalse(t *testing.T) { - - spec := coh.CoherenceResourceSpec{ - JVM: &coh.JVMSpec{ - Gc: &coh.JvmGarbageCollectorSpec{ - Logging: boolPtr(false), - }, - }, - } - - // Create the test deployment - deployment := createTestDeployment(spec) - // Create expected StatefulSet - stsExpected := createMinimalExpectedStatefulSet(deployment) - addEnvVarsToAll(stsExpected, corev1.EnvVar{Name: "JVM_GC_LOGGING", Value: "false"}) - - // assert that the StatefulSet is as expected - assertStatefulSetCreation(t, deployment, stsExpected) -} - -func TestCreateStatefulSetWithJvmSpecWithGarbageCollectorLoggingTrue(t *testing.T) { - - spec := coh.CoherenceResourceSpec{ - JVM: &coh.JVMSpec{ - Gc: &coh.JvmGarbageCollectorSpec{ - Logging: boolPtr(true), - }, - }, - } - - // Create the test deployment - deployment := createTestDeployment(spec) - // Create expected StatefulSet - stsExpected := createMinimalExpectedStatefulSet(deployment) - addEnvVarsToAll(stsExpected, corev1.EnvVar{Name: "JVM_GC_LOGGING", Value: "true"}) - - // assert that the StatefulSet is as expected - assertStatefulSetCreation(t, deployment, stsExpected) -} - func TestCreateStatefulSetWithJvmSpecWithDiagnosticsVolume(t *testing.T) { hostPath := &corev1.HostPathVolumeSource{Path: "/home/root/debug"} diff --git a/docs/about/04_coherence_spec.adoc b/docs/about/04_coherence_spec.adoc index eb40cdf91..7dde6460a 100644 --- a/docs/about/04_coherence_spec.adoc +++ b/docs/about/04_coherence_spec.adoc @@ -604,7 +604,6 @@ JvmGarbageCollectorSpec is options for managing the JVM garbage collector. | Field | Description | Type | Required m| collector | The name of the JVM garbage collector to use. G1 - adds the -XX:+UseG1GC option CMS - adds the -XX:+UseConcMarkSweepGC option Parallel - adds the -XX:+UseParallelGC Default - use the JVMs default collector The field value is case insensitive If not set G1 is used. If set to a value other than those above then the default collector for the JVM will be used. m| *string | false m| args | Args specifies the GC options to pass to the JVM. m| []string | false -m| logging | Enable the following GC logging args -verbose:gc -XX:+PrintGCDetails -XX:+PrintGCTimeStamps -XX:+PrintHeapAtGC -XX:+PrintTenuringDistribution -XX:+PrintGCApplicationStoppedTime -XX:+PrintGCApplicationConcurrentTime Default is true m| *bool | false |=== <> diff --git a/pkg/runner/runner_jvm_test.go b/pkg/runner/runner_jvm_test.go index 96912fc0e..0f4b345fd 100644 --- a/pkg/runner/runner_jvm_test.go +++ b/pkg/runner/runner_jvm_test.go @@ -7,12 +7,13 @@ package runner import ( + "testing" + . "github.com/onsi/gomega" coh "github.com/oracle/coherence-operator/api/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" - "testing" ) func TestJvmArgsEmpty(t *testing.T) { @@ -331,53 +332,6 @@ func TestJvmGarbageCollectorZGC(t *testing.T) { g.Expect(e.OsCmd.Args).To(ConsistOf(expected)) } -func TestJvmGarbageCollectorLoggingTrue(t *testing.T) { - g := NewGomegaWithT(t) - - d := &coh.Coherence{ - ObjectMeta: metav1.ObjectMeta{Name: "test"}, - Spec: coh.CoherenceStatefulSetResourceSpec{ - CoherenceResourceSpec: coh.CoherenceResourceSpec{ - JVM: &coh.JVMSpec{ - Gc: &coh.JvmGarbageCollectorSpec{ - Logging: ptr.To(true), - }, - }, - }, - }, - } - - expectedArgs := append(GetExpectedArgsFileContent(), - "-verbose:gc", - "-XX:+PrintGCDetails", - "-XX:+PrintGCTimeStamps", - "-XX:+PrintHeapAtGC", - "-XX:+PrintTenuringDistribution", - "-XX:+PrintGCApplicationStoppedTime", - "-XX:+PrintGCApplicationConcurrentTime") - - verifyConfigFilesWithArgs(t, d, expectedArgs) - - args := []string{"server", "--dry-run"} - env := EnvVarsFromDeployment(t, d) - - e, err := ExecuteWithArgsAndNewViper(env, args) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(e).NotTo(BeNil()) - g.Expect(e.OsCmd).NotTo(BeNil()) - - g.Expect(e.OsCmd.Dir).To(Equal(TestAppDir)) - g.Expect(e.OsCmd.Path).To(Equal(GetJavaCommand())) - g.Expect(e.OsCmd.Args).To(ConsistOf(GetMinimalExpectedArgsWith(t, - "-verbose:gc", - "-XX:+PrintGCDetails", - "-XX:+PrintGCTimeStamps", - "-XX:+PrintHeapAtGC", - "-XX:+PrintTenuringDistribution", - "-XX:+PrintGCApplicationStoppedTime", - "-XX:+PrintGCApplicationConcurrentTime"))) -} - func TestJvmGarbageCollectorArgsEmpty(t *testing.T) { g := NewGomegaWithT(t)