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
LXD: Set default server name #9121
Conversation
So the idea of the following is to set a default server name, so that when we come to fill in the zones later on, the code in question can identify it correctly.
Currently this still fails to deploy the
|
!!build!! |
1 similar comment
!!build!! |
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.
Something is not coming together for me on this. Can we chat in tomorrow?
@@ -94,6 +94,13 @@ func NewServer(svr lxd.ContainerServer) (*Server, error) { | |||
|
|||
name := info.Environment.ServerName | |||
clustered := info.Environment.ServerClustered | |||
if name == "" && !clustered { | |||
// If the name is set to empty and clustering is false, then it's highly |
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.
Suggest adding the bug number, as a reason why name shouldn't be empty.
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.
Sure
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.
Can we get a unit test for this?
We can mock GetServer
and we have an exported getter for Name()
...
The following check is not required for LXD 2.0 and causes failures when attempting to deploy a application to a machine. For LXD 3.0 it might make sense to enable this as it verifies the whole setup, but we can consider that overkill.
The LXD version API check is really verbose, like silly verboseness We'll drop the logging to Trace so it's still there if the levels are set at the right level.
QAing this one |
Even if we have no cluster available, we still want to find the instances first and if there are no instances at all the provisioner task needs to handle that case first.
The following ensures that the server name is indeed set correctly when the server name is empty. This is because LXD 2.0 returns no server name, as there was no concept of server name as there was only one server (not strictly true, but enough to understand the concept)
!!build!! |
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.
QA'd with apt installed lxc version 2.0.11 and snap installed lxc version 3.4. LGTM
|
Description of change
So the idea of the following is to set a default server name, so
that when we come to fill in the zones later on, the code in
question can identify it correctly.
From the code:
QA steps
Ensure that you've got LXD 2.0 installed (snap is the easiest way, removing
LXD 3.0.x from your system).
The following should not fail with the error
Documentation changes
n/a
Bug reference
https://bugs.launchpad.net/juju/+bug/1786309