lightbend / reactive-cli

https://developer.lightbend.com/docs/lightbend-orchestration/current/
Apache License 2.0
23 stars 16 forks source link

reactive-cli not working with reactive-lib 1.5.0 #177

Closed octonato closed 5 years ago

octonato commented 6 years ago

The yml file generated by reactive-cli will configure the akka management http to use port name "akka-mngt-http", but Cluster Bootstrap in reactive-lib will do lookup using port name "management". As a consequence, the cluster bootstrap will never find other nodes.

This was probably introduced when we updated the akka-management version in reactive-lib (done by your truly).

schrepfler commented 6 years ago

Shouldn't this be a bug in https://github.com/akka/akka-management? If pod-port-name is declared in your application.conf and set to a value then the discovery needs to obey this value and lookup the port with that name.

    kubernetes-api {
      pod-namespace = "default"
      pod-label-selector = "appName=%s"
      pod-port-name = "akka-mgmt-http"
    }
chbatey commented 6 years ago

I think we should remove pod-port-name has Akka Management has concept of port in the lookup.

chbatey commented 6 years ago

Taking a look at this the pod-port-name is only used if there isn't a port in the Lookup. Maybe useful to keep as a default port. However bootstrap will always set the port in the Lookup so this is only applicable if using k8s service discovery for something else.

From the reference.conf:


    # Only used in the case that Lookup.portName is not set. Bootstrap sets this from
    # akka.management.cluster.boostrap.contact-point-discovery.port-name
    pod-port-name = "management"
schrepfler commented 6 years ago

I think a bit of a scrub/unification around the libraries or examples/docs is needed (akka management and rp and lagom).

All/most of the reference documentation and recipes use "akka-mgmt-http" as the pod-port-name and reactive-lib also seems to use this as a default. Wouldn't it make more sense that default in the reference.conf gets changed from "management" to "akka-mgmt-http"?

https://github.com/lightbend/reactive-lib/blob/13e8f9c56c9841e650ffb562dbefa046b3ebb5bd/akka-cluster-bootstrap/src/main/scala/com/lightbend/rp/akkaclusterbootstrap/package.scala#L20

TimMoore commented 6 years ago

@chbatey looks like this was the change that introduced the incompatibility https://github.com/akka/akka-management/pull/274

I'm concerned about this changing from one release to another, because reactive-cli isn't tied to a specific Akka or Akka Management release. It might be used with multiple projects, running different versions.

In this project, the port name is hardcoded: https://github.com/lightbend/reactive-cli/blob/08addd9d3fd431fa5eb05f23c8948a8f87b57366/cli/shared/src/main/scala/com/lightbend/rp/reactivecli/annotations/package.scala#L20

How can we ensure these align?

chbatey commented 6 years ago

Having the defaults of all the projects line up would be best we shouldn't rely on this to make things work, especially given these are pre 1.0 modules so less thought it given to making things backward compatible. That said we should plan to release a 1.0 of bootstrap/kubernetes API.

Safest would be to explicitly set akka.management.cluster.bootstrap.contact-point-discovery.port-name would make it work with any version of Akka Management. pod-port-name is not used for bootstrap.

eed3si9n commented 5 years ago

backward compat bandaid

First layer of fix was implemented in reactive-lib as https://github.com/lightbend/reactive-lib/pull/90/commits/9e2d59bd9fa786d3511cf6d9a34d5947c7988f57. This should provide backward compatibility with existing "akka-mgmt-http"-based reactive-cli.

migration to "management"

To migrate to the new port name,

  1. sbt-reactive-app should generate labels for endpoint names, defaulting to "management" and "remoting".
  2. reactive-cli can pick up the labels and use the configured names if available. Otherwise, fallback to old names.