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

Machine.CreateDevice() - added VLAN to args; only use args.Subnet/VLAN if set #59

Merged
merged 1 commit into from
Sep 19, 2016
Merged

Machine.CreateDevice() - added VLAN to args; only use args.Subnet/VLAN if set #59

merged 1 commit into from
Sep 19, 2016

Conversation

dimitern
Copy link

With the MAAS 2.0 API code path, Machine.CreateDevice() was modified slightly:

  • Added a VLAN field to CreateDeviceArgs (optional)
  • Made args.Subnet optional, only used if set.
  • When both Subnet and VLAN are set, CreateDevice() validates they match.

Needed in order to fix the following juju bug: http://pad.lv/1566791
There's nothing wrong with devices having unconfigured primary interface.

@@ -4,12 +4,14 @@
package gomaasapi

import (
"fmt"
"net/http"

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove blank line.

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Author

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"

Copy link
Contributor

Choose a reason for hiding this comment

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

another line to remove

Copy link
Author

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`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice error message.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks :)

@dimitern
Copy link
Author

$$merge$$

@jujubot
Copy link
Contributor

jujubot commented Sep 19, 2016

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

@jujubot jujubot merged commit 831cb32 into juju:master Sep 19, 2016
@dimitern dimitern deleted the gomaasapi-create-device-no-subnet branch September 19, 2016 10:21
@dimitern
Copy link
Author

Sorry @howbazaar I seem to have forgotten to push these changes - opening a new PR for them and merging it separately.

jujubot added a commit that referenced this pull request Sep 19, 2016
Forgot to push @thumper's suggested fixes :(

See #59 for details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants