fog / fog-openstack

Fog for OpenStack Platform
MIT License
68 stars 130 forks source link

Enhance handling of mock nic data in the mock create_server request. #296

Open zeroDivisible opened 7 years ago

zeroDivisible commented 7 years ago

This is just an idea, so I'm happy to discuss the details when it comes to working on implementing this.

Basically, currently in lib/fog/compute/openstack/requests/create_server.rb, we've got this piece of code:

          if nics
            nics.each do |_nic|
              mock_data["addresses"].merge!(
                "Public" => [{'addr' => Fog::Mock.random_ip}]
              )
            end
          end

So when creating server, our control over the mocked nic details is a bit limited. Would it make sense to enhance this to something like:

if nics
  nics.each do |nic|
    net_id = nic[:net_id]
    nets = Fog::Network::OpenStack::Mock.data["#{@openstack_username}-#{@openstack_tenant}"][:networks]
    fail 'Networks not configured. Make sure the mock data is setup for networks' unless nets
    fail "Network #{net_id} not configured" unless nets[net_id]
    network_name = nets[net_id]["name"]
    mock_data["addresses"].merge!(
      network_name => [{ 'addr' => Fog::Mock.random_ip }]
    )
  end
end

The biggest win here is allowing the user to provide details of the mocked network interfaces.

Carthaca commented 7 years ago

I think this should not fail, if the user does not provide the network interface details, but instead fall back to the old behaviour. So that we are not breaking any working scenarios.

zeroDivisible commented 7 years ago

Sorry, you're right with the above - the snippet which I provided above is taken from the implementation which I've been using, however - your solution works for everybody.

A backwards-compatible approach would be ideal.