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

Spaces API Facade Method - SubnetsByCIDR #11307

Merged
merged 10 commits into from Mar 13, 2020
Merged

Conversation

@manadart
Copy link
Member

manadart commented Mar 11, 2020

Please provide the following details to expedite Pull Request review:

Checklist

  • Checked if it requires a pylibjuju change?
  • Added integration tests for the PR?
  • Added or updated doc.go related to packages changed?
  • Do comments answer the question of why design decisions were made?

Description of change

This patch adds a new SubnetsByCIDR method to a new version of the subnets API server. It simply returns a slice of subnets matching each of the supplied CIDRs.

It is not recruited in this patch, but the intention is to use it as a validation step when clients assume they are referring to a unique subnet when indicating it by CIDR.

The AllSpaces method is removed from the subnets facade for this and future versions. It is not used by any clients and should never have been located on this facade.

QA steps

QA will be covered by steps in a forthcoming patch. Unit tests verify these changes.

Documentation changes

None.

Bug reference

N/A

manadart added 3 commits Mar 11, 2020
with the input CIDR.

This will be ultimately by used by clients to ensure uniqueness when
attempting to identify subnets by CIDR.

SubnetByCIDR now returns an error when multiple matches are found. This
will not affect callers yet, because provider network restrictions still
ensure CIDR uniqueness.
This is intended for use to ensure uniqueness when client-side methods
attempt to refer a single subnet by CIDR.
@manadart manadart changed the title Subnets by cidr Spaces API Facade Method - SubnetsByCIDR Mar 11, 2020
@@ -188,7 +188,7 @@ func (f *FakeSpace) Subnets() (bs []networkingcommon.BackingSubnet, err error) {
AvailabilityZones: zones,
Status: status,
}
outputSubnets = append(outputSubnets, &FakeSubnet{Info: backing, id: strconv.Itoa(i)})
outputSubnets = append(outputSubnets, &FakeSubnet{Info: backing, id: f.SpaceId + strconv.Itoa(i)})

This comment has been minimized.

Copy link
@manadart

manadart Mar 11, 2020

Author Member

Changed this to ensure that IDs are unique. Otherwise they repeat, starting at "0" for each space.

This stub networking implementation is luggage. The sooner it can be thrown away in favour of mocks, the better.

@manadart

This comment has been minimized.

Copy link
Member Author

manadart commented Mar 11, 2020

is returned with subnet parameters.
@manadart manadart force-pushed the manadart:subnets-by-cidr branch from db2d816 to e20a3da Mar 12, 2020
manadart added 2 commits Mar 12, 2020
This method should be a concern of the subnets facade.
facade, to the subnets facade where it belongs.

The AllSpaces method, not used by any client functionality, is deprecated
for all new versions of the subnets facade. This method should never
have resided here.
@manadart manadart force-pushed the manadart:subnets-by-cidr branch from 77f7e37 to fff8d8a Mar 12, 2020
@manadart

This comment has been minimized.

Copy link
Member Author

manadart commented Mar 13, 2020

$$merge$$

@jujubot jujubot merged commit 27d2932 into juju:develop Mar 13, 2020
4 of 5 checks passed
4 of 5 checks passed
Client Tests (ubuntu-latest) Client Tests (ubuntu-latest)
Details
Lint
Details
Schema
Details
merge-multi-juju Build started for merge commit.
Details
check-multi-juju Build finished.
Details
@manadart manadart deleted the manadart:subnets-by-cidr branch Mar 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.