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

HNC: anchor in "conflict" mode can still delete subns #1149

Closed
adrianludwin opened this issue Sep 25, 2020 · 0 comments · Fixed by #1151
Closed

HNC: anchor in "conflict" mode can still delete subns #1149

adrianludwin opened this issue Sep 25, 2020 · 0 comments · Fixed by #1151
Assignees
@adrianludwin
Milestone
hnc-v0.5.3

Comments

@adrianludwin
Copy link
Contributor

Steps to reproduce:

  1. Create subns
  2. Delete subnamespaceOf annotation
  3. Delete anchor

Expected: namespace is not deleted
Actual: namespace is deleted
@adrianludwin adrianludwin added this to the hnc-v0.5.3 milestone Sep 25, 2020
adrianludwin added a commit to adrianludwin/multi-tenancy that referenced this issue Sep 25, 2020
@adrianludwin
Don't delete subns if anchor has conflict
815d9a5 
See issue kubernetes-retired#1149. This affects the ability to turn a subnamespace into a
full namespace. If you remove the subnamespaceOf annotation, move the
namespace to a new parent, and *then* clean up the anchor, everything's
ok. But if you delete the anchor _before_ moving the namespace, the
namespace gets deleted, which is clearly wrong.

This change is the minimal safe change needed to fix this in v0.5. In
v0.6, we should restructure these functions to be more heavily based on
the explicit state of the anchor, not the implied state.

Tested: new e2e test fails (hangs, actually) without this change and
passes with it. All e2e tests pass on GKE 1.17.
@adrianludwin adrianludwin mentioned this issue Sep 25, 2020
Merged
adrianludwin added a commit to adrianludwin/multi-tenancy that referenced this issue Sep 25, 2020
@adrianludwin
Don't delete subns if anchor has conflict
d6d4a2d 
See issue kubernetes-retired#1149. This affects the ability to turn a subnamespace into a
full namespace. If you remove the subnamespaceOf annotation, move the
namespace to a new parent, and *then* clean up the anchor, everything's
ok. But if you delete the anchor _before_ moving the namespace, the
namespace gets deleted, which is clearly wrong.

This change is the minimal safe change needed to fix this in v0.5. In
v0.6, we should restructure these functions to be more heavily based on
the explicit state of the anchor, not the implied state.

Tested: new e2e test fails (hangs, actually) without this change and
passes with it. All e2e tests pass on GKE 1.17.
adrianludwin added a commit to adrianludwin/multi-tenancy that referenced this issue Sep 25, 2020
@adrianludwin
Don't delete subns if anchor has conflict
8fddb26 
See issue kubernetes-retired#1149. This affects the ability to turn a subnamespace into a
full namespace. If you remove the subnamespaceOf annotation, move the
namespace to a new parent, and *then* clean up the anchor, everything's
ok. But if you delete the anchor _before_ moving the namespace, the
namespace gets deleted, which is clearly wrong.

This change is the minimal safe change needed to fix this in v0.5. In
v0.6, we should restructure these functions to be more heavily based on
the explicit state of the anchor, not the implied state.

Tested: new e2e test fails (hangs, actually) without this change and
passes with it. All e2e tests pass on GKE 1.17.
adrianludwin added a commit to adrianludwin/multi-tenancy that referenced this issue Sep 25, 2020
@adrianludwin
Improve anchor deletion
fad97e3 
This change fixes todos in the cherrypick for kubernetes-retired#1150 (see issue kubernetes-retired#1149).
It simplifies and restructures a lot of the logic to make it easier to
follow while looking at less data (e.g. a lot more focus on
anchor.Status.State).

Tested: all e2e tests pass on GKE 1.17.
@adrianludwin adrianludwin mentioned this issue Sep 25, 2020
Merged
adrianludwin added a commit to adrianludwin/multi-tenancy that referenced this issue Sep 25, 2020
@adrianludwin
Improve anchor deletion
daa456c 
This change fixes todos in the cherrypick for kubernetes-retired#1150 (see issue kubernetes-retired#1149).
It simplifies and restructures a lot of the logic to make it easier to
follow while looking at less data (e.g. a lot more focus on
anchor.Status.State).

Tested: all e2e tests pass on GKE 1.17.
@adrianludwin adrianludwin self-assigned this Sep 29, 2020
adrianludwin added a commit to adrianludwin/multi-tenancy that referenced this issue Sep 30, 2020
@adrianludwin
Don't delete subns if anchor has conflict
90530b4 
See issue kubernetes-retired#1149. This affects the ability to turn a subnamespace into a
full namespace. If you remove the subnamespaceOf annotation, move the
namespace to a new parent, and *then* clean up the anchor, everything's
ok. But if you delete the anchor _before_ moving the namespace, the
namespace gets deleted, which is clearly wrong.

This change is the minimal safe change needed to fix this in v0.5. In
v0.6, we should restructure these functions to be more heavily based on
the explicit state of the anchor, not the implied state.

Tested: new e2e test fails (hangs, actually) without this change and
passes with it. All e2e tests pass on GKE 1.17.
adrianludwin added a commit to adrianludwin/multi-tenancy that referenced this issue Sep 30, 2020
@adrianludwin
Improve anchor deletion
ad8b7af 
This change fixes todos in the cherrypick for kubernetes-retired#1150 (see issue kubernetes-retired#1149).
It simplifies and restructures a lot of the logic to make it easier to
follow while looking at less data (e.g. a lot more focus on
anchor.Status.State). It also adds a lots more documentation.

Tested: all e2e tests pass on GKE 1.18 when combined with fixes to the
e2e tests (PRs kubernetes-retired#1160, kubernetes-retired#1162, kubernetes-retired#1163 and kubernetes-retired#1164).
adrianludwin added a commit to adrianludwin/multi-tenancy that referenced this issue Sep 30, 2020
@adrianludwin
Improve anchor deletion
d5c3fe0 
This change fixes todos in the cherrypick for kubernetes-retired#1150 (see issue kubernetes-retired#1149).
It simplifies and restructures a lot of the logic to make it easier to
follow while looking at less data (e.g. a lot more focus on
anchor.Status.State). It also adds a lots more documentation.

Tested: all e2e tests pass on GKE 1.18 when combined with fixes to the
e2e tests (PRs kubernetes-retired#1160, kubernetes-retired#1162, kubernetes-retired#1163 and kubernetes-retired#1164).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@adrianludwin adrianludwin

Labels
None yet
Projects
None yet
Milestone
hnc-v0.5.3
Development

Successfully merging a pull request may close this issue.

Don't delete subns if anchor has conflict (master) adrianludwin/multi-tenancy
1 participant
@adrianludwin