-
Notifications
You must be signed in to change notification settings - Fork 261
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
⚠️ Remove security group rules from status #1957
⚠️ Remove security group rules from status #1957
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mdbooth 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 |
✅ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
/test pull-cluster-api-provider-openstack-e2e-test |
This looks good, skipping |
This should have had /hold |
This is a bug in this patch:
|
D'oh! That's so dumb: when I was moving the code which generates We have test coverage of the various methods called by |
93393bd
to
1fb4ac5
Compare
I've fixed the bug and added some unit tests which would have triggered it. |
1fb4ac5
to
8d20070
Compare
/test pull-cluster-api-provider-openstack-e2e-full-test |
8d20070
to
5e114e1
Compare
/test pull-cluster-api-provider-openstack-e2e-full-test |
5e114e1
to
c4e5c2a
Compare
/test pull-cluster-api-provider-openstack-e2e-full-test |
/lgtm |
/hold cancel |
We were writing the full set of generated security rules to the status. They dominated the size of cluster object and we weren't reading them: we always fetch the current rules from OpenStack during reconciliation.
This also tweaks the rule generation code slightly, which was previously reading security group rules multiple times during a reconciliation. We now do this first and pass the result around.
Fixes: #1956