Open sepiroth887 opened 9 years ago
Before commenting on the implementation suggestions, I'd like to ask how you intend to handle dynamic IP changes of your external services such as the Docker Registry or Vault. If these are dynamically scheduled and HA, this file based solution requires a redeploy of Mesos-DNS to pickup such changes.
a) those "Pets" will remain relatively static (fixed IP space, long lived) b) changes in the config can be driven through puppet (for us thats what we aim for) and dynamically reload mesos-dns that way. c) This is intended to be static for long periods of time. It's not expected to be HA / super dynamic, otherwise i'd suggest a ZK driven implementation.
d) Pets I think will always be part of an Environment (think your Mesos masters or things like Vault etc) ideally I'd love them to be managed by cloud providers but thats not a reality right now.
Alright, that makes sense to me. Now, regarding implementation, I think we can simplify upon your suggestions. Let's start with the config file. I think we should use the record structs available in http://godoc.org/github.com/miekg/dns for deserialisation. I also think need a single field records
for this data which we can unmarshal into a map[string][]dns.RR
.
"records": {
"A": {
"foo.bar": [{"a": "127.0.0.1"}, {"a": "127.0.0.2"}],
},
"SRV": {
"_foo._tcp.bar": [
{"target": "foo.bar", "port": 123, "weight": 0, "priority": 0},
{"target": "foo.bar", "port": 124, "weight": 0, "priority": 0},
{"target": "foo.bar", "port": 125, "weight": 0, "priority": 0}
]
}
}
Then in InsertState
we can add an extra argument static map[string][]dns.RR
which we'd then pass to rg.staticRecords
to do its job of inserting them.
Before all of this, at validation time, we should ensure that all records are valid, including inter-dependencies such as this one stated in RFC 2782:
Target
The domain name of the target host. There MUST be one or more
address records for this name, the name MUST NOT be an alias (in
the sense of RFC 1034 or RFC 2181). Implementors are urged, but
not required, to return the address record(s) in the Additional
Data section. Unless and until permitted by future standards
action, name compression is not to be used for this field.
A Target of "." means that the service is decidedly not
available at this domain.
Cool that is better yes. I also wanted to add some regex validation for the A and SRV RFCS because its otherwise unchecked user input.
Are there any out of RFC chars used right now? And do the entries have to terminate with a "."?
One more:
In the sample records JSON above the SRV section has weight and priority, is that supported? I was hoping to use rg.insertRR() which doesn't seem to cater for this unless its somehow merged into the host string.
If this is driven through insertState then I don't think it makes sense to use dns.RR as it would mean data goes from json > dns.RR > map[string][]string > dns.RR
Also it is non-trivial to parse json into RR tbh. It would be simpler to create a Record struct to deserialize this json and pass it to rg.insertRR()
If this is not desired i'd probably suggest to rewrite the record generator from scratch to store dns.RR to start with instead of doing the conversion during a request from clients.
@sepiroth887: I intend to re-write the RecordGenerator
to store dns.RRs
. My suggestion would be forwards compatible with that.
Regarding the priority and weight of SRV records, they're not currently supported, and hence, would be optional and forwards compatible.
For validation, I'd stay away from regular expressions and use http://godoc.org/github.com/miekg/dns#IsDomainName instead.
that makes sense. Agreed on validation. no need to rewrite complex checks if it's already done :+1:
I will for now have to keep my own version forked as it's a bit time critical for us to have the feature and waiting for dns.RR to be in RecordGenerator may be stretching our deadlines to far.
The implementation i have so far is tested and works for us for the time being and I'd be happy to provide the dns.RR driven logic once RecordGenerator is updated.
Current version here: https://github.com/sepiroth887/mesos-dns
Uh, one thing to add to the proposal:
Ideally mesos-dns should be able to start and serve requests even if it cannot reach a master or the zookeeper ensemble.
This will make it much easier to bootstrap environments as the static section can help drive some of the discovery for artifact/secret stores.
Ideally mesos-dns should be able to start and serve requests even if it cannot reach a master or the zookeeper ensemble.
This seems like a desirable property even without static records. Would you create a separate proposal issue for it please?
If it wasn't clear, this proposal SGTM, with the previously mentioned remarks in mind.
Yup. I do agree. My Go is not great around JSON parsing though. I'd rather wait until there is at least a branch with a promising conversion of the generator leveraging dns.rr before diving in and adding the static record bit.
I guess the easiest way to do the parsing would be to parse as a map[string][]string to avoid wrapping it in another struct. This feels really messy though and i'd prefer using a struct like StaticRecords { A []StaticRecord, SRV []StaticRecord} to wrap those and after parsing the Json push them into dns.rr model like it's done in resolver.go right now.
This can all happen in validation.go i'd think and then passed to generator.go as staticRecords []rrs
There's a missing component to this request IMO: in addition to "static" file based entries, it would be nice to be able to inject "static" entries into zookeeper under a specialized key to augment what mesos-dns normally serves.
Related to #132
Reasoning:
Solution:
Allow Mesos-DNS to serve static entries collected from a JSON file on startup and generate the entries during the master poll.
Architecture:
Additionally all this will have tests for:
Please let me know if this sounds reasonable? Any changes needed?