-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
desginate: Add mdns as hidden master #2105
base: master
Are you sure you want to change the base?
Conversation
This reverts commit 64e7ad9. We've are very close to the release and have multiple pull requests still open for Designate: * crowbar#2095 * crowbar#2104 * crowbar#2105 * SUSE-Cloud/doc-cloud#931 There are way too many question marks on this so I am switching the barclamp back to invisible mode. Lets get these pull requests squared away carefully and without the rush of imminent release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is not really required but only eases out the config from
an admin perspective.
If this isn't critical maybe it could wait till the MU?
@@ -36,12 +34,12 @@ | |||
# non-hardcoded is high enough | |||
pools = [{ | |||
"name" => "default-bind", | |||
"description" => "Default BIND9 Pool", | |||
"description" => "Sample Pool for designate (relies only 1 dns-master)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand this correctly, the user doesn't have control over this from the barclamp side so they are stuck with this "sample" in their configuration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is just a sample file, the user has to edit this file and inject this pool in designate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can the user edit this file if it's being controlled by crowbar? Chef will just overwrite it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can create your own pool rather than the default pool. I think the tricky part here is that you would need to change the pool to use in the designate config. that isn't done yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can the user edit this file if it's being controlled by crowbar? Chef will just overwrite it.
Chef will overwrite file /etc/designate/pools.crowbar.yaml
designate relies on /etc/designate/pools.yaml
so the user can overwrite the real file or just pass that config to the designate with command documented here SUSE-Cloud/doc-cloud#909
This code change has no impact on any part of crowbar apart from changing the sample file. But helps greatly in trying to understand what this file really means. |
╰─ irb
irb(main):001:0> dnsmaster='123.123.123.123'
=> "123.123.123.123"
irb(main):002:0> dns = {:fqdn => 'me.exmaple.com' }
=> {:fqdn=>"me.exmaple.com"}
irb(main):003:0> network_settings = {:mdns_bind_host => dnsmaster }
=> {:mdns_bind_host=>"123.123.123.123"}
irb(main):004:0> pools = [{
irb(main):005:2* "name" => "default-bind",
irb(main):006:2* "description" => "Sample Pool for designate (relies only 1 dns-master)",
irb(main):007:2* "id" => "794ccc2c-d751-44fe-b57f-8894c9f5c842",
irb(main):008:2* "attributes" => {},
irb(main):009:2* "ns_records" => [{ "hostname" => "#{dns[:fqdn]}.", "priority" => 1 }],
irb(main):010:2* "nameservers" => [{ "host" => dnsmaster, "port" => 53 }],
irb(main):011:2* "also_notifies" => [],
irb(main):012:2* "targets" => [{
irb(main):013:4* "type" => "bind9",
irb(main):014:4* "description" => "BIND9 Server 1",
irb(main):015:4* "masters" => [{ "host" => network_settings[:mdns_bind_host], "port" => 5354 }],
irb(main):016:4* "options" => {
irb(main):017:5* "host" => dnsmaster,
irb(main):018:5* "port" => 53,
irb(main):019:5* "rndc_host" => dnsmaster,
irb(main):020:5* "rndc_port" => 953,
irb(main):021:5* "rndc_key_file" => "/etc/designate/rndc.key"
irb(main):022:5> }
irb(main):023:4> }]
irb(main):024:2> }]
=> [{"name"=>"default-bind", "description"=>"Sample Pool for designate (relies only 1 dns-master)", "id"=>"794ccc2c-d751-44fe-b57f-8894c9f5c842", "attributes"=>{}, "ns_records"=>[{"hostname"=>"me.exmaple.com.", "priority"=>1}], "nameservers"=>[{"host"=>"123.123.123.123", "port"=>53}], "also_notifies"=>[], "targets"=>[{"type"=>"bind9", "description"=>"BIND9 Server 1", "masters"=>[{"host"=>"123.123.123.123", "port"=>5354}], "options"=>{"host"=>"123.123.123.123", "port"=>53, "rndc_host"=>"123.123.123.123", "rndc_port"=>953, "rndc_key_file"=>"/etc/designate/rndc.key"}}]}]
(failed reverse-i-search)`': import^C
irb(main):025:0> require 'yaml'
=> true
irb(main):026:0> pools.to_yaml
=> "---\n- name: default-bind\n description: Sample Pool for designate (relies only 1 dns-master)\n id: 794ccc2c-d751-44fe-b57f-8894c9f5c842\n attributes: {}\n ns_records:\n - hostname: me.exmaple.com.\n priority: 1\n nameservers:\n - host: 123.123.123.123\n port: 53\n also_notifies: []\n targets:\n - type: bind9\n description: BIND9 Server 1\n masters:\n - host: 123.123.123.123\n port: 5354\n options:\n host: 123.123.123.123\n port: 53\n rndc_host: 123.123.123.123\n rndc_port: 953\n rndc_key_file: \"/etc/designate/rndc.key\"\n" |
315171a
to
2e89f84
Compare
this change is now needed for HA, I have added comments to the code and update the commit message to reflect why this change is required. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code looks good
2e89f84
to
3fe0370
Compare
Rebased with HA fix |
This reverts commit 64e7ad9. We've are very close to the release and have multiple pull requests still open for Designate: * crowbar#2095 * crowbar#2104 * crowbar#2105 * SUSE-Cloud/doc-cloud#931 There are way too many question marks on this so I am switching the barclamp back to invisible mode. Lets get these pull requests squared away carefully and without the rush of imminent release.
3fe0370
to
c61534d
Compare
|
||
network_settings = DesignateHelper.network_settings(node) | ||
# hidden masters are designate-mdns services, in ha this service will be running on multiple | ||
# hosts and any host can be asked to update the zone (when a recordset, corsspoding to a vm is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be nice to correct the typo here and in the commit message ("corsspoding")
network_settings = DesignateHelper.network_settings(node) | ||
# hidden masters are designate-mdns services, in ha this service will be running on multiple | ||
# hosts and any host can be asked to update the zone (when a recordset, corsspoding to a vm is | ||
# created) on th real-master so all have to be listed as master in the pool. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here ("th") and in the commit message
400008a
to
03a5d0b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please at least make the list of nameservers expand all hosts again. Ideally the hostname ns_record would be configurable (as it needs to point to the public hostname), but thats not a new bug introduced here
# dns servers looks like. | ||
# This pool id can be generated by in proposal, but this will change | ||
# with every delete/create cycle of proposal. This might mess | ||
# up the designate configuration. So the advantage of having | ||
# non-hardcoded is high enough | ||
pools = [{ | ||
"name" => "default-bind", | ||
"description" => "Default BIND9 Pool", | ||
"description" => "Sample Pool for designate", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change? IMHO it should be the default pool, this matches the name line above.
# dns servers looks like. | ||
# This pool id can be generated by in proposal, but this will change | ||
# with every delete/create cycle of proposal. This might mess | ||
# up the designate configuration. So the advantage of having | ||
# non-hardcoded is high enough | ||
pools = [{ | ||
"name" => "default-bind", | ||
"description" => "Default BIND9 Pool", | ||
"description" => "Sample Pool for designate", | ||
"id" => "794ccc2c-d751-44fe-b57f-8894c9f5c842", | ||
"attributes" => {}, | ||
"ns_records" => [{ "hostname" => "#{dns[:fqdn]}.", "priority" => 1 }], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would need to be configurable in the barclamp.
"id" => "794ccc2c-d751-44fe-b57f-8894c9f5c842", | ||
"attributes" => {}, | ||
"ns_records" => [{ "hostname" => "#{dns[:fqdn]}.", "priority" => 1 }], | ||
"nameservers" => dnsservers.map { |ip| { "host" => ip, "port" => 53 } }, | ||
"also_notifies" => dnsslaves.map { |ip| { "host" => ip, "port" => 53 } }, | ||
"nameservers" => [{ "host" => dnsmaster, "port" => 53 }], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this part doesn't make a whole lot of sense to me. this way we have only one DNS server hosting the domain, with other words no High Availability (HA is a requirement, see https://jira.suse.com/browse/SOC-6361 ). We need to list all dns-server masters here, and ideally their public IP address (see SOC-9635 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nameservers
is a list where designate will check if the zone and/or ns-record is created successfully, I have limited it to 1 server and that too dns-master. because ....
The DNS barclamp has only 1 dns-server as master. All others are slaves who just forward incoming request to that master (this how bind is configured). This dns-master stores all the zone and ns-records received from designate.
These slaves could also be part of nameservers where designate validates the creation of zones and ns-records, but that wasnt very useful since they are just going to forwards query to dns-master.
"nameservers" => dnsservers.map { |ip| { "host" => ip, "port" => 53 } }, | ||
"also_notifies" => dnsslaves.map { |ip| { "host" => ip, "port" => 53 } }, | ||
"nameservers" => [{ "host" => dnsmaster, "port" => 53 }], | ||
"also_notifies" => [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leaving this empty by default makes sense to me, however if this is integrated in another hidden master setup (like e.g. SUSE's internal DNS), you need to be able to add additional hosts here. ideally this would be a setting in the barclamp ui.
|
||
# the host with VIP will use that ip as the outgoing ip when connecting to the real-master | ||
if node[:designate][:ha][:enabled] | ||
hiddenmasters += [{ "host" => CrowbarPacemakerHelper.cluster_vip(node, "admin"), "port" => 5354 }] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is already in the list above, right? just once via vip and once via the node ip. I don't understand why we need to add it twice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
afaik Barclamp::Inventory.get_network_by_type
does not return the VIP so the list has admin address,
Also iiuc the that is different from VIP, and the host with VIP will use that ip as the outgoing ip when connecting to the real-master.
Addressing the open review comments in the next commit. Note, "nameservers" will be updated via SCRD-9636 to use public FQDNs. |
03a5d0b
to
82ff9bb
Compare
82ff9bb
to
081e89f
Compare
b90aeaf
to
4757bda
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WIP
In crowbar's world the dns-master is the master of all and slaves forward all queries to dns-master. When designate is enabled, designate-mdns service component(s) become the hidden master(s). We leave 'also_notifies' empty. It can be configured by the users via designate-manage utility as and when needed. designate-mdns service components, in HA, will be running on multiple hosts and any host can be asked to update a zone on th real-master. We add the cluster vip as the hidden master in case HA is enabled otherwise add all servers.
4757bda
to
a8f2c73
Compare
in crowbar's world the dns-master is the master of all and slaves
forward all queries to dns-master. Under such a configuration there is
no need to query other nameservers as they still forward the query to
dns-master. So designate can just verify on one nameserver(dns-master)
and dns-master will take care of passing that info to all slaves.
Same goes for also_notifies: dns-master will notify all slaves in case
of either zone or recordset is updated/deleted.
this also further simplifies the designate pool config reducing the
time required to create zone and recordsets.
This change is not really required but only eases out the config from
an admin perspective.
Also having multiple nameservers confuses designate in some times as
according to these nameserver designate is not authoritative of these
zones and recordsets.