You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2018/10/10 09:31:54 UTC

svn commit: r1843415 - /tomcat/trunk/java/org/apache/catalina/tribes/membership/cloud/KubernetesMembershipProvider.java

Author: remm
Date: Wed Oct 10 09:31:54 2018
New Revision: 1843415

URL: http://svn.apache.org/viewvc?rev=1843415&view=rev
Log:
There's a "uid" in metadata, so try to use it. Also add back a needed test I accidentally removed.

Modified:
    tomcat/trunk/java/org/apache/catalina/tribes/membership/cloud/KubernetesMembershipProvider.java

Modified: tomcat/trunk/java/org/apache/catalina/tribes/membership/cloud/KubernetesMembershipProvider.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/tribes/membership/cloud/KubernetesMembershipProvider.java?rev=1843415&r1=1843414&r2=1843415&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/tribes/membership/cloud/KubernetesMembershipProvider.java (original)
+++ tomcat/trunk/java/org/apache/catalina/tribes/membership/cloud/KubernetesMembershipProvider.java Wed Oct 10 09:31:54 2018
@@ -22,6 +22,7 @@ import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.Reader;
 import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.time.Duration;
@@ -156,6 +157,8 @@ public class KubernetesMembershipProvide
                 @SuppressWarnings("unchecked")
                 LinkedHashMap<String, Object> metadata = (LinkedHashMap<String, Object>) pod.get("metadata");
                 String name = metadata.get("name").toString();
+                Object objectUid = metadata.get("uid");
+                String uid = (objectUid == null) ? name : objectUid.toString();
                 String creationTimestamp = metadata.get("creationTimestamp").toString();
                 @SuppressWarnings("unchecked")
                 LinkedHashMap<String, Object> status = (LinkedHashMap<String, Object>) pod.get("status");
@@ -164,8 +167,12 @@ public class KubernetesMembershipProvide
                 }
                 String podIP = status.get("podIP").toString();
 
-                // id = md5(hostname)
-                byte[] id = md5.digest(name.getBytes());
+                // We found ourselves, ignore
+                if (name.equals(hostName)) {
+                    continue;
+                }
+
+                byte[] id = md5.digest(uid.getBytes(StandardCharsets.US_ASCII));
                 long aliveTime = Duration.between(Instant.parse(creationTimestamp), startTime).getSeconds() * 1000; // aliveTime is in ms
 
                 MemberImpl member = null;
@@ -177,7 +184,6 @@ public class KubernetesMembershipProvide
                     log.error(sm.getString("kubernetesMembershipProvider.memberError"), e);
                     continue;
                 }
-
                 member.setUniqueId(id);
                 members.add(member);
             }



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1843415 - /tomcat/trunk/java/org/apache/catalina/tribes/membership/cloud/KubernetesMembershipProvider.java

Posted by Rémy Maucherat <re...@apache.org>.
On Mon, Oct 15, 2018 at 6:53 AM Keiichi Fujino <kf...@apache.org> wrote:

> Hi Remy.
>
> Sorry for the late reply.
> I have two comments against the current code.
>
> The one is,
> the uniqueId of the local member and the other cluster members are
> different.
>
> The CloudMembershipService#createOrUpdateLocalMember method is as follows.
> ==
> // Set localMember unique ID to md5 hash of hostname
> localMember.setUniqueId(MessageDigest.getInstance("md5")
>         .digest(InetAddress.getLocalHost().getHostName().getBytes()));
> ==
>
> The KubernetesMembershipProvider#parsePods method is as follows.
> ==
> byte[] id = md5.digest(uid.getBytes(StandardCharsets.US_ASCII));
> ==
>
> In other words, uniqueId of local member is obtained from hostname, while
> other cluster members are obtained from uid.
> In this case, some features such as RpcChannel do not work properly.
>
> We need to update the local member's uniqueId with the uid obtained from
> pod, or use a different way to align  the local member's uniqueId with
> other cluster member's uniqueId.
>

Yes, I had changed it (I didn't really like trying to use the "host name"
although it should actually work, and since kube was kindly giving that id)
but it was badly done.

Thanks for catching that mess ;)

Rémy

Re: svn commit: r1843415 - /tomcat/trunk/java/org/apache/catalina/tribes/membership/cloud/KubernetesMembershipProvider.java

Posted by Keiichi Fujino <kf...@apache.org>.
2018年10月10日(水) 18:31 <re...@apache.org>:

> Author: remm
> Date: Wed Oct 10 09:31:54 2018
> New Revision: 1843415
>
> URL: http://svn.apache.org/viewvc?rev=1843415&view=rev
> Log:
> There's a "uid" in metadata, so try to use it. Also add back a needed test
> I accidentally removed.
>
> Modified:
>
> tomcat/trunk/java/org/apache/catalina/tribes/membership/cloud/KubernetesMembershipProvider.java
>
> Modified:
> tomcat/trunk/java/org/apache/catalina/tribes/membership/cloud/KubernetesMembershipProvider.java
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/tribes/membership/cloud/KubernetesMembershipProvider.java?rev=1843415&r1=1843414&r2=1843415&view=diff
>
> ==============================================================================
> ---
> tomcat/trunk/java/org/apache/catalina/tribes/membership/cloud/KubernetesMembershipProvider.java
> (original)
> +++
> tomcat/trunk/java/org/apache/catalina/tribes/membership/cloud/KubernetesMembershipProvider.java
> Wed Oct 10 09:31:54 2018
> @@ -22,6 +22,7 @@ import java.io.InputStream;
>  import java.io.InputStreamReader;
>  import java.io.Reader;
>  import java.net.URLEncoder;
> +import java.nio.charset.StandardCharsets;
>  import java.nio.file.FileSystems;
>  import java.nio.file.Files;
>  import java.time.Duration;
> @@ -156,6 +157,8 @@ public class KubernetesMembershipProvide
>                  @SuppressWarnings("unchecked")
>                  LinkedHashMap<String, Object> metadata =
> (LinkedHashMap<String, Object>) pod.get("metadata");
>                  String name = metadata.get("name").toString();
> +                Object objectUid = metadata.get("uid");
> +                String uid = (objectUid == null) ? name :
> objectUid.toString();
>                  String creationTimestamp =
> metadata.get("creationTimestamp").toString();
>                  @SuppressWarnings("unchecked")
>                  LinkedHashMap<String, Object> status =
> (LinkedHashMap<String, Object>) pod.get("status");
> @@ -164,8 +167,12 @@ public class KubernetesMembershipProvide
>                  }
>                  String podIP = status.get("podIP").toString();
>
> -                // id = md5(hostname)
> -                byte[] id = md5.digest(name.getBytes());
> +                // We found ourselves, ignore
> +                if (name.equals(hostName)) {
> +                    continue;
> +                }
> +
> +                byte[] id =
> md5.digest(uid.getBytes(StandardCharsets.US_ASCII));
>                  long aliveTime =
> Duration.between(Instant.parse(creationTimestamp), startTime).getSeconds()
> * 1000; // aliveTime is in ms
>
>                  MemberImpl member = null;
> @@ -177,7 +184,6 @@ public class KubernetesMembershipProvide
>
>  log.error(sm.getString("kubernetesMembershipProvider.memberError"), e);
>                      continue;
>                  }
> -
>                  member.setUniqueId(id);
>                  members.add(member);
>              }
>
>
>
Hi Remy.

Sorry for the late reply.
I have two comments against the current code.

The one is,
the uniqueId of the local member and the other cluster members are
different.

The CloudMembershipService#createOrUpdateLocalMember method is as follows.
==
// Set localMember unique ID to md5 hash of hostname
localMember.setUniqueId(MessageDigest.getInstance("md5")
        .digest(InetAddress.getLocalHost().getHostName().getBytes()));
==

The KubernetesMembershipProvider#parsePods method is as follows.
==
byte[] id = md5.digest(uid.getBytes(StandardCharsets.US_ASCII));
==

In other words, uniqueId of local member is obtained from hostname, while
other cluster members are obtained from uid.
In this case, some features such as RpcChannel do not work properly.

We need to update the local member's uniqueId with the uid obtained from
pod, or use a different way to align  the local member's uniqueId with
other cluster member's uniqueId.

Another one, uniqueId should be 16 bytes.

Regards..




>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
> --
> Keiichi.Fujino
> <de...@tomcat.apache.org>