Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix [TCP/IP join] members configuration override [HZ-2786] #25075

Closed

Conversation

orcunc
Copy link
Contributor

@orcunc orcunc commented Jul 26, 2023

This issue is reported by community.

When HZ_NETWORK_JOIN_TCPIP_MEMBERS env variable is set, the TcpIpConfig members field is appended instead of getting replaced

  1. Changed the MemberDomConfigProcessor to replace the given value. Added a new test method to test it
  2. There were a lot of unnecessary "throws Exception" in the test class. Removed them

Fixes #21272
Jira https://hazelcast.atlassian.net/browse/HZ-2786

Checklist:

  • Labels (Team:, Type:, Source:, Module:) and Milestone set
  • Label Add to Release Notes or Not Release Notes content set
  • Request reviewers if possible

…ig members field is appended instead of getting replaced

Changed the MemberDomConfigProcessor to replace the given value. [HZ-2786]
@orcunc orcunc added this to the 5.4.0 milestone Jul 26, 2023
@orcunc orcunc self-assigned this Jul 26, 2023
@hz-devops-test
Copy link

The job Hazelcast-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file
---------ERRORS-----------
--------------------------
[ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-builder/hazelcast/src/test/java/com/hazelcast/internal/config/override/ExternalMemberConfigurationOverrideEnvTest.java:101:82: ',' is not followed by whitespace. [WhitespaceAfter]
--------------------------
[ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-builder/hazelcast/src/test/java/com/hazelcast/internal/config/override/ExternalMemberConfigurationOverrideEnvTest.java:101:93: ',' is not followed by whitespace. [WhitespaceAfter]
--------------------------

@hz-devops-test
Copy link

The job Hazelcast-pr-compiler of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file
---------ERRORS-----------
--------------------------
[ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-compiler/hazelcast/src/test/java/com/hazelcast/internal/config/override/ExternalMemberConfigurationOverrideEnvTest.java:101:82: ',' is not followed by whitespace. [WhitespaceAfter]
--------------------------
[ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-compiler/hazelcast/src/test/java/com/hazelcast/internal/config/override/ExternalMemberConfigurationOverrideEnvTest.java:101:93: ',' is not followed by whitespace. [WhitespaceAfter]
--------------------------

@hz-devops-test
Copy link

The job Hazelcast-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file
--------------------------
-------TEST FAILURE-------
--------------------------
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   XMLConfigBuilderTest>AbstractConfigBuilderTest.testCompleteAdvancedNetworkConfig:527->HazelcastTestSupport.assertContains:815 Collection [10.10.1.12] (1) didn't contain expected '10.10.1.11'
[INFO] 
[ERROR] Tests run: 54781, Failures: 1, Errors: 0, Skipped: 243
[INFO] 

[ERROR] There are test failures.

…eeds to append to member list but external configuration needs to replace the existing list . Added a new flag to MemberDomConfigProcessor to indicate where this class is called from [HZ-2786]
@vbekiaris
Copy link
Contributor

I don't think we should change the behaviour of the env variable to overwrite instead of append. Previously, there was an explicit effort to ensure env variables patch an existing config - not overwrite. e.g. see #17878 and related issue #17874.

Maybe the solution is to get rid of the default 127.0.0.1 IP in the members list in hazelcast-docker.xml mentioned in #21272 ?

@orcunc
Copy link
Contributor Author

orcunc commented Jul 31, 2023

There is no direct hazelcast-docker.xml file . I think a hazelcast-default.xml is copied as hazelcast-docker.xml in the distribution/src/assembly/assembly-descriptor.xml file. But I am not sure. I could not understand the build system

Do you think we should delete the line in
hazelcast/src/main/resources/hazelcast-default.xml file ?
Or close this PR without changing anything ?

@orcunc orcunc closed this Aug 15, 2023
@orcunc orcunc deleted the fix/5.4/hz-2786_config_override branch August 15, 2023 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TCP/IP join] members configuration override appends values instead of fully overriding list
3 participants