Ruby module for Docker DNS updates

Question

I wrote a Ruby module to update DNS zones on certain events of the Docker API. Its purpose is to make container reachable by their name over a network. I know this is also possible through /etc/hosts entries, but in my use case I can not use the name resolution of the docker host.

I'm am pretty new to Ruby so I guess there are a lot of things that can be done better. I am also not a professional when it comes to code structuring, design patterns and things like that. I'm happy to get comments on this as well.

require "dockerdns/version"
require 'docker'
require 'dnsruby'

module Dockerdns

  class DockerDNS

    #==========================================================================
    def self.run!(config)
      DockerDNS.new(config).run!
    end
    #==========================================================================

    attr_reader :config

    def initialize(config)
      @config = config
      @domain = domain
      @reversezone = reversezone
      @dnsserver = dnsserver
      @dockerurl = dockerurl
      @ttl = ttl
    end

    def domain
      config["domain"]
    end

    def reversezone
      config["reversezone"]
    end

    def dnsserver
      config["dnsserver"]
    end

    def ttl
      config["ttl"]
    end

    def dockerurl
      if config["dockerurl"]
        return config["dockerurl"]
      end
      return "/var/run/docker.sock"
    end

    def run!
      Docker.url = dockerurl
      Docker.options[:read_timeout] = 5
      begin
      Docker::Event.stream do |event|
        if event.status == "create" then
          next
        elsif event.status == "start" then
          puts "caught event #{event.status} for container id #{event.id}"
          dnsAddOrUpdate(event.id, domain, reversezone, dnsserver)
        elsif event.status == "die" || event.status == "kill" || event.status == "stop" || event.status == "destroy" then
          puts "caught event #{event.status} for container id #{event.id}"
          dnsDelete(event.id)
        else
        puts "Ignoring Docker Event #{event.status}"
        end
      end
      rescue Docker::Error::TimeoutError
        retry
      rescue Excon::Errors::SocketError
        retry
      rescue Exception => e
        puts "Error while streaming events: #{e}"
      end
    end

    def getContainerIP(id)
      ipAddress = Docker::Container.get(id).json["NetworkSettings"]["IPAddress"]
      return ipAddress
    end

    def getContainerName(id)
      hostname = Docker::Container.get(id).json["Config"]["Hostname"]
      return hostname
    end

    def getARecord(fqdn)
      resolver = Dnsruby::Resolver.new(dnsserver).query(fqdn)
      ipAddress = resolver.answer[0].address.to_s
      return ipAddress
    end

    def getPtrRecord(ipAddress)
      resolver = Dnsruby::Resolver.new(dnsserver).query(ipAddress, "PTR")
      fqdn = resolver.answer[0].domainname.to_s
      return fqdn
    end

    def setARecord(ipAddress, hostname, domain)
      record = "#{hostname}.#{domain}"
      puts "setting a-record #{record}"
      resolver = Dnsruby::Resolver.new(dnsserver)
      update = Dnsruby::Update.new(domain)
      # make sure there is no record yet
      #update.absent(record, 'A')
      # add record
      puts "update.add(#{record}, 'A', #{@ttl}, #{ipAddress})"
      update.add(record, 'A', @ttl, ipAddress)
      # send update
      begin
        reply = resolver.send_message(update)
        puts "Update succeeded"
      rescue Exception => e
        puts "Update failed: #{e}"
      end
    end

    def deleteARecord(ipAddress, hostname, domain)
      record = "#{hostname}.#{domain}"
      puts "deleting a-record #{record}"
      resolver = Dnsruby::Resolver.new(dnsserver)
      update = Dnsruby::Update.new(domain)
      # delete record
      puts "update.delete(#{record})"
      update.delete(record)
      # send update
      begin
        reply = resolver.send_message(update)
        puts "Update succeeded"
      rescue Exception => e
        puts "Update failed: #{e}"
      end
    end

    def setPtrRecord(ipAddress, hostname, domain, reversezone)
      record = "#{ipAddress.split('.').last}.#{reversezone}"
      fqdn = "#{hostname}.#{domain}"
      puts "setting ptr-record #{record}"
      resolver = Dnsruby::Resolver.new(dnsserver)
      update = Dnsruby::Update.new(reversezone)
      # make sure there is no record yet
      #update.absent(record)
      # add record
      puts "update.add(#{record}, 'PTR', #{@ttl}, #{fqdn})"
      update.add(record, 'PTR', @ttl, fqdn)
      # send update
      begin
        reply = resolver.send_message(update)
        puts "Update succeeded"
      rescue Exception => e
        puts "Update failed: #{e}"
      end
    end

    def deletePtrRecord(ipAddress, hostname, domain, reversezone)
      record = "#{ipAddress.split('.').last}.#{reversezone}"
      fqdn = "#{hostname}.#{domain}"
      puts "deleting ptr-record #{record}"
      resolver = Dnsruby::Resolver.new(dnsserver)
      update = Dnsruby::Update.new(reversezone)
      # delete record
      puts "update.delete(#{record})"
      update.delete(record)
      # send update
      begin
        reply = resolver.send_message(update)
        puts "Update succeeded"
      rescue Exception => e
        puts "Update failed: #{e}"
      end
    end

    def dnsAddOrUpdate(id, domain, reversezone, dnsserver)
      hostname = getContainerName(id)
      ipAddress = getContainerIP(id)
      setARecord(ipAddress, hostname, domain)
      setPtrRecord(ipAddress, hostname, domain, reversezone)
      getARecord("#{hostname}.#{domain}")
      getPtrRecord(ipAddress)
    end


    def dnsDelete(id)
      hostname = getContainerName(id)
      ipAddress = getARecord("#{hostname}.#{domain}")
      deleteARecord(ipAddress, hostname, domain)
      deletePtrRecord(ipAddress, hostname, domain, reversezone)
    end
  end

end

200_success · Accepted Answer · 2015-12-02 21:17:26Z

The code is pretty logically laid out. Let's start from the top.

attr_reader :config

def initialize(config)
  @config = config
  @domain = domain
  @reversezone = reversezone
  @dnsserver = dnsserver
  @dockerurl = dockerurl
  @ttl = ttl
end

def domain
  config["domain"]
end

def reversezone
  config["reversezone"]
end

def dnsserver
  config["dnsserver"]
end

def ttl
  config["ttl"]
end

def dockerurl
  if config["dockerurl"]
    return config["dockerurl"]
  end
  return "/var/run/docker.sock"
end

Why is there an attr_reader :config? Is it important to expose the configuration as part of the DockerDNS object's interface? (For that matter, do #domain, #reversezone, etc. need to be exposed as well?)

Why does the constructor have @domain = domain, @reversezone = reversezone, etc.? Only @ttl is ever used. Furthermore, the call sequence feels backwards. The job of the constructor is to initialize the object, as its name implies. What you have done, rather, is partially construct the object (@config = config), then call a helper function (#ttl), which in turn calls the attr_reader (#config), to let the constructor cache the value in an instance variable (@ttl = …). The simpler way would be:

def initialize(config)
  @config = config
end

def domain
  @config['domain']
end

…

def ttl
  @config['ttl']
end

def dockerurl
  @config['dockerurl'] || '/var/run/docker.sock'
end

In #run!, the Docker::Event.stream block needs to be indented another level. The if event.status == … checks would be better written using a case expression.

begin
  Docker::Event.stream do |event|
    case event.status
    when 'create'
      next
    when 'start'
      …
    when 'die', 'kill', 'stop', 'destroy'
      …
    else
      puts "Ignoring Docker Event #{event.status}"
    end
  end
rescue …
  …
end

When retrying after an error, it would be a good idea to add some delay. Otherwise, you could be retrying an operation furiously and making a bad situation worse.

You have a

def dnsserver
  config["dnsserver"]
end

… but that method is not as useful as it could be. What you really want is to simplify the resolver = Dnsruby::Resolver.new(dnsserver) calls that you have all over the place. So, instead of #dnsserver, it would be better to have

def resolver
  @resolver ||= Dnsruby::Resolver.new(@config['dnsserver'])
end

That lazily constructs a Resolver the first time it is needed, and keeps the same object around for subsequent uses.

I don't recommend handling exceptions at this level:

def setARecord(…)
  …
  update.add(record, 'A', @ttl, ipAddress)
  # send update
  begin
    reply = resolver.send_message(update)
    puts "Update succeeded"
  rescue Exception => e
    puts "Update failed: #{e}"
  end
end

The error will show up on screen, but the rest of your program (#dnsAddOrUpdate) has no idea that anything went wrong, and proceeds happily with other operations. I suggest just letting the exception propagate — perhaps all the way to the run loop.

Flambino · Accepted Answer · 2015-12-06 12:20:44Z

Here are some notes from a quick once-over. I don't know the Docker API, nor have I played much with automated DNS updating, so I've just looked at the Ruby-ness of things.

The Ruby convention is snake_case for methods and variables. Not camelCase
Dockerdns::DockerDNS.... why wrap a class in a module of same name (but not quite the same name)? I mean, putting things in a module is a great way to namespace things, but the naming seems redundant and confusing.
There's no reason to have accessor methods for hash values and set instance variables (that you never use). So your initialize method can be reduced to one line.
Following the above: Why pass things to instance methods that the methods themselves can already access? E.g.
```
dnsAddOrUpdate(event.id, domain, reversezone, dnsserver)
```
could just be
```
dnsAddOrUpdate(event.id)
```
Don't do this sort of thing
```
def some_value
  x = expression.that_gets.the_value
  return x
end
```
For one, the return is implicit. For another, there's no reason to assign anything just to return it. So for instance getContainerIp (sic; should be snake_case) can just be this:
```
def getContainerIP(id)
  Docker::Container.get(id).json["NetworkSettings"]["IPAddress"]
end
```
No variable assignment, no explicit return. Similar thing for dockerurl (which should really be docker_url) which can be reduced to
```
def dockerurl
  config["dockerurl"] || "/var/run/docker.sock"
end
```
Don't use then unless you're prefixing a conditional on one line. Mostly, though, you'd postfix the conditional, e.g.
```
next if event.status == "create"
```
Of course, this particular line doesn't actually matter much, as the following lines have their own conditionals and won't trigger on "create" anyway.
It'd probably be good limit the retrys. And don't rescue Exception; rescue StandardError instead.

Or let exceptions bubble up - they could be important, and should no doubt be handled, not just printed and forgotten.
DRY - Don't Repeat Yourself. E.g. you call Docker::Container.get(id).json in multiple places; wrap that in a method. If the get(id) call results in an API request each time, you should probably cache it too. No reason to fetch the same JSON multiple times just to pull different values out.

Same goes for Dnsruby::Resolver.new(dnsserver) and other operations that are very much repeated in multiple places.
Don't use some_array[0] - use some_array.first.
I'd place more logic in separate classes, so that each event, and the actions it might cause are encapsulated.

Here's an attempt at refactoring that takes most of the above into account. Mostly it just offloads responsibilities to a Container class, since that's the unit you're manipulating. Don't just run it - I haven't tested it, and I put it together just to show a different structure. Can't guarantee it'll even work right.

module DockerDns

class Updater
  def self.run!(config)
    Updater.new(config).run!
  end

  def initialize(config)
    @config = config

    # Note: Docker config should be set elsewhere as it's global.
    # It it not the responsibility of this class. I've left it in
    # to more easily compare the code, but it should go.
    Docker.url = @config["dockerurl"] || "/var/run/docker.sock"
    Docker.options[:read_timeout] = 5
  end

  def run!
    Docker::Event.stream do |event|
      container = Container.new(id, @config)
      case event.status
      when "start"
        config.create_or_update_dns_records!
      when "die", "kill", "stop", "destroy"
        config.delete_dns_records!
      else
        puts "Ignoring Docker Event #{event.status}"
      end
    end
  rescue Docker::Error::TimeoutError, Excon::Errors::SocketError
    retry
  rescue StandardError => e
    puts "Error while streaming events: #{e}"
  end
end

class Container
  def initialize(id, config)
    @id = id
    @config = config
  end

  def create_or_update_dns_records!
    set_a_record!
    set_ptr_record!
  end

  def delete_dns_records!
    delete_a_record!
    delete_ptr_record!
  end

  def ptr_record
    @ptr_record ||= resolver.query(ip_address, "PTR").answer.first.domainname.to_s
  end

  def a_record
    @a_record ||= resolver.query(fqdn).answer.first.address.to_s
  end

  def set_ptr_record!
    update = Dnsruby::Update.new(reversezone)
    update.add(record, 'PTR', @config["ttl"], fqdn)
    resolver.send_message(update)
  end

  def set_a_record!
    update = Dnsruby::Update.new(domain)
    update.add(fqdn, 'A', @config["ttl"], ip_address)
    resolver.send_message(update)
  end

  def delete_ptr_record!
    update = Dnsruby::Update.new(reversezone)
    update.delete(record)
    resolver.send_message(update)
  end

  def delete_a_record!
    update = Dnsruby::Update.new(domain)
    update.delete(record)
    resolver.send_message(update)
  end

  def fqdn
    "#{hostname}.#{domain}"
  end

  def domain
    @config["domain"]
  end

  def hostname
    json["Config"]["Hostname"]
  end

  def ip_address
    json["NetworkSettings"]["IPAddress"]
  end

  def record
    "#{ipAddress.split('.').last}.#{@config["reversezone"]}"
  end

  private

  def resolver
    @resolver ||= Dnsruby::Resolver.new(@config["dnsserver"])
  end

  def json
    @json ||= Docker::Container.get(@id).json
  end
end

end

john.dough · Accepted Answer · 2015-12-08 21:15:13Z

Thanks for your review guys. Here's what I changed

require 'docker'
require 'dnsruby'

class DockerDNS

  #==========================================================================
  def self.run!(config)
    DockerDNS.new(config).run!
  end
  #==========================================================================

  def initialize(config)
    @config = config
  end

  def domain
      @config["domain"]
  end

  def reversezone
      @config["reversezone"]
  end

  def resolver
    @resolver ||= Dnsruby::Resolver.new(@config['dnsserver'])
  end

  def ttl
    @config["ttl"]
  end

  def docker_url
    @config["dockerurl"] || '/var/run/docker.sock'
  end

reduced the initialize() and docker_url() method to one line
changed dnsserver to resolver to get a resolver instance

def run!
    Docker.url = docker_url
    Docker.options[:read_timeout] = 5
    begin
        Docker::Event.stream do |event|
          case event.status
        when "create"
              next
          when "start"
              puts "caught event #{event.status} for container id #{event.id}"
              create_or_update_dns_records!(event.id, domain)
          when "die", "kill", "stop", "destroy"
              puts "caught event #{event.status} for container id #{event.id}"
              delete_dns_records!(event.id)
          else
              puts "Ignoring Docker Event #{event.status}"
          end
        end
    rescue Docker::Error::TimeoutError, Excon::Errors::SocketError
        retry
    rescue StandardError => e
        puts "Error while streaming events: #{e}"
    end
  end

refactored to case statement
catching StandardError now

def container_ip(id)
    Docker::Container.get(id).json["NetworkSettings"]["IPAddress"]
  end

  def container_name(id)
    Docker::Container.get(id).json["Config"]["Hostname"]
  end

  def a_record(fqdn)
    resolver.answer.first.address.to_s
  end

  def ptr_record(ipAddress)
    resolver.query(ipAddress, "PTR").answer.first.domainname.to_s
  end

changed these methods to one liners too
refactored method names to snake_case

def set_a_record(ipAddress, hostname)
    record = "#{hostname}.#{domain}"
    puts "setting a-record #{record}"
    update = Dnsruby::Update.new(domain)
    # add record
      puts "update.add(#{record}, 'A', #{ttl}, #{ipAddress})"
    update.add(record, 'A', ttl, ipAddress)
    # send update
    begin
      reply = resolver.send_message(update)
      puts "Update succeeded"
    rescue Exception => e
      puts "Update failed: #{e}"
    end
  end

  def delete_a_record(ipAddress, hostname)
    record = "#{hostname}.#{domain}"
    puts "deleting a-record #{record}"
    update = Dnsruby::Update.new(domain)
    # delete record
    puts "update.delete(#{record})"
    update.delete(record)
    # send update
    begin
      reply = resolver.send_message(update)
      puts "Update succeeded"
    rescue Exception => e
      puts "Update failed: #{e}"
    end
  end

  def set_ptr_record(ipAddress, hostname)
    record = "#{ipAddress.split('.').last}.#{reversezone}"
      fqdn = "#{hostname}.#{domain}"
    puts "setting ptr-record #{record}"
    update = Dnsruby::Update.new(reversezone)
    # add record
      puts "update.add(#{record}, 'PTR', #{ttl}, #{fqdn})"
    update.add(record, 'PTR', ttl, fqdn)
    # send update
    begin
      reply = resolver.send_message(update)
      puts "Update succeeded"
    rescue Exception => e
      puts "Update failed: #{e}"
    end
  end

  def delete_ptr_record(ipAddress, hostname)
    record = "#{ipAddress.split('.').last}.#{reversezone}"
    fqdn = "#{hostname}.#{domain}"
    puts "deleting ptr-record #{record}"
    update = Dnsruby::Update.new(reversezone)
    # delete record
      puts "update.delete(#{record})"
    update.delete(record)
    # send update
    begin
      reply = resolver.send_message(update)
      puts "Update succeeded"
    rescue Exception => e
      puts "Update failed: #{e}"
    end
  end

  def create_or_update_dns_records!(id)
    hostname = container_name(id)
    ipAddress = container_ip(id)
    set_a_record(ipAddress, hostname)
    set_ptr_record(ipAddress, hostname)
    a_record("#{hostname}.#{domain}")
    ptr_record(ipAddress)
  end


  def delete_dns_records!(id)
    hostname = container_name(id)
    ipAddress = a_record("#{hostname}.#{domain}")
    delete_a_record(ipAddress, hostname)
    delete_ptr_record(ipAddress, hostname)
  end
end

The rest of the method profit from these changes as they are a bit more compact. I also removed the unnecessary parameters.

If there were more use cases for this code, I'd consider splitting it up into separate classes as suggested by Flambino

Stack Exchange Network

current community

your communities

more stack exchange communities

Ruby module for Docker DNS updates

3 Answers 3

Your Answer

Not the answer you're looking for? Browse other questions tagged beginner ruby api networking ip-address or ask your own question.

Hot Network Questions

Ruby module for Docker DNS updates

3 Answers 3

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged beginner ruby api networking ip-address or ask your own question.

Related

Hot Network Questions