Fix support of NodePort for Advanced Network setup [CN-752] #23766
Conversation
SeriyBg
added
Type: Defect
Source: Internal
AyberkSorgun
changed the title
| @@ -58,7 +58,7 @@ default Integer extractPort(JsonValue subsetJson) { | |||
| } | |||
| } | |||
| } | |||
| if (ports.size() == 1) { | |||
| if (ports.size() > 0) { | |||
There was a problem hiding this comment.
Is there a reason why changed the size check here?
There was a problem hiding this comment.
The reason is that for the advanced network setup, it will return a null port. And although it will work when the port is specified in the config, without it specified it will still be null
There was a problem hiding this comment.
But when I check the code I see we only care about the first port. Is this the way we want to go with this?
There was a problem hiding this comment.
Ideally, not, but it requires the changes in the core API so that one endpoint can have separate ports for different protocols. Until this change is done, the whole part is handled by the core that is returning the port configured in the advanced-network section
There was a problem hiding this comment.
I have an additional question, do we depend on some port being the first element in ports list?
There was a problem hiding this comment.
No, as with the advanced network enabled, the Hazelcast will override the port based on the socket protocol. For the other cases, it is recommended to set the hazelcast-service-port port name
There was a problem hiding this comment.
@SeriyBg Hi, note on this. This change appears to be breaking our deployment with 5.3.0. The port name hazelcast-service-port is too long for k8s port names, so the check never matches. In k8s we have:
"ports": [
{
"name": "http",
"port": 8443,
"protocol": "TCP"
},
{
"name": "admin",
"port": 8444,
"protocol": "TCP"
},
{
"name": "hazelcast",
"port": 5701,
"protocol": "TCP"
}
]It does not find the hazelcast port.
#23931) Backport of: #23766 Checklist: - [X] Labels (`Team:`, `Type:`, `Source:`, `Module:`) and Milestone set - [X] Label `Add to Release Notes` or `Not Release Notes content` set - [X] Request reviewers if possible - [X] Send backports/forwardports if fix needs to be applied to past/future releases - [X] New public APIs have `@Nonnull/@Nullable` annotations - [X] New public APIs have `@since` tags in Javadoc
#23932) Backport of: #23766 Checklist: - [X] Labels (`Team:`, `Type:`, `Source:`, `Module:`) and Milestone set - [X] Label `Add to Release Notes` or `Not Release Notes content` set - [X] Request reviewers if possible - [X] Send backports/forwardports if fix needs to be applied to past/future releases - [X] New public APIs have `@Nonnull/@Nullable` annotations - [X] New public APIs have `@since` tags in Javadoc
When Advanced networking is enabled, the K8s plugin might discover several endpoints (per each port) for each member's pod. The changes ensure that the discovery plugin will match only the private IP per endpoint, ignoring the port values.
Checklist:
Team:,Type:,Source:,Module:) and Milestone setAdd to Release NotesorNot Release Notes contentset@Nonnull/@Nullableannotations@sincetags in Javadoc