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

Compute v2: Create aggregate #739

Merged
merged 3 commits into from
Jan 29, 2018
Merged

Conversation

dstdfx
Copy link
Contributor

@dstdfx dstdfx commented Jan 27, 2018

For #738

Links to the line numbers/files in the OpenStack source code that support the
code in this PR:

API doc:

Source code:

@coveralls
Copy link

coveralls commented Jan 27, 2018

Coverage Status

Coverage increased (+0.1%) to 73.259% when pulling 313db8b on dstdfx:aggregates-create into 4a3f5ae on gophercloud:master.

@dstdfx
Copy link
Contributor Author

dstdfx commented Jan 27, 2018

@jtopjian I'll add acceptance tests for this PR and #740 after they are merged. It must be useful to test them together in a single testcase.

@dstdfx dstdfx mentioned this pull request Jan 27, 2018
7 tasks
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jan 27, 2018

Build failed.

@dstdfx
Copy link
Contributor Author

dstdfx commented Jan 27, 2018

recheck

@theopenlab-ci
Copy link

theopenlab-ci bot commented Jan 27, 2018

Build succeeded.

Copy link
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

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

@dstdfx Thank you for working on this :)

Two small changes.

Also, the UnmarshalJSON stuff should be a separate PR. I'm OK with including it here, but in the future, please only one major change per PR.


type CreateOpts struct {
// The name of the host aggregate.
Name string `json:"name"`
Copy link
Contributor

Choose a reason for hiding this comment

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

name is a required field, so this should be:

Name string `json:"name" required:"true"`

@@ -1,7 +1,21 @@
/*
Package aggregates returns information about the host aggregates in the
Package aggregates creates aggregates and returns information about the host aggregates in the
Copy link
Contributor

Choose a reason for hiding this comment

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

Might as well rename this to:

Package aggregates manages information about the host aggregates in the
OpenStack cloud

@dstdfx
Copy link
Contributor Author

dstdfx commented Jan 28, 2018

@jtopjian Okay, will keep it in mind.

@theopenlab-ci
Copy link

theopenlab-ci bot commented Jan 28, 2018

Build succeeded.

@jtopjian
Copy link
Contributor

@dstdfx Looks good to me. Thank you for working on this. :)

@jtopjian jtopjian merged commit a924af7 into gophercloud:master Jan 29, 2018
dstdfx added a commit to dstdfx/gophercloud that referenced this pull request Jan 30, 2018
@dstdfx dstdfx deleted the aggregates-create branch January 31, 2018 19:47
dstdfx added a commit to dstdfx/gophercloud that referenced this pull request Jan 31, 2018
jtopjian pushed a commit that referenced this pull request Feb 2, 2018
* Add acceptance test for create and delete aggregate #739, #740

* Add reusable functions for Create/Delete aggregates

* Fix acceptance test
cardoe pushed a commit to cardoe/gophercloud that referenced this pull request Aug 27, 2020
…gophercloud#739)

* vendor: adding customdiff support

* Networking v2: Deprecate subnet allocation_pools for allocation_pool

This commit deprecates the openstack_networking_subnet_v2
allocation_pools argument in favor of allocation_pool.

allocation_pool is a singular word which makes more sense to use
declaratively.

In addition, allocation_pools was originally a TypeList, but the
OpenStack API will not respect the ordering defined by the user for
multiple pools, which triggered unneeded diffs.

allocation_pool is now a TypeSet which accounts for an unordered
response.

* Networking v2: Check allocation_pool first during subnet creation

* Networking v2: Refactor same allocation pool detection

* Networking v2: Add ConflictsWith for allocation_pool(s)
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.

None yet

3 participants