From 6888d3e2506c40bed00b23e7a5bf4bcd723e0919 Mon Sep 17 00:00:00 2001 From: Jonathan Knight Date: Thu, 17 Oct 2024 17:02:00 +0300 Subject: [PATCH 1/4] The operator should not disable the Coherence IPMonitor --- api/v1/coherence_types.go | 9 +++--- api/v1/create_job_coherencespec_test.go | 1 + .../create_statefulset_coherencespec_test.go | 3 +- docs/about/04_coherence_spec.adoc | 2 +- docs/coherence/090_ipmonitor.adoc | 29 ------------------- docs/networking/020_dual_stack.adoc | 4 +-- pkg/runner/runner.go | 2 +- 7 files changed, 12 insertions(+), 38 deletions(-) delete mode 100644 docs/coherence/090_ipmonitor.adoc diff --git a/api/v1/coherence_types.go b/api/v1/coherence_types.go index 873c90360..77f99c000 100644 --- a/api/v1/coherence_types.go +++ b/api/v1/coherence_types.go @@ -356,8 +356,8 @@ type CoherenceSpec struct { // that the latest coherence.jar is being used. // +optional SkipVersionCheck *bool `json:"skipVersionCheck,omitempty"` - // Enables the Coherence IP Monitor feature. - // The Operator disables the IP Monitor by default. + // Enables or disables the Coherence IP Monitor feature. + // The IP Monitor is enabled by default. EnableIPMonitor *bool `json:"enableIpMonitor,omitempty"` // LocalPort sets the Coherence unicast port. // When manually configuring unicast ports, a single port is specified and the second port is automatically selected. @@ -506,8 +506,9 @@ func (in *CoherenceSpec) UpdatePodTemplateSpec(podTemplate *corev1.PodTemplateSp c.Env = append(c.Env, corev1.EnvVar{Name: EnvVarCohAllowEndangered, Value: strings.Join(in.AllowEndangeredForStatusHA, ",")}) } - if in.EnableIPMonitor != nil && *in.EnableIPMonitor { - c.Env = append(c.Env, corev1.EnvVar{Name: EnvVarEnableIPMonitor, Value: "TRUE"}) + if in.EnableIPMonitor != nil { + ip := strings.ToUpper(fmt.Sprintf("%v", *in.EnableIPMonitor)) + c.Env = append(c.Env, corev1.EnvVar{Name: EnvVarEnableIPMonitor, Value: ip}) } in.Management.AddSSLVolumesForPod(podTemplate, c, VolumeNameManagementSSL, VolumeMountPathManagementCerts) diff --git a/api/v1/create_job_coherencespec_test.go b/api/v1/create_job_coherencespec_test.go index fb8ec0094..2fb04d85e 100644 --- a/api/v1/create_job_coherencespec_test.go +++ b/api/v1/create_job_coherencespec_test.go @@ -310,6 +310,7 @@ func TestCreateJobWithCoherenceSpecWithIpMonitorDisabled(t *testing.T) { deployment := createTestCoherenceJob(spec) // Create expected Job jobExpected := createMinimalExpectedJob(deployment) + addEnvVarsToJob(jobExpected, coh.ContainerNameCoherence, corev1.EnvVar{Name: coh.EnvVarEnableIPMonitor, Value: "FALSE"}) // assert that the Job is as expected assertJobCreation(t, deployment, jobExpected) diff --git a/api/v1/create_statefulset_coherencespec_test.go b/api/v1/create_statefulset_coherencespec_test.go index 4001ebb01..7d2f29d05 100644 --- a/api/v1/create_statefulset_coherencespec_test.go +++ b/api/v1/create_statefulset_coherencespec_test.go @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2023, Oracle and/or its affiliates. + * Copyright (c) 2020, 2024, Oracle and/or its affiliates. * Licensed under the Universal Permissive License v 1.0 as shown at * http://oss.oracle.com/licenses/upl. */ @@ -310,6 +310,7 @@ func TestCreateStatefulSetWithCoherenceSpecWithIpMonitorDisabled(t *testing.T) { deployment := createTestDeployment(spec) // Create expected StatefulSet stsExpected := createMinimalExpectedStatefulSet(deployment) + addEnvVars(stsExpected, coh.ContainerNameCoherence, corev1.EnvVar{Name: coh.EnvVarEnableIPMonitor, Value: "FALSE"}) // assert that the StatefulSet is as expected assertStatefulSetCreation(t, deployment, stsExpected) diff --git a/docs/about/04_coherence_spec.adoc b/docs/about/04_coherence_spec.adoc index 1ae189a16..6044b5593 100644 --- a/docs/about/04_coherence_spec.adoc +++ b/docs/about/04_coherence_spec.adoc @@ -238,7 +238,7 @@ see: <> m| *bool | fa m| wka | Specify an existing Coherence deployment to be used for WKA. If an existing deployment is to be used for WKA the ExcludeFromWKA is implicitly set to true. + see: <> m| *<> | false m| skipVersionCheck | Certain features rely on a version check prior to starting the server, e.g. metrics requires >= 12.2.1.4. The version check relies on the ability of the start script to find coherence.jar but if due to how the image has been built this check is failing then setting this flag to true will skip version checking and assume that the latest coherence.jar is being used. m| *bool | false -m| enableIpMonitor | Enables the Coherence IP Monitor feature. The Operator disables the IP Monitor by default. m| *bool | false +m| enableIpMonitor | Enables or disables the Coherence IP Monitor feature. The IP Monitor is enabled by default. m| *bool | false m| localPort | LocalPort sets the Coherence unicast port. When manually configuring unicast ports, a single port is specified and the second port is automatically selected. If either of the ports are not available, then the default behavior is to select the next available port. For example, if port 9000 is configured for the first port (port1) and it is not available, then the next available port is automatically selected. The second port (port2) is automatically opened and defaults to the next available port after port1 (port1 + 1 if available). m| *int32 | false m| localPortAdjust | LocalPortAdjust sets the Coherence unicast port adjust value. To specify a range of unicast ports from which ports are selected, include a port value that represents the upper limit of the port range. m| *https://pkg.go.dev/k8s.io/apimachinery/pkg/util/intstr#IntOrString | false |=== diff --git a/docs/coherence/090_ipmonitor.adoc b/docs/coherence/090_ipmonitor.adoc deleted file mode 100644 index 813471514..000000000 --- a/docs/coherence/090_ipmonitor.adoc +++ /dev/null @@ -1,29 +0,0 @@ -/////////////////////////////////////////////////////////////////////////////// - - Copyright (c) 2021, Oracle and/or its affiliates. - Licensed under the Universal Permissive License v 1.0 as shown at - http://oss.oracle.com/licenses/upl. - -/////////////////////////////////////////////////////////////////////////////// - -= Coherence IPMonitor - -== Coherence IPMonitor - -The Coherence IPMonitor is a failure detection mechanism used by Coherence to detect machine failures. It does this by pinging the echo port, (port 7) on remote hosts that other cluster members are running on. When running in Kubernetes, every Pod has its own IP address, so it looks to Coherence like every member is on a different host. Failure detection using IPMonitor is less useful in Kubernetes than it is on physical machines or VMs, so the Operator disables the IPMonitor by default. This is configurable though and if it is felt that using IPMonitor is useful to an application, it can be re-enabled. - -To re-enable IPMonitor set the boolean flag `enableIpMonitor` in the `coherence` section of the Coherence resource yaml: - -[source,yaml] -.coherence-storage.yaml ----- -apiVersion: coherence.oracle.com/v1 -kind: Coherence -metadata: - name: storage -spec: - coherence: - enableIpMonitor: true ----- - -Setting `enableIpMonitor` will disable the IPMonitor, which is the default behaviour when `enableIpMonitor` is not specified in the yaml. diff --git a/docs/networking/020_dual_stack.adoc b/docs/networking/020_dual_stack.adoc index 7899585fb..2a5e6c8f6 100644 --- a/docs/networking/020_dual_stack.adoc +++ b/docs/networking/020_dual_stack.adoc @@ -11,13 +11,13 @@ == Dual Stack Networking This section describes using Coherence and the Operator with a dual stack Kubernetes cluster, -where Pods and Services can have both IPv4 and IPv4 interfaces. +where Pods and Services can have both IPv4 and IPv6 interfaces. [NOTE] ==== This section only really applies to making Coherence bind to the correct local IP address for inter-cluster communication. Normally for other Coherence endpoints, such as Extend, gRPC, management, metrics, etc. Coherence will bind to all -local addresses ubless specifically configured otherwise. +local addresses unless specifically configured otherwise. This means that in and environment such as dual-stack Kubernetes where a Pod has both an IPv4 and IPv6 address, those Coherence endpoints will be reachable using either the IPv4 or IPv6 address of the Pod. ==== diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index 03b714664..c0f0d9b7a 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -362,7 +362,7 @@ func createCommand(details *RunDetails) (string, *exec.Cmd, error) { // Disable IPMonitor ipMon := details.Getenv(v1.EnvVarEnableIPMonitor) - if ipMon != "TRUE" { + if strings.ToUpper(ipMon) == "FALSE" { details.addArg("-Dcoherence.ipmonitor.pingtimeout=0") } From 4a525a508f0100e4d3de8c634c685120ceccf7bb Mon Sep 17 00:00:00 2001 From: Jonathan Knight Date: Thu, 17 Oct 2024 17:58:21 +0300 Subject: [PATCH 2/4] Fix tests --- .github/workflows/trivy.yaml | 4 +++- pkg/runner/runner_coherence_test.go | 3 ++- pkg/runner/runner_test.go | 1 - 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/trivy.yaml b/.github/workflows/trivy.yaml index 33dd8ef60..48b059068 100644 --- a/.github/workflows/trivy.yaml +++ b/.github/workflows/trivy.yaml @@ -97,4 +97,6 @@ jobs: - name: Image Scan shell: bash - run: make trivy-scan + run: | + echo "${{ secrets.GITHUB_TOKEN }}" | docker login ghcr.io -u $ --password-stdin + make trivy-scan diff --git a/pkg/runner/runner_coherence_test.go b/pkg/runner/runner_coherence_test.go index d2af76187..d0b0c6135 100644 --- a/pkg/runner/runner_coherence_test.go +++ b/pkg/runner/runner_coherence_test.go @@ -405,7 +405,7 @@ func TestCoherenceEnableIpMonitor(t *testing.T) { env := EnvVarsFromDeployment(d) expectedCommand := GetJavaCommand() - expectedArgs := GetMinimalExpectedArgsWithoutPrefix("-Dcoherence.ipmonitor.pingtimeout") + expectedArgs := GetMinimalExpectedArgs() e, err := ExecuteWithArgsAndNewViper(env, args) g.Expect(err).NotTo(HaveOccurred()) @@ -436,6 +436,7 @@ func TestCoherenceDisableIpMonitor(t *testing.T) { expectedCommand := GetJavaCommand() expectedArgs := GetMinimalExpectedArgs() + expectedArgs = append(expectedArgs, "-Dcoherence.ipmonitor.pingtimeout=0") e, err := ExecuteWithArgsAndNewViper(env, args) g.Expect(err).NotTo(HaveOccurred()) diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index 225ee294b..83259e917 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -147,7 +147,6 @@ func AppendCommonExpectedArgs(args []string) []string { "-Dcoherence.metrics.http.port=9612", "-Dcoherence.distributed.persistence-mode=on-demand", "-Dcoherence.override=k8s-coherence-nossl-override.xml", - "-Dcoherence.ipmonitor.pingtimeout=0", "-Dcoherence.k8s.operator.diagnostics.dir=/coherence-operator/jvm/unknown/unknown", "-XX:HeapDumpPath=/coherence-operator/jvm/unknown/unknown/heap-dumps/unknown-unknown.hprof", "-Dcoherence.k8s.operator.can.resume.services=true", From 8447752930eb90fa866287a87fd573521356b83d Mon Sep 17 00:00:00 2001 From: Jonathan Knight Date: Fri, 18 Oct 2024 12:55:25 +0300 Subject: [PATCH 3/4] disable IP monitor in k8s tests with network policies --- test/certification/minimal.yaml | 3 +++ test/certification/persistence-active-1.yaml | 1 + test/certification/persistence-active-3.yaml | 1 + test/certification/persistence-snapshot.yaml | 1 + 4 files changed, 6 insertions(+) diff --git a/test/certification/minimal.yaml b/test/certification/minimal.yaml index 487eee1bb..17e3b0f71 100644 --- a/test/certification/minimal.yaml +++ b/test/certification/minimal.yaml @@ -2,3 +2,6 @@ apiVersion: coherence.oracle.com/v1 kind: Coherence metadata: name: certify-minimal +spec: + coherence: + enableIpMonitor: false diff --git a/test/certification/persistence-active-1.yaml b/test/certification/persistence-active-1.yaml index 644dab0b9..1a193c3c1 100644 --- a/test/certification/persistence-active-1.yaml +++ b/test/certification/persistence-active-1.yaml @@ -11,6 +11,7 @@ spec: initialDelaySeconds: 10 periodSeconds: 10 coherence: + enableIpMonitor: false cacheConfig: test-cache-config.xml management: enabled: true diff --git a/test/certification/persistence-active-3.yaml b/test/certification/persistence-active-3.yaml index 2366e3017..15feaa606 100644 --- a/test/certification/persistence-active-3.yaml +++ b/test/certification/persistence-active-3.yaml @@ -11,6 +11,7 @@ spec: initialDelaySeconds: 10 periodSeconds: 10 coherence: + enableIpMonitor: false cacheConfig: test-cache-config.xml management: enabled: true diff --git a/test/certification/persistence-snapshot.yaml b/test/certification/persistence-snapshot.yaml index 1185c7ad1..4a66c4948 100644 --- a/test/certification/persistence-snapshot.yaml +++ b/test/certification/persistence-snapshot.yaml @@ -11,6 +11,7 @@ spec: initialDelaySeconds: 10 periodSeconds: 10 coherence: + enableIpMonitor: false cacheConfig: test-cache-config.xml management: enabled: true From cd1afe66a6445bc9c108b7df1e1d28e9a19548db Mon Sep 17 00:00:00 2001 From: Jonathan Knight Date: Fri, 18 Oct 2024 14:42:44 +0300 Subject: [PATCH 4/4] disable IP monitor in k8s tests with network policies --- test/certification/certifiy_deployment_test.go | 10 ++++++++++ test/certification/certify_management_test.go | 2 ++ test/certification/certify_metrics_test.go | 2 ++ 3 files changed, 14 insertions(+) diff --git a/test/certification/certifiy_deployment_test.go b/test/certification/certifiy_deployment_test.go index e1cfcdb01..ef68004c6 100644 --- a/test/certification/certifiy_deployment_test.go +++ b/test/certification/certifiy_deployment_test.go @@ -33,6 +33,13 @@ func TestCertifyMinimalSpec(t *testing.T) { Namespace: ns, Name: "certify-minimal", }, + Spec: v1.CoherenceStatefulSetResourceSpec{ + CoherenceResourceSpec: v1.CoherenceResourceSpec{ + Coherence: &v1.CoherenceSpec{ + EnableIPMonitor: ptr.To(false), + }, + }, + }, } err := testContext.Client.Create(context.TODO(), d) @@ -56,6 +63,9 @@ func TestCertifyScaling(t *testing.T) { Spec: v1.CoherenceStatefulSetResourceSpec{ CoherenceResourceSpec: v1.CoherenceResourceSpec{ Replicas: ptr.To(int32(1)), + Coherence: &v1.CoherenceSpec{ + EnableIPMonitor: ptr.To(false), + }, ReadinessProbe: &v1.ReadinessProbeSpec{ InitialDelaySeconds: ptr.To(int32(10)), PeriodSeconds: ptr.To(int32(10)), diff --git a/test/certification/certify_management_test.go b/test/certification/certify_management_test.go index ef51b3c32..f00589430 100644 --- a/test/certification/certify_management_test.go +++ b/test/certification/certify_management_test.go @@ -34,6 +34,7 @@ func TestCertifyManagementDefaultPort(t *testing.T) { Spec: v1.CoherenceStatefulSetResourceSpec{ CoherenceResourceSpec: v1.CoherenceResourceSpec{ Coherence: &v1.CoherenceSpec{ + EnableIPMonitor: ptr.To(false), Management: &v1.PortSpecWithSSL{ Enabled: ptr.To(true), }, @@ -101,6 +102,7 @@ func TestCertifyManagementNonStandardPort(t *testing.T) { Spec: v1.CoherenceStatefulSetResourceSpec{ CoherenceResourceSpec: v1.CoherenceResourceSpec{ Coherence: &v1.CoherenceSpec{ + EnableIPMonitor: ptr.To(false), Management: &v1.PortSpecWithSSL{ Enabled: ptr.To(true), Port: ptr.To(int32(30009)), diff --git a/test/certification/certify_metrics_test.go b/test/certification/certify_metrics_test.go index d5c79c3d6..5ac335432 100644 --- a/test/certification/certify_metrics_test.go +++ b/test/certification/certify_metrics_test.go @@ -34,6 +34,7 @@ func TestCertifyMetricsDefaultPort(t *testing.T) { Spec: v1.CoherenceStatefulSetResourceSpec{ CoherenceResourceSpec: v1.CoherenceResourceSpec{ Coherence: &v1.CoherenceSpec{ + EnableIPMonitor: ptr.To(false), Metrics: &v1.PortSpecWithSSL{ Enabled: ptr.To(true), }, @@ -101,6 +102,7 @@ func TestCertifyMetricsNonStandardPort(t *testing.T) { Spec: v1.CoherenceStatefulSetResourceSpec{ CoherenceResourceSpec: v1.CoherenceResourceSpec{ Coherence: &v1.CoherenceSpec{ + EnableIPMonitor: ptr.To(false), Metrics: &v1.PortSpecWithSSL{ Enabled: ptr.To(true), Port: ptr.To(int32(9619)),