Add cluster ip for Kubernetes Nat Manager #1156

matkt · 2020-06-26T14:35:15Z

Signed-off-by: Karim TAAM karim.t2am@gmail.com

PR description

Kubernetes nat manager could work with LoadBalancer services. This PR add compatibility with ClusterIP services.
A code refactor was made in order to facilitate the addition of new services thereafter such as the NodePort

Sample of ClusterIP service :

---
apiVersion: v1
kind: Service
metadata:
  labels:
    app.kubernetes.io/name: besu
    app.kubernetes.io/release: "1.0.0"
  name: besu
spec:
  ports:
  - name: "json-rpc"
    port: 8546
    targetPort: 8545
  - name: "rlpx"
    port: 30303
    targetPort: 30303
  selector:
    app.kubernetes.io/name: besu
    app.kubernetes.io/release: "1.0.0"
  type: ClusterIP

You can find the IP that will be used by BESU by launching the following command

kubectl describe services besu

Name:              besu
Namespace:         default
Labels:            app.kubernetes.io/name=besu
                   app.kubernetes.io/release=1.0.0
Annotations:       kubectl.kubernetes.io/last-applied-configuration:
                     {"apiVersion":"v1","kind":"Service","metadata":{"annotations":{},"labels":{"app.kubernetes.io/name":"besu","app.kubernetes.io/release":"1....
Selector:          app.kubernetes.io/name=besu,app.kubernetes.io/release=1.0.0
Type:              ClusterIP
IP:                **<IP USED BY KUBERNETES NAT MANAGER>**
Port:              json-rpc  8546/TCP
TargetPort:        8545/TCP
Endpoints:         <none>
Port:              rlpx  30303/TCP
TargetPort:        30303/TCP
Endpoints:         <none>
Session Affinity:  None
Events:            <none>

Update of the fallback mechanism

If the nat-method is equal to AUTO (default). A bad configuration (invalid service, ingress not found, etc.) will cause a switch to the NONE mode.
If the nat-method is equal to KUBERNETES and Xnat-method-fallback-enabled is equal to true. A bad configuration will cause a switch to the NONE mode.
If the nat-method is equal to KUBERNETES and Xnat-method-fallback-enabledis equal to false. A bad configuration will cause a stop of BESU in order to indicate to him that the automatic configuration has failed

New error message added

If you are trying to use not implemented service you will now have an issue :

Failed update information using pod metadata : NodePort is not implemented.

Tested

Test	Result expected
Deploy with a ClusterIP	Work	OK
Deploy with a LoadBalancer	Work	OK
Deploy with a NodePort (AUTO)	Throw an exception and switch to NONE	OK
Deploy with a Missing service (AUTO)	Throw an exception and switch to NONE	OK
Deploy without fallback (KUBERNETES and fallback=false)	Stop besu with an exception	OK
Deploy without fallback (KUBERNETES and fallback=true)	Throw an exception and switch to NONE	OK

Fixed Issue(s)

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

AbdelStark

Changes requested

AbdelStark · 2020-06-26T16:23:13Z

nat/src/main/java/org/hyperledger/besu/nat/NatService.java

-                + e.getMessage()
-                + ". NONE mode will be used");
-        disableNatManager();
+            String.format(


Don't use String.format, use standard logging placeholder instead.

AbdelStark · 2020-06-26T16:26:16Z

nat/src/main/java/org/hyperledger/besu/nat/kubernetes/service/ClusterIpBasedDetector.java

+
+import io.kubernetes.client.models.V1Service;
+
+public class ClusterIpBasedDetector implements IpDetector {


Not worth creating a class for such a simple implementation IMO. You can inject the code as a lambda directly.

AbdelStark · 2020-06-26T16:26:35Z

nat/src/main/java/org/hyperledger/besu/nat/kubernetes/KubernetesNatManager.java

+    final String serviceType = v1Service.getSpec().getType();
+    switch (KubernetesServiceType.fromName(serviceType)) {
+      case CLUSTER_IP:
+        return new ClusterIpBasedDetector(v1Service);


Return a lambda instead IMO

AbdelStark · 2020-06-26T16:29:17Z

nat/src/test/java/org/hyperledger/besu/nat/NatServiceTest.java

@@ -197,6 +197,24 @@ public void assertThatManagerSwitchToNoneForInvalidNatEnvironment()
    assertThat(natService.queryLocalIPAddress(fallbackLocalIp)).isEqualTo(fallbackLocalIp);
  }

+  @Test(expected = RuntimeException.class)


Use assertThatThrownBy instead.
Example here:

besu/besu/src/test/java/org/hyperledger/besu/chainexport/RlpBlockExporterTest.java

Line 213 in 71f2872

assertThatThrownBy(() -> exporter.exportBlocks(outputPath, Optional.of(-1L), Optional.empty()))

AbdelStark · 2020-06-26T16:29:34Z

nat/src/test/java/org/hyperledger/besu/nat/docker/DockerNatManagerTest.java

@@ -61,7 +63,14 @@ public void assertThatExternalIPIsEqualToRemoteHost()
  @Test
  public void assertThatExternalIPIsEqualToDefaultHostIfIpDetectorCannotRetrieveIP()
      throws ExecutionException, InterruptedException {
-    when(hostBasedIpDetector.detectExternalIp()).thenReturn(Optional.empty());
+    NatManager natManager =


You right . Done

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

AbdelStark

Looks good to me.

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

AbdelStark

Looks good to me.

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

AbdelStark

Looks good to me.

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

Add #1156 and #1149 Signed-off-by: Karim TAAM <karim.t2am@gmail.com> Co-authored-by: Ratan (Rai) Sur <ratan.r.sur@gmail.com>

Add hyperledger#1156 and hyperledger#1149 Signed-off-by: Karim TAAM <karim.t2am@gmail.com> Co-authored-by: Ratan (Rai) Sur <ratan.r.sur@gmail.com> (cherry picked from commit baa11a2) Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

Add #1156 and #1149 Signed-off-by: Karim TAAM <karim.t2am@gmail.com> Co-authored-by: Ratan (Rai) Sur <ratan.r.sur@gmail.com> (cherry picked from commit baa11a2) Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

matkt added 2 commits June 26, 2020 16:08

add cluster ip for nat manager

0de3ebe

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

Merge branch 'upstream/master' into feature/cluster-ip-for-nat-manager

53387c1

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

matkt marked this pull request as ready for review June 26, 2020 15:17

AbdelStark suggested changes Jun 26, 2020

View reviewed changes

fix review issues

02a9b1b

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

AbdelStark approved these changes Jun 26, 2020

View reviewed changes

Merge branch 'upstream/master' into feature/cluster-ip-for-nat-manager

1db8b0a

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

matkt self-assigned this Jun 29, 2020

timbeiko added this to the Chupacabra Sprint 68 milestone Jun 30, 2020

matkt added 2 commits July 1, 2020 14:16

change Xnat-kube-pod-name to Xnat-kube-service-name

90eecea

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

Merge commit 'master' into feature/cluster-ip-for-nat-manager

3e9e56b

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

AbdelStark approved these changes Jul 1, 2020

View reviewed changes

matkt added 2 commits July 1, 2020 14:41

fix test

2fb0de1

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

add flag for the nat method fallback mechanism

8a60dae

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

AbdelStark approved these changes Jul 1, 2020

View reviewed changes

fix typo

4322991

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

matkt merged commit 50db46f into hyperledger:master Jul 1, 2020

matkt added the doc-change-required Indicates an issue or PR that requires doc to be updated label Jul 1, 2020

AbdelStark removed the doc-change-required Indicates an issue or PR that requires doc to be updated label Jul 1, 2020

matkt added the doc-change-required Indicates an issue or PR that requires doc to be updated label Jul 24, 2020

matkt mentioned this pull request Jul 24, 2020

Update CHANGELOG.md for 1.5.1 release #1256

Merged

matkt added a commit that referenced this pull request Jul 24, 2020

Update CHANGELOG.md for 1.5.1 (#1256)

baa11a2

Add #1156 and #1149 Signed-off-by: Karim TAAM <karim.t2am@gmail.com> Co-authored-by: Ratan (Rai) Sur <ratan.r.sur@gmail.com>

bgravenorst removed the doc-change-required Indicates an issue or PR that requires doc to be updated label Mar 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cluster ip for Kubernetes Nat Manager #1156

Add cluster ip for Kubernetes Nat Manager #1156

matkt commented Jun 26, 2020 •

edited

Loading

AbdelStark left a comment

AbdelStark Jun 26, 2020

matkt Jun 26, 2020

AbdelStark Jun 26, 2020

matkt Jun 26, 2020

AbdelStark Jun 26, 2020

matkt Jun 26, 2020

AbdelStark Jun 26, 2020

matkt Jun 26, 2020

AbdelStark Jun 26, 2020

matkt Jun 26, 2020

AbdelStark left a comment

AbdelStark left a comment

AbdelStark left a comment


		import io.kubernetes.client.models.V1Service;

		public class ClusterIpBasedDetector implements IpDetector {

Add cluster ip for Kubernetes Nat Manager #1156

Add cluster ip for Kubernetes Nat Manager #1156

Conversation

matkt commented Jun 26, 2020 • edited Loading

PR description

Update of the fallback mechanism

New error message added

Tested

Fixed Issue(s)

AbdelStark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AbdelStark left a comment

Choose a reason for hiding this comment

AbdelStark left a comment

Choose a reason for hiding this comment

AbdelStark left a comment

Choose a reason for hiding this comment

matkt commented Jun 26, 2020 •

edited

Loading