Skip to content
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

provider/lxd: improve endpoint handling #6994

Merged
merged 1 commit into from Feb 16, 2017
Merged

Conversation

axw
Copy link
Contributor

@axw axw commented Feb 16, 2017

Description of change

We improve the cloud endpoint handling so that
users can now specify any of the following:

  • (empty) => connect to the unix socket, identify the
    bridge device used for default profile's eth0 NIC,
    and identify the IP address assigned to the bridge
    device on the host. Query LXD for the port it is
    listening on for HTTPS requests.
  • host => connect to the specified host on the default
    port (8443) over HTTPS.
  • host:port => connect to the specified host and port
    over HTTPS.
  • https://host[:port]/path => connect to the specified
    URL over HTTPS.

In the future we should also support unix://, but for
now there are too many assumptions, so we reject any
endpoint starting with unix://.

We will no longer use the unix socket just because the
endpoint is a local address. We only use the unix socket
to create certificate credentials for the local LXD,
and to configure LXD to listen on HTTPS and obtain the
appropriate endpoint for communication from within.

QA steps

  1. juju bootstrap localhost

  2. (with lxd entry in clouds.yaml, endpoint set to 10.0.8.1, where 10.0.8.1 is the LXD host address on lxdbr0; LXD is not listening on HTTPS yet, there are no certs in "lxc config trust list")
    juju bootstrap lxd

  3. (with lxd entry in clouds.yaml, endpoint set to 10.0.8.1:8553, where 10.0.8.1 is the LXD host address on lxdbr0; LXD is configured to listen on 8553)
    juju bootstrap lxd

  4. (with lxd entry in clouds.yaml, endpoint set to https://10.0.8.1:8553, where 10.0.8.1 is the LXD host address on lxdbr0; LXD is configured to listen on 8553)
    juju bootstrap lxd

  5. (with lxd entry in clouds.yaml, endpoint set to https://10.0.8.1:8553, where 10.0.8.1 is the LXD host address on lxdbr0; LXD is configured to listen on 8443)
    juju bootstrap lxd
    ERROR Get https://10.0.8.1:8553/1.0: Unable to connect to: 10.0.8.1:8553

  6. with juju 2.0, juju bootstrap localhost; with this branch, juju destroy-controller -y localhost

Documentation changes

We should update the LXD provider docs to describe the various endpoint formats accepted.

Bug reference

None.

Copy link
Member

@jameinel jameinel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try swapping between things like "127.0.0.1" and "http://10.0.3.1:8443" etc and all of those worked as we'd like?
Do we think it would be worthwhile to tweak our functional tests so that it covers more of this case so we know that everything stays wired up and we don't regress here?

Name: "bar",
}},
})
c.Assert(err, gc.ErrorMatches,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be easier to read as:
c.Assert(err.Error(), gc.Equals, LXD is not listening on address 8.8.8.8 +
(reported addresses...)
Since you're comparing to the error string anyway, and then you don't have to escape all of the regex characters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good call. Done, with an added NotNil check before it.

@axw
Copy link
Contributor Author

axw commented Feb 16, 2017

Did you try swapping between things like "127.0.0.1" and "http://10.0.3.1:8443" etc and all of those worked as we'd like?

127.0.0.1 definitely won't work, because the endpoint is passed along verbatim. The controller agent obviously won't be able to dial LXD on loopback. I've only tested with the single eth0 nic device in bridged mode, as we support today.

Do we think it would be worthwhile to tweak our functional tests so that it covers more of this case so we know that everything stays wired up and we don't regress here?

If you mean the functional tests in the provider/lxd package, I don't think so. They're pretty weak, and I wouldn't want them mucking about with my LXD to be honest.

I think it would be worthwhile to update the CI tests with all of the scenarios we care about. For now, I think the unit tests protect the bits we care about well enough. As we go forward with more networking scenarios, it would be good to have end-to-end tests.

We improve the cloud endpoint handling so that
users can now specify any of the following:

 - (empty) => connect to the unix socket, identify the
   bridge device used for default profile's eth0 NIC,
   and identify the IP address assigned to the bridge
   device on the host. Query LXD for the port it is
   listening on for HTTPS requests.
 - host => connect to the specified host on the default
   port (8443) over HTTPS.
 - host:port => connect to the specified host and port
   over HTTPS.
 - https://host[:port]/path => connect to the specified
   URL over HTTPS.

In the future we should also support unix://, but for
now there are too many assumptions, so we reject any
endpoint starting with unix://.

We will no longer use the unix socket just because the
endpoint is a local address. We only use the unix socket
to create certificate credentials for the local LXD,
and to configure LXD to listen on HTTPS and obtain the
appropriate endpoint for communication from within.
@axw
Copy link
Contributor Author

axw commented Feb 16, 2017

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Feb 16, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 0647dc0 into juju:2.1 Feb 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants