[WIP] Bug 1936511 Do not override net uuid if subnet uuid is defined. #177
Conversation
If the user provides a subnet UUID under the networks filter, we should not override the net uuid of the subnet because by property of the subnet having UUID it means it already has been created, which means that here already is an ummutable link between the subnet and _a_ network Overriding the value of the net uuid in the subnet definition at that point can result in an invalid query, because the current code allows for listing nertwork UUIDs and subnet UUIDs under the same entry of providerSpec.Networks which do not belong together. The resulting lookup can end up in returning a port to be created which cannot be created by OpenStack. A port to be attached to a subnet that does not belong to the network listed. This patch give preference to the subnet associated with the subnet.UUID
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| if snetParam.UUID == "" { | ||
| sopts.ID = snetParam.UUID | ||
| } | ||
| sopts.NetworkID = netID |
There was a problem hiding this comment.
If you look a the code before for networks the following entry will fail if the subnet does not belong to the network
ProviderSpec: ...
Networks:
- UUID: my_net_uuid
subnets:
- uuid_of_subnet_not_under_my_net_uuid
It is even possible to create a problem if the subnet DOES belong to the net_uuid but the network filter returns more than one net.
For example, if for some reason there are two networks called "my_network" then the following entry will cause problems:
Providerspec:
Networks:
- filter:
name: my_network
subnets:
- uuid: uuid_of_subnet_belonging_to_one_of_the_networks
There was a problem hiding this comment.
and one more It think:
again two networks with name "my_network"
Providerspec:
Networks:
- filter:
- name: my_network
subnets:
- tags: primry-network
In this case oneport will be created correctly, while a second port will be created incorrectly (probably fail with an openstack error).
Note that I used network "name" to create the problem, but any filter under Networks, that returns more than one network will cause the same problem.
There was a problem hiding this comment.
So, while it might not be the best user experience, because subnet is a neseted field under networks, it makes sense that if you define a network, then define a subnet that is not in that network, it will fail to create the resource. The easy solution is to just not set the network ID since it is not required, and a subnet can always be found by its ID. Also, all this code does is make it so you can never set the subnet ID...
There was a problem hiding this comment.
"all this code does is make it so you can never set the subnet iD" << that is incorrect.
What this code does is use the subnet.uuid defined by the user. Which is probably what the user intends.
If the user, does not define the subnet.uuid, then the network filters/and uuid (if provided) prevails.
But the moment a user defines a subnet.uuid under a network filter, that is what should override anything else. We don't need any other piece of information to attach the port. The subnet.uuid is the only piece of information we need to create the ports because there exists one and only one network with that subnet.uuid in its list of subnets.
The problem we have been seen as of late is that users are defining a network through a filter, then adding a subnet.uuid. In that case the subnet.uuid is what should be used because it is the more exact match.
Anything else in the providerspec.networks is superfluous to the subnet lookup. There is one and only one subnet that will match the subnet.uuid.
So, if the user is providing a subnet.uuid then there already exists a network uuid associated with that subnet.uuid. We should not be overriding the network.uuid in the struct representing the subnet. If we do so, we basically make that struct invalid, and no port will get connected, that is specially true if the user has defined a network filter which will return two or more networks. There can only be one network that matches the network filter and the subnet.uuid filter. Every other network will not be attached because the subnet.uuid will only match one network.
That becomes a problem has, for example, defined two networks with the same tag, and the providerspc.network filter is matching on tags. If the user defines a -subnet.uuid entry then only one network will be connected, which probably not what the user intended.
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
|
Stale issues rot after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle rotten |
openshift-ci
bot
added
lifecycle/rotten
|
Rotten issues close after 30d of inactivity. Reopen the issue by commenting /close |
|
@openshift-bot: Closed this PR. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Currently when provider tries to update cluster status, it gets error: User "system:serviceaccount:openstack-provider-system:default" cannot update resource "clusters/status" in API group "cluster.k8s.io" in the namespace "default" This happens because "status" is separate subresource and access to it must be granted separately.
4 participants
If the user provides a subnet UUID under the networks filter,
we should not override the net uuid of the subnet because
by property of the subnet having UUID it means it already has
been created, which means that here already is an immutable link
between the subnet and a network
Overriding the value of the net uuid in the subnet definition at that point
can result in an invalid query, because
the current code allows for listing network UUIDs and subnet UUIDs under
the same entry of providerSpec.Networks which do not belong together.
The resulting lookup can end up in returning a port to be created which
cannot be created by OpenStack.
A port to be attached to a subnet that does not belong to the network
listed.
This patch give preference to the subnet associated with the subnet.UUID