Nuke Consul. #2452

rabbah · 2017-07-01T15:40:50Z

A quick attempt to strip consul - mostly because I don't want to understand why it doesn't deploy with docker-for-mac.

Need to adjust all the deployment parameters (maybe cherry picking from #2277 and merging PRs is the way to go.

Related to #2254..

rabbah · 2017-07-01T16:43:19Z

one issue found is that our configuration of the db run on "localhost" but the deployment of the containers is on the remote host which means the precomputed db_prefix will not match.

rabbah · 2017-07-01T17:22:34Z

@ddragosd I didn't squash my last two commits in case they're helpful to you. I'm happy to abandon this pr as noted I'm trying to get on docker for mac and didn't want to deal with consul.

markusthoemmes · 2017-07-02T02:49:16Z

core/controller/src/main/scala/whisk/core/loadBalancer/InvokerSupervision.scala

@@ -142,7 +139,6 @@ class InvokerPool(

 def publishStatus() = {
 val json = invokerStatus.toMap.mapValues(_.asString).toJson


remove this line as well.

markusthoemmes · 2017-07-02T02:49:35Z

core/controller/src/main/scala/whisk/core/loadBalancer/InvokerSupervision.scala

@@ -142,7 +139,6 @@ class InvokerPool(

 def publishStatus() = {


Rename to printStatus?

how about logStatus?

ddragosd · 2017-07-02T06:38:40Z

You're making it easier with this PR @rabbah 😄 ; we can merge this one instead, and drop #2277. I'll try it as soon as I get a chance.

markusthoemmes · 2017-07-02T10:07:33Z

ansible/roles/controller/tasks/deploy.yml

@@ -15,7 +15,7 @@
 mode: 0777
 become: true

- name: (re)start controller
+- name: (re)start controller "{{ db.whisk.actions }}"


I assume the db print is a "debug" statement?

yes - will remove; will need to resolve the db prefix without consul; we were getting away with this undetected.

markusthoemmes · 2017-07-02T10:09:20Z

ansible/roles/invoker/tasks/deploy.yml

+ -e DOCKER_REGISTRY={{ docker_registry }}
+ -e DOCKER_IMAGE_PREFIX={{ docker_image_prefix }}
+ -e DOCKER_IMAGE_TAG={{ docker_image_tag }}
+ -e INVOKER_CONTAINER_NETWORK=bridge


I don't think this was hardcoded to "bridge" before, was it? This needs to be configurable.

markusthoemmes · 2017-07-02T10:12:49Z

core/invoker/src/main/scala/whisk/core/container/ContainerUtils.scala

@@ -92,14 +92,13 @@ trait ContainerUtils {
 val cpuArg = Array("-c", cpuShare.toString)
 val memoryArg = Array("-m", s"${limits.memory.megabytes}m", "--memory-swap", s"${limits.memory.megabytes}m")
 val capabilityArg = Array("--cap-drop", "NET_RAW", "--cap-drop", "NET_ADMIN")
- val consulServiceIgnore = Array("-e", "SERVICE_IGNORE=true")


Needs to be changed in the new pool as well, see DockerContainer.scala line 75 (you probably didn't get it as a match in your directory-wide "consul" search? 😆 )

:) thanks for the catch!!

markusthoemmes · 2017-07-02T10:15:47Z

docs/about.md

@@ -81,13 +81,9 @@ The record of the action contains mainly the code to execute (shown above) and d

 In this particular case, our action doesn’t take any parameters (the function’s parameter definition is an empty list), thus we assume we haven’t set any default parameters and haven’t sent any specific parameters to the action, making for the most trivial case from this point-of-view.

-### Who’s there to invoke the action: Consul
+### Who’s there to invoke the action: Load balancer


Make consistent use of capitalization: Load balancer vs Load Balancer

markusthoemmes · 2017-07-02T10:16:38Z

docs/reference.md

@@ -212,7 +212,6 @@ The following packages are available to be used in the Node.js 6.9.1 environment
 - [cheerio v0.22.0](https://www.npmjs.com/package/cheerio) - Fast, flexible & lean implementation of core jQuery designed specifically for the server.
 - [cloudant v1.6.2](https://www.npmjs.com/package/cloudant) - This is the official Cloudant library for Node.js.
 - [commander v2.9.0](https://www.npmjs.com/package/commander) - The complete solution for node.js command-line interfaces.
- [consul v0.27.0](https://www.npmjs.com/package/consul) - A client for Consul, involving service discovery and configuration.


Why touch this? It's a package installed in our Node.js runtime, which has nothing to do with our system's consul.

rabbah · 2017-07-02T13:08:32Z

ansible/environments/docker-machine/group_vars/all

@@ -9,7 +9,7 @@ docker_dns: ""
 # a hostname that is resolved on the client, via /etc/hosts for example.
 whisk_api_localhost_name: "openwhisk"

-db_prefix: docker_whisk_
+db_prefix: "{{ ansible_user_id|lower }}_{{ ansible_hostname|lower }}_"


this is a change we might want to keep as hard coded.

ddragosd

LGTM. I'm trying to setup things locally and test this in the context of Docker for Mac.

One other thought I had was around ENV VARS: it's nice that we define them explicitly, one by one, as it makes everyone aware of breaking changes caused by new properties; the disadvantage is that developers need to add a new required property in 3 places. But I personally like this pain; any newly added property marked as required impacts Mesos, Kube, and docker-compose setups. Exposing props explicitly as ENV VARs seems a great approach from this point of view.

ddragosd · 2017-07-03T02:14:16Z

core/invoker/src/main/scala/whisk/core/invoker/Invoker.scala

- kafkaHost ++
- consulServer ++
- whiskVersion
+ kafkaHost


are we leaving whiskVersion out ?

it isn't used - so it's deadcode.
could add it back.

ghost · 2017-07-18T16:25:16Z

Hey all. I was just wondering what has happened to this PR? It seems like it would be greatly appreciated by everyone who does not want to heavily rely on Ansible to deploy OpenWhisk. After a couple weeks of sitting here, did it fall off of the radar?

markusthoemmes · 2017-07-18T17:04:53Z

@DanLavine we need to prepare some things first before we can get this in. It's well on our radar.

rabbah · 2017-07-18T17:46:30Z

Rebased to resolve conflicts.

We need to deal with this comment for docker-machine #2452 (comment).

rabbah · 2017-07-19T11:04:34Z

Last fix broke Travis but I didn't investigate yet.

rabbah · 2017-07-19T11:40:17Z

an invalid value, which appears to include a variable that is undefined. The error was: 'docker_image_tag' is undefined\n\nThe error appears to have been in ...

markusthoemmes · 2017-07-19T12:03:38Z

@rabbah the last travis was green already, I was aware of the issue.

rabbah · 2017-07-20T10:48:52Z

pg3/896 🔵

NOTE: we need to address the db prefix issue - it will break docker-machine users. One option is to allow for the prefix to be hard coded for that environment.

rabbah · 2017-07-23T13:10:39Z

Rebased to resolve last round of conflicts - also address docker-machine issue in last unsquashed commit for review.

Update controller config. Update params for invoker and clean up some deadcode. Hardcoded db prefix for docker-machine since db init runs on host not inside VM.

rabbah · 2017-07-31T17:26:45Z

pg3/934 🚀

markusthoemmes

LGTM

- Update controller config. - Update params for invoker and clean up some deadcode. - Hardcoded db prefix for docker-machine since db init runs on host not inside VM.

rabbah force-pushed the xconsul branch from f5d9dee to cf29c8d Compare July 1, 2017 17:19

rabbah requested a review from markusthoemmes July 1, 2017 18:27

rabbah assigned markusthoemmes Jul 1, 2017

rabbah requested a review from ddragosd July 1, 2017 18:27

rabbah added review Review for this PR has been requested and yet needs to be done. deployment labels Jul 1, 2017

markusthoemmes reviewed Jul 2, 2017

View reviewed changes

rabbah force-pushed the xconsul branch from b405ae2 to bbb9e6a Compare July 2, 2017 13:05

rabbah commented Jul 2, 2017

View reviewed changes

rabbah force-pushed the xconsul branch from bbb9e6a to fa76997 Compare July 2, 2017 13:28

ddragosd reviewed Jul 3, 2017

View reviewed changes

rabbah force-pushed the xconsul branch from fa76997 to 5c08ab2 Compare July 3, 2017 12:17

ghost mentioned this pull request Jul 6, 2017

Deploy Invoker manually apache/openwhisk-deploy-kube#34

Merged

rabbah force-pushed the xconsul branch from 5c08ab2 to c3bd9d5 Compare July 18, 2017 17:36

rabbah mentioned this pull request Jul 18, 2017

Disconnect WhiskConfig from Consul #2277

Closed

rabbah mentioned this pull request Jul 19, 2017

Use docker API via unix-socket in /var/run/docker.sock #2440

Closed

markusthoemmes force-pushed the xconsul branch from c3bd9d5 to 89597d5 Compare July 19, 2017 07:34

ghost mentioned this pull request Jul 19, 2017

Add persistence to consul and couchdb apache/openwhisk-deploy-kube#40

Closed

rabbah force-pushed the xconsul branch from 3f3f17f to 5469b84 Compare July 23, 2017 13:10

rabbah mentioned this pull request Jul 23, 2017

Force registrator and invokers containers recreation #1901

Closed

lionelvillard mentioned this pull request Jul 25, 2017

Docker for mac support #2524

Closed

markusthoemmes mentioned this pull request Jul 26, 2017

Allow multiple controllers and invokers in local environments. #2527

Closed

rabbah force-pushed the xconsul branch from 5469b84 to 4352aca Compare July 29, 2017 13:06

Nuke Consul.

0c744c0

Update controller config. Update params for invoker and clean up some deadcode. Hardcoded db prefix for docker-machine since db init runs on host not inside VM.

rabbah force-pushed the xconsul branch from 4352aca to 0c744c0 Compare July 29, 2017 13:10

rabbah mentioned this pull request Jul 31, 2017

don't attempt to run registrator if already running #2540

Closed

markusthoemmes approved these changes Jul 31, 2017

View reviewed changes

markusthoemmes merged commit e466e48 into apache:master Jul 31, 2017

ddragosd mentioned this pull request Jul 31, 2017

controller fails to start due to consul apache/openwhisk-devtools#45

Closed

csantanapr mentioned this pull request Sep 1, 2017

Docker for mac take 2 #2686

Merged

rabbah deleted the xconsul branch September 4, 2017 12:51

shazron mentioned this pull request Jan 20, 2018

Remove Consul from "The Internal flow of processing" section diagram in about.md #3207

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nuke Consul. #2452

Nuke Consul. #2452

rabbah commented Jul 1, 2017 •

edited

Loading

rabbah commented Jul 1, 2017

rabbah commented Jul 1, 2017

markusthoemmes Jul 2, 2017

markusthoemmes Jul 2, 2017

rabbah Jul 2, 2017

ddragosd commented Jul 2, 2017

markusthoemmes Jul 2, 2017

rabbah Jul 2, 2017

markusthoemmes Jul 2, 2017

markusthoemmes Jul 2, 2017

rabbah Jul 2, 2017

markusthoemmes Jul 2, 2017

markusthoemmes Jul 2, 2017

rabbah Jul 2, 2017

ddragosd left a comment

ddragosd Jul 3, 2017

rabbah Jul 3, 2017

ghost commented Jul 18, 2017

markusthoemmes commented Jul 18, 2017 •

edited

Loading

rabbah commented Jul 18, 2017 •

edited

Loading

rabbah commented Jul 19, 2017

rabbah commented Jul 19, 2017

markusthoemmes commented Jul 19, 2017

rabbah commented Jul 20, 2017 •

edited

Loading

rabbah commented Jul 23, 2017

rabbah commented Jul 31, 2017

markusthoemmes left a comment

		@@ -142,7 +139,6 @@ class InvokerPool(

		def publishStatus() = {
		val json = invokerStatus.toMap.mapValues(_.asString).toJson

Nuke Consul. #2452

Nuke Consul. #2452

Conversation

rabbah commented Jul 1, 2017 • edited Loading

rabbah commented Jul 1, 2017

rabbah commented Jul 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddragosd commented Jul 2, 2017

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

ddragosd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Jul 18, 2017

markusthoemmes commented Jul 18, 2017 • edited Loading

rabbah commented Jul 18, 2017 • edited Loading

rabbah commented Jul 19, 2017

rabbah commented Jul 19, 2017

markusthoemmes commented Jul 19, 2017

rabbah commented Jul 20, 2017 • edited Loading

rabbah commented Jul 23, 2017

rabbah commented Jul 31, 2017

markusthoemmes left a comment

Choose a reason for hiding this comment

rabbah commented Jul 1, 2017 •

edited

Loading

markusthoemmes commented Jul 18, 2017 •

edited

Loading

rabbah commented Jul 18, 2017 •

edited

Loading

rabbah commented Jul 20, 2017 •

edited

Loading