Skip to content
This repository has been archived by the owner on Jan 31, 2019. It is now read-only.

currently symbolapi does not use consul, if the cluster is unhealthy … #133

Merged
merged 1 commit into from
May 26, 2015

Conversation

rhelmer
Copy link
Contributor

@rhelmer rhelmer commented May 21, 2015

…then puppet apply fails for no useful reason

r? @phrawzty @jdotpz - I don't know that we'll ever really need to do any configuration here, this service is totally standalone and public (it provides an on-demand symbolication service to the Gecko Profiler, using the public S3 symbols bucket)

@phrawzty
Copy link
Contributor

Are you sure you meant to remove the class include? From your comment it seems like you meant to remove the require parameter of the service.

@jdotpz
Copy link
Contributor

jdotpz commented May 22, 2015

Well, in the scenario you outlined rhelmer, consul is hosed, we're hosed enough that symbolapi coming up bad will be the least of our worries. :)

I think it's actually valuable to keep these bits in there versus the risk of a bad puppet run. One day, we may just want to use our standard way of passing k/v to symbol nodes.

@rhelmer
Copy link
Contributor Author

rhelmer commented May 22, 2015

@phrawzty

Are you sure you meant to remove the class include? From your comment it seems like you meant to remove the require parameter of the service.

The problem is I don't want consul join to run on these nodes at all, there's no point.

@jdotpz

Well, in the scenario you outlined rhelmer, consul is hosed, we're hosed enough that symbolapi coming up bad will be the least of our worries. :)

I think it's actually valuable to keep these bits in there versus the risk of a bad puppet run. One day, we may just want to use our standard way of passing k/v to symbol nodes.

Although this app is part of the Socorro cluster, it's not really a "socorro app" per se. It's totally standalone, and the idea of explaining to users of the service (Gecko developers) that it's busted because it can't connect to the Socorro Consul cluster strikes me as uncomfortable and weird :)

@phrawzty
Copy link
Contributor

@rhelmer

The problem is I don't want consul join to run on these nodes at all, there's no point.

Imho JP makes a good pont re: future-proofing this service - moreover, I get nervous any time we willingly introduce a ❄️ into the infra. Furthermore, it should be noted that socorro::role::common contains more than just a consul join - that's where the hostname and logging are set up too, and we definitely want those for Symbolapi.

That said, if we really want to do this, here's a potential solution that doesn't involve re-jiggering a bunch of Puppet:

#  puppet/modules/socorro/manifests/role/symbolapi.pp
class socorro::role::symbolapi {

include socorro::role::common

  # Symbolapi nodes are standalone and should not join the larger cluster.
  # The creates param will always match, therefore the exec will never trigger.
  Exec['join_consul_cluster'] {
    creates => '/'
  }

  service {
    'mozilla-snappy':
      ensure  => running,
      enable  => true
  }

}

@@ -1,8 +1,6 @@
# Set up a symbolapi node.
class socorro::role::symbolapi {

include socorro::role::common

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an aside, the service below will fail because you left the require param in place.

@rhelmer rhelmer force-pushed the symbolapi-do-not-join-consul branch from cd5d51a to 4b84ca2 Compare May 26, 2015 19:16
@rhelmer
Copy link
Contributor Author

rhelmer commented May 26, 2015

@phrawzty @jdotpz OK I think you're both right - in that case I think we should just remove the require => Exec['consul join'] so failure to join the consul cluster doesn't prevent the service from starting up.

@phrawzty
Copy link
Contributor

Squishy squish then r+.

…then puppet apply fails for no useful reason
@rhelmer rhelmer force-pushed the symbolapi-do-not-join-consul branch from 39ba2a3 to b607f5a Compare May 26, 2015 20:39
rhelmer added a commit that referenced this pull request May 26, 2015
currently symbolapi does not use consul, if the cluster is unhealthy …
@rhelmer rhelmer merged commit 5b4770a into mozilla:master May 26, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants