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

Update transient & report invalid HNS states #496

Merged
merged 1 commit into from
Mar 6, 2020

Conversation

yiqigao217
Copy link
Contributor

Update transient HNS states and report invalid states if bypassing the
admission controller. Add a channel in the HNS reconciler to enqueue
HNS for reconciliation. Add HNS reconciler and HC reconciler into each
other so that they can enqueue the other resource. Sync the HNS
instances in HNS reconciler, instead of syncing the
spec.requiredChildren in "syncChildren()" func in HC reconciler. Add
integration tests to test all different states.

Tested with integration tests ("make test" & "make test HNS=1") and
on GKE cluster. To test manually, I added the "enable-hns-reconciler"
flag and commented out the webhook "Prevent changing parent of a
required child" part, since it uses the "RequiredChildOf" field in the
forest. I tested deleting the subnamespace, changing the annotation,
change the parent of the subnamespace. They all worked as expected.

Part of #457 , #459 , #487 .

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 5, 2020
@yiqigao217
Copy link
Contributor Author

/assign @adrianludwin
/assign @sophieliu15

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.

Great commenting!

Only high-level change is that you can/should probably undo the changes to the kubectl client, as explained below.

incubator/hnc/pkg/kubectl/root.go Outdated Show resolved Hide resolved
incubator/hnc/pkg/kubectl/root.go Outdated Show resolved Hide resolved
incubator/hnc/pkg/kubectl/root.go Outdated Show resolved Hide resolved
incubator/hnc/pkg/reconcilers/hierarchical_namespace.go Outdated Show resolved Hide resolved
incubator/hnc/pkg/reconcilers/hierarchical_namespace.go Outdated Show resolved Hide resolved
incubator/hnc/pkg/reconcilers/hierarchical_namespace.go Outdated Show resolved Hide resolved
incubator/hnc/pkg/reconcilers/hierarchical_namespace.go Outdated Show resolved Hide resolved
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

@k8s-ci-robot k8s-ci-robot added lgtm Indicates that a PR is ready to be merged. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 6, 2020
Update transient HNS states and report invalid states if bypassing the
admission controller. Add a channel in the HNS reconciler to enqueue
HNS for reconciliation. Add HNS reconciler and HC reconciler into each
other so that they can enqueue the other resource. Sync the HNS
instances in HNS reconciler, instead of syncing the
spec.requiredChildren in "syncChildren()" func in HC reconciler. Add
integration tests to test all different states.

Tested with integration tests ("make test" & "make test HNS=1") and
on GKE cluster. To test manually, I added the "enable-hns-reconciler"
flag and commented out the webhook "Prevent changing parent of a
required child" part, since it uses the "RequiredChildOf" field in the
forest. I tested deleting the subnamespace, changing the annotation,
change the parent of the subnamespace. They all worked as expected.
@k8s-ci-robot k8s-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 6, 2020
@rjbez17
Copy link

rjbez17 commented Mar 6, 2020

/lgtm
/approve
/hold cancel

@k8s-ci-robot k8s-ci-robot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Mar 6, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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

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/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants