Skip to content
This repository was archived by the owner on Sep 21, 2022. It is now read-only.

Conversation

@ericpromislow
Copy link

Currently the mysql host is assumed to be the same as the default
network host. This change allows deploy-time selection of the
main mysql host for a cluster.

Call discover_external_ip only if there's no
cf_mysql.mysql.advertise_host config value.

@cfdreddbot
Copy link

Hey ericpromislow!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@cf-gitbot
Copy link
Collaborator

We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/118524849.

@aarondl
Copy link

aarondl commented Aug 12, 2016

@menicosia: To respond to your comment in pivotal tracker, we've landed several pull requests that are just like this one. Essentially the problem here is assumptions. This code makes assumptions on the environment that don't hold in every situation. Most other components use hostnames to communicate because DNS allows for some extra tricks that are more flexible than IP addresses. It's usually a form of service discovery and/or load balancing. We should strive to be consistent here and make this like everything else and also unlock the value you get from that extra layer of abstraction. I will get this PR rebased so it applies cleanly.

Currently the mysql host is assumed to be the same as the default
network host. This change allows deploy-time selection of the
main mysql host for a cluster.

Call discover_external_ip only if there's no
cf_mysql.mysql.advertise_host config value.
cppforlife pushed a commit to cppforlife/cf-mysql-release that referenced this pull request Dec 21, 2016
…thub.com/onsi/gomega

Bump cloudfoundry-incubator/galera-healthcheck:
  Lyle Franklin:
     Added mysqld binary location to config object instead of hardcoding it in sequence_number.go ( Travis Unknown )
  Ben Calegari:
     Alias /galera_status to / to maintain backwards compatability ( Travis Unknown )
     Update /sequence_number to function even if DB is down ( Travis Unknown )
     Change sequence_number Check() to private ( Travis Unknown )
     Test galera binary ( Travis Unknown )
     Move http response responsibility to endpoint objects ( Travis Unknown )
     Push flag parsing logic into config package ( Travis Unknown )
     Remove integration test suite ( Travis Unknown )
Bump :
  Onsi Fakhouri:
     Merge pull request cloudfoundry#112 from robdimsdale/master
     Merge pull request cloudfoundry#113 from cf-guardian/master
     update changelog: mention WithTransform and boolean matchers
     pull oracle matcher into its own package
     Merge pull request cloudfoundry#108 from jim-slattery-rs/composition
     Merge pull request cloudfoundry#111 from sykesm/verify-proto
     Merge pull request cloudfoundry#105 from sykesm/verify-form
     add 1.5 to travis
     Merge pull request cloudfoundry#103 from rosenhouse/occurred
     ghttp server can take an io.Writer
     ghttp handles failures and panics in handlers
     Merge pull request cloudfoundry#98 from matt-royal/respond-with-json-sets-content-type
     tell travis to use the latest golang
     explain why gomega refuses to compare <nil> to <nil>
     Merge pull request cloudfoundry#96 from craigfurman/HaveOccurredMatcher-nil-check-consistency
     Merge pull request cloudfoundry#95 from jfmyers9/fix-respond-with-ptr
     Merge pull request cloudfoundry#93 from james-lawrence/directory-matcher
     Merge pull request cloudfoundry#91 from james-lawrence/file-based-matchers
     Merge pull request cloudfoundry#89 from alext/improve_succeed_output
     Merge pull request cloudfoundry#83 from hobeone/patch-1
     ContainElement no longer bails if a passed-in matcher errors
     session.Wait now emits the correct line number when failures occur
  Zhou Yu:
     Reformat
  Rob Dimsdale:
     Add VerifyBody method.
  Matthew Sykes:
     Add VerifyProtoRepresenting and RespondWithProto
     Add matchers to verify form data
  jim-slattery-rs:
     make Or() support MatchMayChangeInTheFuture() - simplify tests by using empty And() or Or() instead of Receive() with a closed channel
     make And() propagate MatchMayChangeInTheFuture() - considers both Match() cases and considers the appropriate matcher(s) in making decision - also added a comment to WithTransform.MatchMayChangeInTheFuture(), as it will not have correct behavior with non-deterministic transformer
     make WithTransform() propagate MatchMayChangeInTheFuture() from wrapped matcher
     make Not() propagate MatchMayChangeInTheFuture() from wrapped matcher
     add SatisfyAll() and SatisfyAny() as aliases for And() and Or(), respectively
     WithTransform() supports transform function with explicit types
     Added Add, Or, Not, WithTransform matchers, for composability. - Allows matchers to be composed into complex expressions that work even with the Eventually() assertion. - Also makes it easy to create new matchers -- can often write a function that composes a new matcher out of existing ones.
  Brian Cunnie:
     Standardize spelling: occured --> occurred
  Luan Santos and Matt Royal:
     Allow user to explicitly set a Content-Type header on ghttp JSON handlers
     Set Content-Type to application/json in RespondWithJSON* methods
  Craig Furman:
     Negative assertions on nil pointers to types that conform to error interface should not panic
  James Myers:
     Fix RespondWithJSONEncodedPtr
  James Lawrence:
     directory matcher string fixes
     file based matchers
  Alex Tomlins:
     Improve Succeed() failure message.
  Alex Suraci:
     query params match irregardless of order
  Daniel Hobe:
     Return prettyprinted JSON strings for errors.

Signed-off-by: Aditya Anchuri <aanchuri@pivotal.io>

[#106993650]
@robdimsdale robdimsdale merged commit 867662b into cloudfoundry:develop Jan 27, 2017
@robdimsdale
Copy link
Member

We've merged this, but for future reference, p('cf_mysql.mysql.advertise_host') || discover_external_ip doesn't work as you might expect.

In order to make use of optional properties, we need to use if_p() - for example:

node_host = discover_external_ip
if_p('cf_mysql.mysql.advertise_host') do |advertise_host|
  node_host = advertise_host
end

@menicosia
Copy link
Contributor

@aarondl, @ericpromislow,

I've accepted this story, so this change will be in cf-mysql-release v33.

I am still not sure when an Operator would want to set this property. My concern is outlined in this comment, but in summary: From my reading of the docs, it seems like wsrep_node_address should be set individually per node. I'm not able to think of a use case where you'd want to set each node to use the same value?

--
Marco Nicosia
Product Manager

@aarondl
Copy link

aarondl commented Jan 31, 2017

It's possible in certain scenarios that not every node shares the same configuration values. In a multi-az installation they can have distinct values.

Since each node is it's own job, we can assign it a different value here (though this manifest doesn't actually have multiple AZs it does illustrate the functionality):
https://github.com/cloudfoundry/cf-release/blob/master/example_manifests/minimal-aws.yml#L134

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.

6 participants