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
OpenStack: Remove SGS created by CPO on destroy #7378
Conversation
77e3e52
to
ce73a8a
Compare
If any of reviewers have a better idea how to find these SGs and not include any SG of other cluster I'd be grateful for advice. |
/retest
|
// We'll look through the ones on each of the ports and attempt to remove it from the port and delete it. | ||
// Most of the time it's a conflict, but last port should be guaranteed to allow deletion. | ||
// TODO(dulek): Currently this is the only way to do it and if delete fails there's no way to get back to | ||
// that SG. This is bad and we should make groups created by CPO tagged by cluster ID ASAP. |
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.
Is this issue being tracked somewhere so we don't forget about it?
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.
https://issues.redhat.com/browse/OSASINFRA-3200. This is a broader problem really, we don't have a good way to distinguish LBs between clusters too and currently we can have clashes by names in CPO. :/
cloud-provider-openstack can be configured to create security groups for the NodePorts of the load balancers it is creating. These SGs are then attached to the nodes. On `cluster destroy` we're orphaning them. This commit makes sure that we're looking for them. As they aren't tagged or have a proper cluster ID in the name, we will look at each of the ports, list its SGs and evaluate them comparing names with the pattern. If it matches, `destroy` will attempt to delete such SG.
/lgtm |
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.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mandre The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@dulek: The following test failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
4f59664
into
openshift:master
cloud-provider-openstack can be configured to create security groups for the NodePorts of the load balancers it is creating. These SGs are then attached to the nodes. On
cluster destroy
we're orphaning them. This commit makes sure that we're looking for them.As they aren't tagged or have a proper cluster ID in the name, we will look at each of the ports, list its SGs and evaluate them comparing names with the pattern. If it matches,
destroy
will attempt to delete such SG.