Kubernetes connector #434

skarsaune · 2020-03-23T21:42:11Z

To be reviewed after #431:
Support using Jolokia via kubernetes client

rhuss · 2020-03-24T10:53:07Z

Thanks !

Good news: Looks like I can work on the integration to master towards the end of the week.

skarsaune · 2020-06-20T14:19:03Z

@rhuss : Ready for review now that #431 is merged

rhuss · 2020-06-22T06:21:44Z

Thanks @skarsaune ! I have a busy week ahead and will revisit this PR on friday. Maybe in the meantime you could give the error messages a review again and e.g. switch from the "I" perspective to a more neutral wording as in the previous PR ? mostly to align to the style of the other error messages ...

skarsaune · 2020-06-24T19:39:20Z

Thanks @skarsaune ! I have a busy week ahead and will revisit this PR on friday. Maybe in the meantime you could give the error messages a review again and e.g. switch from the "I" perspective to a more neutral wording as in the previous PR ? mostly to align to the style of the other error messages ...

Have adapted error messages and corrected some javadoc.

rhuss

Thanks for this connector, really nice idea !

Looks good in general except that I would not allow connecting to a pod when you find multiple matchings one:

More than one pod serves by a service and the connectors is in service mode
More than one pod selected by pod pattern

The reason is, that in every pod another JVM is running so when you access e.g. a service with multiple pods then ever Jolokia requests might hit another JVM, so that any result returned is really meaningless. JMX metrics are only useful in the context of a single JVM, so for accessing such a JVM you need to select a single pod.

Also I'm not clear what happens when a connected Pod dies. Does the connector then goes in an error state ? Or does it reconnect, then potentially connected to a totally different Pod ?

In short, I would not allow services to be specified and I would enforce a pod name to connect to. Maybe a pattern for pods, but then for a reconnect you can still get a different pod.

client/kubernetes/pom.xml

.../kubernetes/src/main/java/org/jolokia/kubernetes/client/KubernetesJmxConnectionProvider.java

rhuss · 2020-06-26T11:46:29Z

client/kubernetes/src/main/java/org/jolokia/kubernetes/client/KubernetesJmxConnector.java

+          String actualNamespace = null;
+          String actualName = null;
+          for (final V1Service service : api
+              .listServiceForAllNamespaces(null, null, false, null, null, null, null, 5, null)


This can be a potentially very expensive operation. There is definitely a service list command which already restricts to a certain namespace (which I think is the usual case).

Tbh, for a service I wouldn't allow pattern matching as the service is user managed (not so much for a pod). I would require the service + namespace as mandatory.

btw, the fabric8 client doesn't have this ugly, 9 params arguments method style (which is also why I would prefer the fabric8 client)

I can make namespace mandatory

you could default to the "current" namespace if the namespace is not given. Not sure how to model 'optional' parameters in the JMX connector URL.

Ok, I'm used to always specify namespace in operations or --all-namespaces. Wondering if using default namespace will be a source of confusion. If for instance test and prod run in the same cluster I'm inclined to make namespace mandatory.

client/kubernetes/src/main/java/org/jolokia/kubernetes/client/KubernetesJmxConnector.java

rhuss · 2020-06-26T11:53:53Z

client/kubernetes/src/main/java/org/jolokia/kubernetes/client/KubernetesJmxConnector.java

+      CoreV1Api api) throws MalformedURLException {
+    try {
+      for (V1Pod pod : api
+          .listNamespacedPod(actualNamespace, false, null, null, null, null, null, null, 5, false)


there is definitely a way to query pods directly by using the service selector directly for a query.

rhuss · 2020-06-26T11:55:33Z

client/kubernetes/src/main/java/org/jolokia/kubernetes/client/KubernetesJmxConnector.java

+          .getItems()) {
+        for (V1OwnerReference ref : pod.getMetadata().getOwnerReferences()) {
+          //pod that references the service
+          if (ref.getName().equals(actualName)) {


A Pod is not owned by a Service but by e..g a Deployment, StatefulSet or a Job. So this will be true only if the deployment is called like the service which by no means is a given.

Another reason to select the pods directly with the service's label selector to find the affected pods.

Returning randomly any of the pods backing a service is very dangerous. See the review comment for why using a service for a connector is generally a bad idea.

I am aware of this, and we can document it. It is basically the same problem as when accessing Jolokia over a load balancer. It is not optimal, and can be confusing, but if it is the only way then I can still be valuable if you know what you are doing. Use cases such as executing an operation where you are not concerned with what pod you hit. But I guess the argument is that if you have access to services you also have access to pods, hence the load balancer comparison is not really justified. In light of that service support should perhaps be dropped.

rhuss · 2020-06-26T11:58:44Z

client/kubernetes/src/main/java/org/jolokia/kubernetes/client/KubernetesJmxConnector.java

+          String actualNamespace = null;
+          String actualPodName = null;
+          for (final V1Pod pod : api
+              .listPodForAllNamespaces(null, null, false, null, null, null, null, 5, null)


same argument above: In order to find a specific pod without any ambiguity, you shouldn't use patterns or at least throw an error if multiple are found. And is you don't have a pattern you can do a direct select. Listing all pods in all namespaces is a quite expensive operation.

There are 2 usecases.

You want to connect to a specific pod for investigation. Use the exact pod name.

You want to connect to any given pod of a service, as they typically display the same behaviour. One pod will be selected at connect time. When disconnected it will naturally stop working, but it depends a bit on the tool behaves, whether stats are not updated or you get explicit errors. A reconnect will connect to the first matching pod.

2. as they typically display the same behaviour

Are you sure ? As pods are spread over different nodes and since metric information is stateful, I don't regard the pods as interchangeable wrt/ to metrics. Consider os information from MBean, MBeans that access file systems etf. In a multi-arch cluster the different pods can even run on different architectures (arm vs i86 etc)

Wouldn't a user be surprised that the memory graph make a jump in jconsole if a reconnection happens ? There is no indication by JMX that it is now connected to another JVM.

However, I see your use case in favor of convenience. One idea could be that only when going over a service or deployment the JMX connector URL that kind of roulette applies. That way a user has to know what she does. However when using the 'pod' variant of the URL, then a single pod has to be selected. (that would require to also allow a "deployment", "replicaset" (or "rs"), "statefulset" and "job" kind of JMX connectors (but happy to start with service and deployment as they are probably the most common).

wdyt ?

I think the risks of using wildcards in pod url can be documented. In my current project the last 2 years shows very homogenous behavior over pods and setup is identical, so personally I would like the convenience of a URL that you can store across deploys. For instance if I inspect a pod or port forward, I tend to pick the top of the list. For JMC specifically I am thinking of both wizard / browser to interactively connect to pods, and alternatively probe pods in cluster for Jolokia (if user has configured it to). I could look at the possibility of interactively or always hard wire to a given pod.

client/kubernetes/src/main/java/org/jolokia/kubernetes/client/MinimalHttpClientAdapter.java

rhuss · 2020-06-26T12:14:11Z

client/kubernetes/src/main/java/org/jolokia/kubernetes/client/KubernetesJmxConnector.java

+public class KubernetesJmxConnector extends JolokiaJmxConnector {
+
+  private static Pattern POD_PATTERN = Pattern
+      .compile("/api/v1/namespaces/([^/]+)/pods/([^/]+)/proxy/(.+)");


Maybe instead of exposing the K8A API path literally a more concise and shorter JMX connector URL would make sense, something like service:jmx:kubernetes:pod/mynamespace/mypodname:port/path and service:jmx:kubernetes:service/mynamespace/mysvname:port/path and then translate that to the full K8s API path ?

This might be more convenient for the user on expense to be less flexible (but I'm pretty sure that the API path format won't change any time soon for Kubernetes)

Nice input. Will think about it.

Have done some investigation. Can drop the /api/v1 and proxy from the path to reduce the boilerplate. Apart from that I have not come up with any more intuitive path. The result would be service:jmx:kubernetes:/namespaces/myns/pods/mypod/path

Still, wondering if we should also drop "namespaces" and "pods" , it becomes less self documenting but briefer?

Yeah, if we skip the connection to the K8s API syntax, then I don't see a reason why not make it even shorter (as you can't reverse the path parts anyway, that style of 'named' arguments doesn't make much sense imo)

So service:jmx:kubernetes:///mynamespace/mypod/path then? Sure, can change.

…alternate authentication

…on header

…seful connections, example activemq, jvms with jvm agent

skarsaune · 2020-08-30T18:17:15Z

client/kubernetes/src/main/java/org/jolokia/kubernetes/client/KubernetesJmxConnector.java

-          String namespace = matcher.group(1);
-          String podPattern = matcher.group(2);
-          String path = matcher.group(3);
+          String namespace = matcher.group("namespace");


Note, this requires Java 1.7 , I can happily replace with numeric groups, however was not sure if Java 1.6 is feasible anymore (?)

We abandoned Java 1.6 already afair (my gosh, really not deep in Jolokia anymore :(.

Lets keep it ...

skarsaune · 2020-08-30T18:23:54Z

With the last 2 additions my use cases work satisfactory now. Things that would be good for review.

Authorization through proxy with custom header - any more elegant ideas? I did a quick PoC on using cookies, however that turned out very hard with the http client setup in fabric8
Enable specifying port - whether the JMX url still is intuitive

No further changes in the pipeline, apart from review input.

rhuss · 2020-08-31T06:56:26Z

Cool, thanks @skarsaune ! Let me try to get this in over the week and then do a release afterwards (finally)

skarsaune · 2020-10-04T12:35:31Z

@rhuss : As far as I can see everything should be resolved now, but the varying conversations overlap. Should I mark them as resolved? Or is there a status on what remains to proceed?

rhuss · 2020-10-04T15:43:37Z

@skarsaune I would say, let's merge see it and next weekend I can make a new release then. Currently I'm more in feedback-only mode with Jolokia, so happy to release and work on any feedback that is coming up then.

skarsaune force-pushed the kubernetes-connector branch 2 times, most recently from 9f7e854 to bbd595a Compare April 7, 2020 18:41

skarsaune force-pushed the kubernetes-connector branch from bbd595a to 1c5d624 Compare June 19, 2020 17:22

skarsaune marked this pull request as ready for review June 19, 2020 17:25

skarsaune force-pushed the kubernetes-connector branch from 1c5d624 to 4b98021 Compare June 24, 2020 19:38

skarsaune force-pushed the kubernetes-connector branch 3 times, most recently from 625fc9b to 6489844 Compare June 24, 2020 19:55

rhuss requested changes Jun 26, 2020

View reviewed changes

skarsaune added 3 commits July 6, 2020 12:16

First cut kubernetes support

cb7fca4

Initial support for authentication for pods in k8s

c5d7ee2

Documentation clarifications based on review

2c1af94

skarsaune force-pushed the kubernetes-connector branch from 6489844 to 2c1af94 Compare July 6, 2020 10:16

Simplify URL scheme, remove support for connecting to services

25b31a4

skarsaune force-pushed the kubernetes-connector branch from 62d2cb1 to 25b31a4 Compare July 11, 2020 10:18

skarsaune added 2 commits July 24, 2020 11:43

Switch to fabric8 kubernetes client. shortening service URL. Support …

12164cb

…alternate authentication

Revert last credentials change, needs more testing and work

b0f9875

skarsaune force-pushed the kubernetes-connector branch 2 times, most recently from 46b44ee to 4e8171c Compare August 30, 2020 17:57

Solution to problem that Kubernetes proxy strips standard Authorizati…

373fb85

…on header

skarsaune force-pushed the kubernetes-connector branch from 4e8171c to 373fb85 Compare August 30, 2020 18:07

Support specifying port in connection URL, this is required to make u…

29d250a

…seful connections, example activemq, jvms with jvm agent

skarsaune commented Aug 30, 2020

View reviewed changes

Introduce constant for alternate Authorization header

a2ff078

rhuss merged commit a8bf5f1 into jolokia:master Oct 4, 2020

andrewazores mentioned this pull request Oct 8, 2021

[Story] Jolokia Compatibility cryostatio/cryostat#683

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kubernetes connector #434

Kubernetes connector #434

skarsaune commented Mar 23, 2020

rhuss commented Mar 24, 2020

skarsaune commented Jun 20, 2020

rhuss commented Jun 22, 2020

skarsaune commented Jun 24, 2020

rhuss left a comment

rhuss Jun 26, 2020

rhuss Jun 26, 2020

skarsaune Jun 27, 2020

rhuss Jun 29, 2020

skarsaune Jul 5, 2020

rhuss Jun 26, 2020

rhuss Jun 26, 2020

rhuss Jun 26, 2020

skarsaune Jul 5, 2020

rhuss Jun 26, 2020

skarsaune Jun 29, 2020

rhuss Jun 29, 2020

skarsaune Jul 5, 2020

rhuss Jun 26, 2020

skarsaune Jun 29, 2020

skarsaune Jul 6, 2020

skarsaune Jul 11, 2020

rhuss Jul 17, 2020

skarsaune Jul 21, 2020

skarsaune Aug 30, 2020

rhuss Oct 4, 2020

skarsaune commented Aug 30, 2020

rhuss commented Aug 31, 2020

skarsaune commented Oct 4, 2020

rhuss commented Oct 4, 2020

Kubernetes connector #434

Kubernetes connector #434

Conversation

skarsaune commented Mar 23, 2020

rhuss commented Mar 24, 2020

skarsaune commented Jun 20, 2020

rhuss commented Jun 22, 2020

skarsaune commented Jun 24, 2020

rhuss 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

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

skarsaune commented Aug 30, 2020

rhuss commented Aug 31, 2020

skarsaune commented Oct 4, 2020

rhuss commented Oct 4, 2020