Skip to content
This repository has been archived by the owner on Jun 26, 2023. It is now read-only.

Changes detect namespace deletion into object validator #1355

Merged
merged 1 commit into from
Feb 3, 2021
Merged

Changes detect namespace deletion into object validator #1355

merged 1 commit into from
Feb 3, 2021

Conversation

frbimo
Copy link
Contributor

@frbimo frbimo commented Jan 23, 2021 •
edited
Loading

This PR changes detect namespace deletion that was done in reconciler
into object validator by utilizing controller-runtime's client

fixes #1241
supersede PR #1284

test:
make test

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 23, 2021
@k8s-ci-robot
Copy link
Contributor

Hi @frbimo. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 23, 2021
@frbimo
Copy link
Contributor Author

frbimo commented Jan 23, 2021

/assign @adrianludwin

@k8s-ci-robot k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 30, 2021
@frbimo
Copy link
Contributor Author

frbimo commented Jan 31, 2021 •
edited
Loading

rebased due to #1367

adrianludwin
adrianludwin reviewed Jan 31, 2021
View reviewed changes
Copy link
Contributor

@adrianludwin adrianludwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I missed this PR when you first submitted it!

incubator/hnc/internal/validators/object.go Outdated Show resolved Hide resolved
incubator/hnc/internal/validators/object.go Outdated Show resolved Hide resolved
incubator/hnc/internal/validators/object.go Outdated Show resolved Hide resolved
incubator/hnc/internal/validators/object_test.go Outdated Show resolved Hide resolved
incubator/hnc/internal/validators/object_test.go Outdated Show resolved Hide resolved
incubator/hnc/internal/validators/object_test.go Outdated Show resolved Hide resolved
incubator/hnc/internal/validators/object.go Show resolved Hide resolved
@frbimo
Copy link
Contributor Author

frbimo commented Feb 1, 2021

ptal, in case i missed something

adrianludwin
adrianludwin reviewed Feb 1, 2021
View reviewed changes
Copy link
Contributor

@adrianludwin adrianludwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/ok-to-test

incubator/hnc/internal/reconcilers/hierarchy_config.go Show resolved Hide resolved
incubator/hnc/internal/validators/object.go Outdated Show resolved Hide resolved
incubator/hnc/internal/validators/object.go Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 1, 2021
@frbimo
Copy link
Contributor Author

frbimo commented Feb 3, 2021

request final review @adrianludwin

@frbimo
This PR changes detect func of namespace deletion that was done in re…
af37382 
…conciler

into object validator by utilizing controller-runtime's client
adrianludwin
adrianludwin reviewed Feb 3, 2021
View reviewed changes
Copy link
Contributor

@adrianludwin adrianludwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve
/hold
/assign @rjbez17

Nice work! Ryan, can you ptal?

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 3, 2021
@rjbez17
Copy link

rjbez17 commented Feb 3, 2021

/lgtm
/approve
/hold cancel

Thanks for this!

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 3, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adrianludwin, frbimo, rjbez17

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:
  • OWNERS [adrianludwin,rjbez17]

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

@rjbez17 rjbez17 mentioned this pull request Feb 3, 2021
Closed
@k8s-ci-robot k8s-ci-robot merged commit 9327e36 into kubernetes-retired:master Feb 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@adrianludwin adrianludwin adrianludwin left review comments

@rjbez17 rjbez17 Awaiting requested review from rjbez17

@srampal srampal Awaiting requested review from srampal

Assignees

@rjbez17 rjbez17

@adrianludwin adrianludwin

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

HNC: detect namespace deletion without a controller
4 participants
@frbimo @k8s-ci-robot @rjbez17 @adrianludwin