-
Notifications
You must be signed in to change notification settings - Fork 36
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
Machine.CreateDevice() - added VLAN to args; only use args.Subnet/VLAN if set #59
Machine.CreateDevice() - added VLAN to args; only use args.Subnet/VLAN if set #59
Conversation
@@ -4,12 +4,14 @@ | |||
package gomaasapi | |||
|
|||
import ( | |||
"fmt" | |||
"net/http" | |||
|
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.
Remove blank line.
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.
❌
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.
Did we decide to use ❌ to mark "Fixed"? Now it looks weird to me and I think I'd rather use 👍 instead
@@ -7,6 +7,7 @@ import ( | |||
"net/http" | |||
|
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.
another line to remove
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.
👍
}, | ||
errText: `missing Subnet not valid`, | ||
errText: `given subnet "1.2.3.4/5" on VLAN 42 does not match given VLAN 10`, |
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.
Nice error message.
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.
Thanks :)
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju-gomaasapi |
Sorry @howbazaar I seem to have forgotten to push these changes - opening a new PR for them and merging it separately. |
With the MAAS 2.0 API code path, Machine.CreateDevice() was modified slightly:
Needed in order to fix the following juju bug: http://pad.lv/1566791
There's nothing wrong with devices having unconfigured primary interface.