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

Let subns with propagated objects be deleted #1139

Merged

Conversation

adrianludwin
Copy link
Contributor

@adrianludwin adrianludwin commented Sep 23, 2020

See issue #1130. The HNC object validator was preventing the K8s
namespace controller from cleaning out a namespace to allow it to be
deleted. This change lets HNC understand that when a namespace is being
deleted, we should ignore the subnamespaceOf annotation. This allows the
HierarchyConfiguration object to be reset to its initial state (like a
full namespace would when it's being deleted), orphaning the namespace
and allowing all its propagated objects to be removed.

Tested: added new tests and ran them multiple times while watching the
logs. The subnamespace test failed without this change and passed every
time with it, with the logs showing the expected behaviour.

Fixes #1130

See issue kubernetes-retired#1130. The HNC object validator was preventing the K8s
namespace controller from cleaning out a namespace to allow it to be
deleted. This change lets HNC understand that when a namespace is being
deleted, we should ignore the subnamespaceOf annotation. This allows the
HierarchyConfiguration object to be reset to its initial state (like a
full namespace would when it's being deleted), orphaning the namespace
and allowing all its propagated objects to be removed.

Tested: added new tests and ran them multiple times while watching the
logs. The subnamespace test failed without this change and passed every
time with it, with the logs showing the expected behaviour.
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 23, 2020
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 23, 2020
@adrianludwin
Copy link
Contributor Author

NB: I'm currently running all e2e tests on this patch; I'll remove my hold when they all pass.

/hold
/assign @rjbez17
/assign @yiqigao217
/cc @GinnyJI

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 23, 2020
@adrianludwin adrianludwin added this to the hnc-v0.5.3 milestone Sep 23, 2020
@rjbez17
Copy link

rjbez17 commented Sep 23, 2020

/approve
/lgtm

assuming the e2e tests pass

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 23, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adrianludwin, 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

Copy link
Contributor

@yiqigao217 yiqigao217 left a comment

Choose a reason for hiding this comment

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

/lgtm

@adrianludwin
Copy link
Contributor Author

All e2e tests passed, including the new ones.

/hold cancel

@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 Sep 23, 2020
@k8s-ci-robot k8s-ci-robot merged commit 3ac6a10 into kubernetes-retired:hnc-v0.5 Sep 23, 2020
@adrianludwin adrianludwin deleted the allow-ns-delete branch September 23, 2020 19:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants