Skip to content

Make security groups reconcile/delete async#1918

Merged
k8s-ci-robot merged 1 commit into
kubernetes-sigs:mainfrom
CecileRobertMichon:async-nsgs-2
Mar 2, 2022
Merged

Make security groups reconcile/delete async#1918
k8s-ci-robot merged 1 commit into
kubernetes-sigs:mainfrom
CecileRobertMichon:async-nsgs-2

Conversation

@CecileRobertMichon
Copy link
Copy Markdown
Contributor

@CecileRobertMichon CecileRobertMichon commented Dec 13, 2021 •
edited
Loading

What type of PR is this?
/kind feature

What this PR does / why we need it: Broke out NSGs from #1684. This follows the pattern set in #1541 and applies it to security groups.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1919

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

  • squashed commits
  • includes documentation
  • adds unit tests

Release note:

Make security groups reconcile/delete async

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 13, 2021
@k8s-ci-robot k8s-ci-robot added area/provider/azure Issues or PRs related to azure provider sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 13, 2021
CecileRobertMichon
CecileRobertMichon commented Dec 13, 2021
View reviewed changes
Comment thread azure/services/securitygroups/spec.go
@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

CecileRobertMichon commented Dec 13, 2021

/assign @mboersma @Jont828

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Dec 13, 2021

@CecileRobertMichon: GitHub didn't allow me to assign the following users: Jont828.

Note that only kubernetes-sigs members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

Details

In response to this:

/assign @mboersma @Jont828

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.

@CecileRobertMichon CecileRobertMichon force-pushed the async-nsgs-2 branch 7 times, most recently from 3f279c7 to 43e5067 Compare December 14, 2021 01:07
@CecileRobertMichon CecileRobertMichon mentioned this pull request Dec 14, 2021
Closed
3 tasks
Jont828
Jont828 reviewed Dec 14, 2021
View reviewed changes
Comment thread azure/services/securitygroups/client.go Outdated
Comment thread azure/services/securitygroups/client.go Outdated
Comment thread azure/services/securitygroups/securitygroups.go Outdated
Comment thread azure/services/securitygroups/securitygroups.go Outdated
Comment thread azure/services/securitygroups/securitygroups_test.go
Comment thread azure/services/securitygroups/spec.go Outdated
Comment thread azure/services/securitygroups/spec.go
@CecileRobertMichon CecileRobertMichon mentioned this pull request Dec 14, 2021
Merged
3 tasks
@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

CecileRobertMichon commented Dec 14, 2021

/retest

1 similar comment
@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

CecileRobertMichon commented Dec 14, 2021

/retest

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 15, 2021
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 16, 2021
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 11, 2022
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 4, 2022
@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

CecileRobertMichon commented Feb 4, 2022

updated the copyrights :)

@shysank
Copy link
Copy Markdown
Contributor

shysank commented Feb 4, 2022

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 4, 2022
Jont828
Jont828 reviewed Feb 4, 2022
View reviewed changes
Comment thread azure/services/securitygroups/securitygroups.go Outdated
Comment thread azure/services/securitygroups/securitygroups.go
Comment thread azure/services/securitygroups/securitygroups_test.go Outdated
Jont828
Jont828 reviewed Feb 4, 2022
View reviewed changes
Comment thread azure/services/securitygroups/securitygroups.go Outdated
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 4, 2022
@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

CecileRobertMichon commented Feb 7, 2022

@Jont828 thanks for the review. All comments should be addressed, PTAL

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 10, 2022
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 14, 2022
@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

CecileRobertMichon commented Feb 14, 2022

rebased

/assign @Jont828 @shysank

@CecileRobertMichon
Copy link
Copy Markdown
Contributor Author

CecileRobertMichon commented Feb 28, 2022

rebased on top of #2093

@shysank
Copy link
Copy Markdown
Contributor

shysank commented Feb 28, 2022

/retest

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Mar 2, 2022 •
edited
Loading

@CecileRobertMichon: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-cluster-api-provider-azure-apidiff f383406 link false /test pull-cluster-api-provider-azure-apidiff

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

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.

@shysank
Copy link
Copy Markdown
Contributor

shysank commented Mar 2, 2022

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 2, 2022
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Mar 2, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: shysank

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 2, 2022
@k8s-ci-robot k8s-ci-robot merged commit af95eed into kubernetes-sigs:main Mar 2, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.3 milestone Mar 2, 2022
@CecileRobertMichon CecileRobertMichon deleted the async-nsgs-2 branch February 17, 2023 23:26
@jackfrancis jackfrancis mentioned this pull request Jan 31, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Jont828 Jont828 Jont828 left review comments

@mboersma mboersma Awaiting requested review from mboersma

+1 more reviewer

@shysank shysank shysank left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@mboersma mboersma

@shysank shysank

@Jont828 Jont828

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/provider/azure Issues or PRs related to azure provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

v1.3

Development

Successfully merging this pull request may close these issues.

async securitygroups

5 participants

@CecileRobertMichon @k8s-ci-robot @shysank @mboersma @Jont828